get extra transition when modifying style during transitionend event

RESOLVED FIXED in mozilla1.9.3a1

Status

()

Core
CSS Parsing and Computation
P2
normal
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: dbaron, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
mozilla1.9.3a1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
Created attachment 419454 [details]
testcase (modified version of dolske's)

In bug 521890 comment 13 dolske posted a testcase showing double transitionend events being fired in a testcase reduced from his work on HTML5 videocontrols.

The problem seems to be the following (I didn't verify all the details of the following in the debugger, but I think it makes sense based on the ones I did):

 * at the time we call the transitionend event handler, we've posted a RestyleForAnimation on the element that's transitioning, but we haven't processed it yet
 * in this testcase (a modified version of dolske's), style is modified during the transitionend event handler
 * when that happens, we now have both animation and non-animation restyles for the element; we process the non-animation restyles first, and detect that opacity has changed a minute amount, and initiate *another* transition of 'opacity' on the element
Should the firing of the transitionend event just flush out styles?
(Reporter)

Comment 2

8 years ago
Maybe, although I want to think about whether there are other problems with the double restyle queue setup.
(Reporter)

Comment 3

8 years ago
I think it's actually possible to get into this situation in other ways,
especially after bug 528306 lands:  all you need is a posted restyle
that's pending when the refresh happens.

So I think the right solution here is to make the transition manager
hold on to completed transitions for one refresh cycle after they
complete.  This will allow this check:
    if (oldPT.mEndValue == pt.mEndValue) {
      // If we got a style change that changed the value to the endpoint
      // of the currently running transition, we don't want to interrupt
      // its timing function.
      // WalkTransitionRule already called RestyleForAnimation.
      return;
    }
in nsTransitionManager::ConsiderStartingTransition to prevent a new 
transition from starting in these cases.
(Reporter)

Comment 4

8 years ago
The patch at
http://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/431fe376650a/transition-keep-one-cycle
but I need to:
 (a) write tests for it, and
 (b) figure out what to do about its relationship to http://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/431fe376650a/transition-no-compute-distance , which it's currently on top of
(Reporter)

Comment 5

8 years ago
Er, I forgot the verb in that sentence.  Should have said "seems to fix this bug".
(Reporter)

Comment 6

8 years ago
Created attachment 420650 [details] [diff] [review]
patch

I added a test.

Regarding the relationship to the other patch in the previous comment:  to swap the order of the patches, I'd just take the introduction of the oldPT variable (and removal of endVal) in that patch and put it in this one.
Attachment #420650 - Flags: review?(bzbarsky)
Attachment #420650 - Flags: review?(bzbarsky) → review+
(Reporter)

Comment 8

8 years ago
http://hg.mozilla.org/mozilla-central/rev/263ab164ef4b
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
You need to log in before you can comment on or make changes to this bug.