Closed
Bug 1140134
Opened 9 years ago
Closed 9 years ago
property in a CSS animation being overridden by later animation causes later properties in that animation to be skipped
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
VERIFIED
FIXED
mozilla39
People
(Reporter: dbaron, Assigned: dbaron)
References
Details
(Keywords: regression)
Attachments
(2 files)
876 bytes,
text/html
|
Details | |
3.40 KB,
patch
|
dholbert
:
review+
lmandel
:
approval-mozilla-aurora+
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This is a regression from bug 1078122 that I found by code inspection while working on bug 1125455. When a property in a CSS animation is overridden by later animation, it causes later properties in that animation to be skipped, because the code uses return rather than continue. We just shipped this in Firefox 36. [Tracking Requested - why for this release]: I think this is a pretty basic regression. It's probably not triggered a huge amount, but I'd be surprised if nothing hits it.
Assignee | ||
Comment 1•9 years ago
|
||
I confirmed with Linux nightly builds that it matches the one-day regression window I expected from code inspection: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=33c0181c4a25&tochange=29fbfc1b31aa
Assignee | ||
Comment 2•9 years ago
|
||
Both orange boxes should bounce (the first horizontally, the second vertically). The regression is that the second one doesn't move.
Assignee | ||
Comment 3•9 years ago
|
||
The fix is (I presume, not tested yet): https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/0d82194623ba/dont-skip-in-animation but I still need to write an automated test.
Assignee | ||
Comment 4•9 years ago
|
||
Both sets of new tests pass with the patch, but without the patch the "top is animating" test fails.
Attachment #8573563 -
Flags: review?(dholbert)
Assignee | ||
Comment 5•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca8e40b6a5a5
Comment 6•9 years ago
|
||
Comment on attachment 8573563 [details] [diff] [review] Don't skip the rest of the properties in an animation after hitting one that we shouldn't apply Looks good! r=me
Attachment #8573563 -
Flags: review?(dholbert) → review+
Assignee | ||
Comment 7•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d052486d57c3
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8573563 [details] [diff] [review] Don't skip the rest of the properties in an animation after hitting one that we shouldn't apply Approval Request Comment [Feature/regressing bug #]: bug 1078122 [User impact if declined]: Parts of CSS animations that should apply don't (because when one property in an animation is overridden by a later one in the animation-name list, we skip the rest of the properties for that animation) [Describe test coverage new/current, TreeHerder]: Added an automated test that covers it. (I think we had tests for overriding, but not for the other properties still working when one was overridden.) [Risks and why]: Very low risk; it fixes a return statement to be a continue statement as it should have been. [String/UUID change made/needed]: None.
Attachment #8573563 -
Flags: approval-mozilla-beta?
Attachment #8573563 -
Flags: approval-mozilla-aurora?
Comment 9•9 years ago
|
||
Tracking regression. We should be able to get this fix into 37 Beta 4.
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d052486d57c3
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment 11•9 years ago
|
||
Comment on attachment 8573563 [details] [diff] [review] Don't skip the rest of the properties in an animation after hitting one that we shouldn't apply Beta+ Aurora+
Attachment #8573563 -
Flags: approval-mozilla-beta?
Attachment #8573563 -
Flags: approval-mozilla-beta+
Attachment #8573563 -
Flags: approval-mozilla-aurora?
Attachment #8573563 -
Flags: approval-mozilla-aurora+
Updated•9 years ago
|
Flags: qe-verify+
Comment 14•9 years ago
|
||
This was verified as fixed on Firefox 37 Beta 4 on Windows 8.1 x64, Mac OS X 10.8.5, and Ubuntu 12.04 x64. Additional exploratory testing was also done for animations (https://etherpad.mozilla.org/Fx37b4) and did not reveal any new issues.
Comment 15•9 years ago
|
||
I was able to reproduce this issue on Firefox 37 Beta 4 (20150223185154) using Windows 7 64bit. Verified fixed on Firefox 39.0a1 (2015-03-12), Firefox 38.0a2 (2015-03-12) and Firefox 38.0a2 (2015-03-13) using Windows 7 64bit, Ubuntu 14.04 32bit and Mac OS X 10.9.5.
Comment 16•9 years ago
|
||
Sorry, I made a mistake: I wanted to say that I reproduced this issue on Firefox 37 Beta 1 (20150223185154).
You need to log in
before you can comment on or make changes to this bug.
Description
•