1917331 - CSS custom properties do not transition with certain floating point values

Status: RESOLVED FIXED

(Core :: CSS Transitions and Animations, defect)

Description

[John Renner]

Attachment: ffbug2.html

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:130.0) Gecko/20100101 Firefox/130.0

Steps to reproduce:

1. Declare a custom property with syntax <length> (though this also works with <number>):

@property --x {
    syntax: "<length>";
    inherits: false; /* field required but value irrelevant to this bug */
    initial-value: 0px;
}

2. Style a component with a transition for this custom property:

div {
    position: absolute;
    left: var(--x);
    transition: --x 1s;
}

3. Set X to one of these problematic floats. (e.g. 112.24859)

document.querySelector('div').style.setProperty("--x", "112.24859px");

Actual results:

The element snaps immediately to its new position without animation.

Expected results:

The element should have smoothly transitioned to the new position. If you use a different float such as: 100.1 you'll see a smooth animation.

If you perform this same experiment by defining a transition for left instead of --x, the transition will succeed even with the "bad" number 112.24859.

I've attached a repro that lets you try different values (100.1 or 112.24859) and different transition properties (--x or left). You can see that the jumping only occurs when you transition --x to 112.24859px.

Note: there are many such floating points that trigger this. This is just one example. As I said above, this also occurs if --x is a <number> (and you use left: calc(var(--x) * 1px) )

This example animates smoothly in both safari and chrome

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::CSS Transitions and Animations' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

So I haven't dug into this (cc'ing some folks which are familiar with the relevant code), but this is most likely due to how we round / uncompute the values triggering the "transition has changed, thus cancel it" code-path.

In particular, this floating point value when going through a regular length and re-serializing, it gives a subtly different value:

document.body.style.letterSpacing = "112.24859px"
document.body.style.letterSpacing // "112.249px"

So maybe we need to make those equality checks more subtle somewhere.

Comment 3

[Jonathan Kew [:jfkthame]]

Feels like we might have a mismatch of float vs double resulting in unexpected results from "equality' checks.

Comment 4

[Boris Chiou [:boris]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

In particular, this floating point value when going through a regular length and re-serializing, it gives a subtly different value:

Yes. I noticed the uncompute() function for the custom properties' case uses to_css_string()
https://searchfox.org/mozilla-central/rev/f549a50b1e39b1e6bea19912d92545c4c0a06b7b/servo/components/style/properties_and_values/value.rs#359

Basically, at somewhere, e.g. creating a new CSS transition, the self (computed value) was:

Value { v: Component(Length(112.24859 px)), url_data: URLExtraData { chrome_rules_enabled: false, base: "...", referrer: "" } }

But after we called to_css_string(), it became 112.249px.

Comment 5

[Boris Chiou [:boris]]

Attachment: Bug 1917331 - Add test for a random floating-point number to make sure we create the transition for the custom properties.

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1917331 - Keep parsed value when uncomputing for animations. r=boris,#style

I think this is a bit more consistent, and makes us preserve the
invariant that from_computed_value().to_computed_value() round-trips.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a2fc04a7e4c1
Keep parsed value when uncomputing for animations. r=boris,firefox-style-system-reviewers
https://hg.mozilla.org/integration/autoland/rev/c969e2c6952e
Add test for a random floating-point number to make sure we create the transition for the custom properties. r=layout-reviewers,firefox-style-system-reviewers,emilio

Comment 8

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

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f61858eb2d62
Fix debug_assert triggered by a wpt on android.

Comment 10

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/a2fc04a7e4c1
https://hg.mozilla.org/mozilla-central/rev/c969e2c6952e
https://hg.mozilla.org/mozilla-central/rev/f61858eb2d62

Comment 11

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

Upstream PR merged by moz-wptsync-bot

Comment 12

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

Upstream PR merged by moz-wptsync-bot

Comment 13

[John Renner]

just tried it out in nightly and it works great! thanks all involved!