1924363 - CSS round function results in an incorrect value when dividing a sum of numbers by three

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Roman Komarov]

Attachment: CodePen - Math not mathing.html

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

Steps to reproduce:

1. Open https://codepen.io/kizu/pen/poMRYgg?editors=1100 or the attached HTML
2. Look at the first paragraph (simplified example) where the --result is calculated as round(down, (7 - 1) / 3, 1).
3. Look at the more complicated original example, at the elements with a thick hotpink outline.

Actual results:

• In the first paragraph, the result of the calculation is 1.
• In the follow-up complex example, the number in the first outlined element is 1 and is 0 in the next one.

Notably, this happens only when there is a calculation involved: (7 - 1) / 2 is buggy, while 6 / 2 is not. Other browsers (Chrome, Safari) handle this correctly.

Expected results:

• In the first paragraph, the result should be 2.
• The follow-up example should be the same as the one for the workaround: 0 in the first outlined element, and 1 in the second.

Comment 1

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

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

Comment 2

[Mayank Bansal]

I can repro.

Bisection:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=89d21fac92b8f98201b8715c6c050aace4b46afb&tochange=9330f8cb5765b67a89ac1427300f344a63d42cdc

Suspect: bug 1843695, which just enabled abs() and dign() functions on all platforms. So the actual regressor could be bug 1814588

cc: tlouw.

Comment 3

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

Set release status flags based on info from the regressing bug 1814588

:tlouw, since you are the author of the regressor, bug 1814588, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 4

[Tiaan Louw [:tlouw]]

During this calculation: (7 - 1) / 3 is simplified to (2.3333333 - 0.33333334) for some reason, which equals 1.99999996 and rounded down equals 1 and not 2 as expected.

Comment 5

[Tiaan Louw [:tlouw]]

Attachment: Bug 1924363 - Eagerly resolve sum nodes during parsing r=#layout

Product nodes are eagerly resolved during parse time, but sum nodes are
not. This might cause floating point inprecision in sum nodes, which
leads to invalid calculations, e.g. round(down, (7 - 1) / 3, 1) would
end up being round(down, (2.3333333 - 0.33333334), 1), then
round(down, 1.99999996, 1), which equals 1, which is incorrect.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2943453155ac
Eagerly resolve sum nodes during parsing r=layout-reviewers,emilio

Comment 7

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

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

Comment 8

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/2943453155ac

Comment 9

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

Upstream PR merged by moz-wptsync-bot

Comment 10

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

Upstream PR merged by moz-wptsync-bot

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider uplifting to ESR128? Not sure how common this is in the wild.

Comment 12

[Tiaan Louw [:tlouw]]

Attachment: Bug 1924363 - Eagerly resolve sum nodes during parsing r=#layout

Product nodes are eagerly resolved during parse time, but sum nodes are
not. This might cause floating point inprecision in sum nodes, which
leads to invalid calculations, e.g. round(down, (7 - 1) / 3, 1) would
end up being round(down, (2.3333333 - 0.33333334), 1), then
round(down, 1.99999996, 1), which equals 1, which is incorrect.

Original Revision: https://phabricator.services.mozilla.com/D225936

Comment 13

[Phabricator Automation]

esr128 Uplift Approval Request

• User impact if declined: Incorrect results of calculations inside CSS calc() function.
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: See same in bug.
• Risk associated with taking this patch: None, it has been released for a while now without issues.
• Explanation of risk level: n/a
• String changes made/needed: n/a
• Is Android affected?: yes

Comment 14

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/4f008c71b12e