Closed Bug 880075 Opened 6 years ago Closed 6 years ago

More refactoring of nsTransitionManager::ConsiderStartingTransition

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: nrc, Assigned: nrc)

Details

Attachments

(2 files)

Followup to bug 879255 (probably should have noticed these earlier, but hey).
  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.
Attachment #758902 - Flags: review? → review?(dbaron)
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+
https://hg.mozilla.org/mozilla-central/rev/a3e65bf60566
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.