Closed
Bug 880075
Opened 11 years ago
Closed 11 years ago
More refactoring of nsTransitionManager::ConsiderStartingTransition
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: nrc, Assigned: nrc)
Details
Attachments
(2 files)
1.41 KB,
patch
|
dbaron
:
review-
|
Details | Diff | Splinter Review |
1.43 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
Followup to bug 879255 (probably should have noticed these earlier, but hey).
Assignee | ||
Comment 1•11 years ago
|
||
if (!shouldAnimate) { if (haveCurrentTransition) { // We're in the middle of a transition, but just got a // non-transition style change changing to exactly the // current in-progress value. (This is quite easy to cause // using 'transition-delay'.) This comment still doesn't make sense to me, the if clauses only ensure that we have a non-transition change and that there currently exists a transition of the same property, not that the change takes us to the current point.
Assignee | ||
Comment 2•11 years ago
|
||
Attachment #758902 -
Flags: review?
Assignee | ||
Updated•11 years ago
|
Attachment #758902 -
Flags: review? → review?(dbaron)
Assignee | ||
Comment 3•11 years ago
|
||
We only want to take one patch or the other, and if we take the second, we probably should change the comment to reflect what is happening or at least be more detailed.
Attachment #758904 -
Flags: review?(dbaron)
Comment on attachment 758902 [details] [diff] [review] check the current value of the transition So the "changing to exactly the current in-progress value" bit is the "!shouldAnimate" part of the condition. Though that's also true if we just got a style change changing to a non-interpolable value. But I think in either case the current comment is correct and the code is wrong.
Attachment #758902 -
Flags: review?(dbaron) → review-
Comment on attachment 758904 [details] [diff] [review] move the def of pts I'd suggest changing the comment: // We're in the middle of a transition, but just got a // non-transition style change changing to exactly the // current in-progress value. (This is quite easy to cause // using 'transition-delay'.) to something more like this (wrapped at 72 chars): We're in the middle of a transition, and just got a non-transition style change to something that we can't animate. This might happen because we got a non-transition style change changing to the current in-progress value (which is particularly easy to cause when we're currently in the 'transition-delay'). It also might happen because we just got a style change to a value that can't be interpolated.
Attachment #758904 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 6•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a3e65bf60566
Comment 7•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a3e65bf60566
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in
before you can comment on or make changes to this bug.
Description
•