Closed Bug 1324985 Opened 3 years ago Closed 3 years ago

Disable firing the transitionrun and transitionstart on Aurora 52.

Categories

(Core :: CSS Parsing and Computation, defect)

41 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
firefox52 --- fixed
firefox53 --- unaffected

People

(Reporter: mantaroh, Assigned: mantaroh)

Details

Attachments

(2 files, 6 obsolete files)

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)
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.
Component: DOM: Animation → CSS Parsing and Computation
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
Attachment #8820533 - Attachment is obsolete: true
Attachment #8820541 - Flags: review?(bbirtles)
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?.
Attachment #8820541 - Attachment is obsolete: true
Attachment #8820541 - Flags: review?(bbirtles)
Attachment #8820561 - Attachment description: Disable the events of ransitionstart /transitionrun on Aurora 52 → part 1 - Disable the events of ransitionstart /transitionrun on Aurora 52
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?
Attachment #8820562 - Flags: review?(amarchesini)
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/
Attachment #8820561 - Flags: review?(bbirtles) → review+
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.
Attachment #8820561 - Attachment is obsolete: true
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?
Attachment #8820562 - Flags: review?(amarchesini) → review+
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.
Attachment #8820562 - Attachment is obsolete: true
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.
Attachment #8820983 - Attachment is obsolete: true
Attachment #8824260 - Attachment is obsolete: true
Attachment #8824334 - Flags: approval-mozilla-aurora?
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
Attachment #8824334 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attached patch bug1324985.patchSplinter Review
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
Flags: needinfo?(mantaroh)
You need to log in before you can comment on or make changes to this bug.