1650189 - Transitions not working when specifying 'all' and an exception property with 0s duration

Status: VERIFIED FIXED

(Core :: CSS Transitions and Animations, defect)

Description

[Till Berger]

Attachment: Transition bug

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

See the attached test case. Hover the box.

Actual results:

When hovering the box, both width and height jump, even though transitions are set for all properties to 1s, and only height is set to 0s. Specifying a very small duration for the height instead (e.g. 1ms) makes the test case basically work as expected.

Expected results:

When hovering the box, the width should transition smoothly while the height should jump.

This seems to be broken since Firefox 77, it worked before and it works in other browsers.

Comment 1

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c0376a215632c46f2ffa35597a6b9b2d33e18322&tochange=60859a3d5c43263e3aa452d74e76360da6336e21

Comment 2

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

Emilio, are you interested in looking into this?

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Sure thing. CC'ing mrobinson who authored most of those patches too, just in case he can get to take a look sooner than me.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

I can confirm reverting https://hg.mozilla.org/integration/autoland/rev/89d28e030ba6eff042c8a572393a1455b295a853 fixes it, which is a bit odd, but not all that surprising from that set of changes I guess.

Digging a bit on why...

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Ouch... silly mistake in that patch, surprised no test caught it before, really :(

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Till, how do you want to be credited for the test-case? Usually we do stuff like <link rel="author" href="mailto:[e-mail]" title="Emilio Cobos Álvarez">, but I'd understand if you didn't want to have a mailto: link in a public test suite, so I could just use the http://mellthas.de link, or just don't mention you altogether, your call.

Thanks for filing and for the excellent test-case btw :)

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1650189 - Fix an off-by-one in the transition property iterator. r=mrobinson,birtles

By the time we get to iterate over the longhands of a shorthand, we've
already advanced the range iterator, so we look at the next duration and
such, which causes this bug.

I'm seriously baffled that no existing test caught this when it
landed, neither in our internal test suite nor wpt... :/

Comment 8

[Emilio Cobos Álvarez (:emilio)]

I went for the http:// link for now but I'll update before or after landing if appropriate.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9161641 [details]
Bug 1650189 - Fix an off-by-one in the transition property iterator. r=mrobinson,birtles

Beta/Release Uplift Approval Request

• User impact if declined: Some transitions will use the wrong duration.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Fixes a silly mistake that apparently is hard to hit in practice.
• String changes made/needed: none

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9161641 [details]
Bug 1650189 - Fix an off-by-one in the transition property iterator. r=mrobinson,birtles

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Simple fix for CSS transitions regression in 77.
• User impact if declined: See comment 0, some CSS transitions will use the wrong duration.
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple fix + test.
• String or UUID changes made by this patch: none

Comment 11

[Till Berger]

(In reply to Emilio Cobos Álvarez (:emilio) (PTO until Jul 10) from comment #6)

Till, how do you want to be credited for the test-case? Usually we do stuff like <link rel="author" href="mailto:[e-mail]" title="Emilio Cobos Álvarez">, but I'd understand if you didn't want to have a mailto: link in a public test suite, so I could just use the http://mellthas.de link, or just don't mention you altogether, your call.

Thanks for filing and for the excellent test-case btw :)

The credit with the link is fine, thanks!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a98d6188d72
Fix an off-by-one in the transition property iterator. r=mrobinson

Comment 13

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/24462 for changes under testing/web-platform/tests

Comment 14

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 15

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/7a98d6188d72

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9161641 [details]
Bug 1650189 - Fix an off-by-one in the transition property iterator. r=mrobinson,birtles

Approved for 79.0b5. Thanks for including a test.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c887a5a9c796

Comment 18

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

(In reply to Emilio Cobos Álvarez (:emilio) (PTO until Jul 10) from comment #7)

I'm seriously baffled that no existing test caught this when it
landed, neither in our internal test suite nor wpt... :/

Yes, me too!

Thanks for fixing this so quickly!

Comment 19

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 20

[Simona Badau, Desktop QA]

Reproduced the initial issue on Firefox 78.0.1.
Verified as fixed using the latest Nightly 80.0a1 (Build ID: 20200707094747) on Ubuntu 18.04, Mac OS X 10.15, and Windows 10 x64.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9161641 [details]
Bug 1650189 - Fix an off-by-one in the transition property iterator. r=mrobinson,birtles

Approved for 78.1esr.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/89c8569239fe

Comment 23

[Simona Badau, Desktop QA]

Verified as fixed also on the latest Firefox 79 beta 5 and on Firefox 78.1.0 esr (on a Treeherder build, Build ID: 20200707210843), on Ubuntu 18.04, Mac OS X 10.15 and Windows 10 x64.