1324985 - Disable firing the transitionrun and transitionstart on Aurora 52.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: (WIP) Disable firing transitionstart /transitionrun on Aurora 52

I landed transitionrun and transitionstart implementation on bug 1287983(Aurora 52), then I landed transitioncancel on bug 1264125(Nightly 53).

The each events are separated, so I think that this is not convinient for web developers in particular term.
I'd like to avoid releasing the events which related to CSS-Transition Level 2 on different timing.

So this cahngeset will disable firing transitionrun / transitionstart on Firefox 52.(We leave the each event handler interface, but these aren't fire never)

Comment 1

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

If we're going to stop firing transitionrun / transitionstart, we should also drop/hide the changes to the EventHandler interface. Web developers often feature detect by looking for, e.g. the presence of 'ontransitionstart' on an interface.

Comment 2

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Disable firing transitionstart /transitionrun on Aurora 52

Hi Brian,
Before requesting approval-aurora, Could you please review this patch?

I removed the code of firing transitionrun / transitionstart, and modified disable corresponded tests.

try result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d4392359b0ee00af45b675166c31637fb74afe5&group_state=expanded

Comment 3

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8820541 [details] [diff] [review]
Disable firing transitionstart /transitionrun on Aurora 52

Oh, Sorry,
I submited patch before reading comment #2, so I cancel the review?.

Comment 4

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: part 1 - Disable the events of ransitionstart /transitionrun on Aurora 52

Comment 5

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: part 2 - Hide transitionrun / transitionstart event on Aurora.

Hi Brian,

Thank you for your feedback.

(In reply to Brian Birtles (:birtles) from comment #1)
> If we're going to stop firing transitionrun / transitionstart, we should
> also drop/hide the changes to the EventHandler interface. Web developers
> often feature detect by looking for, e.g. the presence of
> 'ontransitionstart' on an interface.

I've removed the code of exposing these events.

Brian, Baku,
Could you review this code?

Comment 6

[Mantaroh Yoshinaga[:mantaroh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=870b4f27301ab64107a6c1e8d29c9914785d4846&group_state=expanded

Comment 7

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

Comment on attachment 8820561 [details] [diff] [review]
part 1 - Disable the events of ransitionstart /transitionrun on Aurora 52

Review of attachment 8820561 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/style/test/test_animations_event_order.html
@@ +445,2 @@
>                  [ divs[0], 'transitionend' ],
> +                'Sorting of transitionrunend events by time');

Nit: s/transitionrunend/transitionend/

@@ +472,2 @@
>                  [ divs[0], 'transitionend' ],
> +                'Sorting of transitionrunend events by time' +

Nit: s/transitionrunend/transitionend/

@@ +496,2 @@
>                  [ 'opacity', 'transitionend' ],
> +                'Sorting of transitionrunend events by transition-property')

Nit: s/transitionrunend/transitionend/

Comment 8

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: part 1 - Disable the events of ransitionstart /transitionrun on Aurora 52

Thanks, Brian,

(In reply to Brian Birtles (:birtles) from comment #7)
> Comment on attachment 8820561 [details] [diff] [review]
> part 1 - Disable the events of ransitionstart /transitionrun on Aurora 52
> 
> Review of attachment 8820561 [details] [diff] [review]:
> Nit: s/transitionrunend/transitionend/

Sorry, This is typo, I fixed it.

Comment 9

[Andrea Marchesini [:baku]]

Comment on attachment 8820562 [details] [diff] [review]
part 2 - Hide transitionrun / transitionstart event on Aurora.

Review of attachment 8820562 [details] [diff] [review]:
-----------------------------------------------------------------

Can you please comment them out and add a comment instead?

Comment 10

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: part 2 - Hide transitionrun / transitionstart event on Aurora.

Thanks, baku,

(In reply to Andrea Marchesini [:baku] from comment #9)
> Comment on attachment 8820562 [details] [diff] [review]
> part 2 - Hide transitionrun / transitionstart event on Aurora.
> 
> Review of attachment 8820562 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Can you please comment them out and add a comment instead?

I commented out this line, and added the comment.

Comment 11

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Disable firing transitionstart /transitionrun on Aurora 52

I squashed separated patch into this patch.

I'd like to relesae the new events from CSS Transitions Level 2 together in the same release.

Approval Request Comment:
[Feature/Bug causing the regression]: bug 1287983, bug 1264125
[User impact if declined]: transitionrun and transitionstart will ship ahead of transitioncancel; this may be confusing for developers who would prefer to assume that if any of these new events are available, they all are. For users, the only impact would be possibly broken content that assumes transitioncancel is available when it is not.
[Is this code covered by automated tests?]: Yes, this patch updates existing tests to reflect the disabled functionality.
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No, low risk.
[Why is the change risky/not risky?]: It merely disables functionality that has yet to ship.
[String changes made/needed]: None.

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 8824334 [details] [diff] [review]
Disable firing transitionstart /transitionrun on Aurora 52

disable transitionstart/transitionrun for aurora52.

Nit: s/ransitionstart/t&/ in the commit message

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bc05fa599f8c

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Backed out for bustage.
https://hg.mozilla.org/releases/mozilla-aurora/rev/4195687bda73679988aaff7b27a8c3b31e38a0fc

https://treeherder.mozilla.org/logviewer.html#?job_id=67018714&repo=mozilla-aurora

Comment 15

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: bug1324985.patch

Sorry, the previous patch is contained duplicate definition lines.
I fixed this definition and commit comment.

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

Comment 16

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/091c74eb5714ac9e211e76c8809e4c3e2b8b98b2