1336899 - transitioncancel is fired after transition has already fired transitionend

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Attachment: Test case

According to CSS transitions, only running transitions are cancelled yet in the attached test we fire a transitioncancel event for a finished transition.

Admittedly, in CSS transitions level 2[1] this is less than clear where, "not idle or after → idle" should probably be written "not idle and not after → idle".

Basically this code fails to check for the after state:

  // Handle cancel events firts
  if (mPreviousTransitionPhase != TransitionPhase::Idle &&
      currentPhase == TransitionPhase::Idle) {
    TimeStamp activeTimeStamp = ElapsedTimeToTimeStamp(aActiveTime);
    events.AppendElement(TransitionEventParams{ eTransitionCancel,
                                                aActiveTime,
                                                activeTimeStamp });
  }

While we're at it can we should also fix the following nits:

1. s/firts/first/ in the comment above

2. One unnecessary space before = in:

  TimeStamp zeroTimeStamp   = AnimationTimeToTimeStamp(zeroDuration);
  TimeStamp startTimeStamp  = ElapsedTimeToTimeStamp(intervalStartTime);
  TimeStamp endTimeStamp    = ElapsedTimeToTimeStamp(intervalEndTime);

3. The following lines are not needed:

    intervalStartTime = zeroDuration;
    intervalEndTime   = zeroDuration;

[1] https://drafts.csswg.org/css-transitions-2/#event-dispatch

Comment 1

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Marking this as P1 since we really shouldn't ship transitioncancel with this bug.

Comment 2

[Mantaroh Yoshinaga[:mantaroh]]

Thanks Brian,

Sorry, I misled the spec.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=deddb0ab91aa2429f6ff8d91205ae86d3e3f3ce9

Comment 3

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Bug 1336899 - Should not fire the transitioncancel after ending CSS Transition.

Review commit: https://reviewboard.mozilla.org/r/109982/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/109982/

Comment 4

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Comment on attachment 8833875 [details]
Bug 1336899 - Should not fire the transitioncancel after ending CSS Transition.

https://reviewboard.mozilla.org/r/109982/#review110986

Thanks!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1895e4ebd7e8
Should not fire the transitioncancel after ending CSS Transition. r=birtles

Comment 6

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Updated spec to make this clearer: https://github.com/w3c/csswg-drafts/commit/d6695bb6f4f185ad12f6977aac6a07932e68800d

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1895e4ebd7e8