get extra transition when modifying style during transitionend event

RESOLVED FIXED in mozilla1.9.3a1



CSS Parsing and Computation
8 years ago
8 years ago


(Reporter: dbaron, Unassigned)


(Blocks: 1 bug)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(2 attachments)



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?

Comment 2

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

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.
in nsTransitionManager::ConsiderStartingTransition to prevent a new 
transition from starting in these cases.

Comment 4

8 years ago
The patch at
but I need to:
 (a) write tests for it, and
 (b) figure out what to do about its relationship to , which it's currently on top of

Comment 5

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

Comment 6

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

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+

Comment 8

8 years ago
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.