1859048 - CSS round(up, N, N) resolves to 2N rather than to N (and similarly, round(down, -N, N) produces -2N rather than -N)

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Jane Ori]

Steps to reproduce:

round(up, 1px, 1px) computes to 2px, it should be 1px

round(down, -1px, 1px) computes to -2px, it should be -1px

Here is a reduced test case:
https://codepen.io/propjockey/pen/bGOJOQe/3fa426c2617e00b5481f35583b70c1ff?editors=1100

Actual results:

round(up, 1px, 1px) computes to 2px
round(down, -1px, 1px) computes to -2px,

Expected results:

round(up, 1px, 1px) should be 1px
round(down, -1px, 1px) should be -1px

Comment 1

[Daniel Holbert [:dholbert]]

Are you seeing any browser give your expected results?

In my testing, Safari 17 seems to match our behavior here, and it looks like Chrome doesn't yet implement round().

Having said that, I agree it seems unintuitive; and https://drafts.csswg.org/css-values/#funcdef-round does seem to suggest that we should be resolving to 1px here:

The round(<rounding-strategy>?, A, B) function [...]
If A is exactly equal to an integer multiple of B, round() resolves to A exactly

Comment 2

[Jane Ori]

Hey! Thanks for hoping on it so quick!

Are you seeing any browser give your expected results?

Yes, Safari 17 fixed the bug

(I also reported it to them but it was closed as resolved)
https://bugs.webkit.org/show_bug.cgi?id=263145

Also, the spec mentions that round(down) is equivalent to JS's Math.floor() and round(up) is Math.ceil(), so technically all browsers give the expected result in JS.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1859048 - Fix incorrect CSS rounding behavior when value is an exact multiple of step. r=#style

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1859048 - Add some testcases for round() of value that is an integer multiple of the step size. r=#style

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0dc5343a24be
Fix incorrect CSS rounding behavior when value is an exact multiple of step. r=firefox-style-system-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/04359a37825b
Add some testcases for round() of value that is an integer multiple of the step size. r=firefox-style-system-reviewers,emilio

Comment 6

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

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

Comment 7

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/0dc5343a24be
https://hg.mozilla.org/mozilla-central/rev/04359a37825b

Comment 8

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

Upstream PR merged by moz-wptsync-bot

Comment 9

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

The patch landed in nightly and beta is affected.
:jfkthame, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox120 to wontfix.

For more information, please visit BugBot documentation.

Comment 10

[Jonathan Kew [:jfkthame]]

Comment on attachment 9361901 [details]
Bug 1859048 - Fix incorrect CSS rounding behavior when value is an exact multiple of step. r=#style

Beta/Release Uplift Approval Request

• User impact if declined: The CSS round() function may return the wrong result.

(While round() is probably not yet in wide use on the web, the error here is so egregiously wrong and the fix so trivial that I think we should take it.)

• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small, targeted fix for the specific case (value is exact multiple of step) that was mishandled
• String changes made/needed:
• Is Android affected?: Yes

Comment 11

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

Comment on attachment 9361901 [details]
Bug 1859048 - Fix incorrect CSS rounding behavior when value is an exact multiple of step. r=#style

Approved for 120.0b8

Comment 12

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/4ef8fec1bdb3
https://hg.mozilla.org/releases/mozilla-beta/rev/38f5ce2d997a