Closed Bug 1859048 Opened 6 months ago Closed 6 months ago

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

Categories

(Core :: CSS Parsing and Computation, defect)

Firefox 120
defect

Tracking

()

RESOLVED FIXED
121 Branch
Tracking Status
firefox-esr115 --- wontfix
firefox118 --- wontfix
firefox119 --- wontfix
firefox120 --- fixed
firefox121 --- fixed

People

(Reporter: janeori, Assigned: jfkthame)

References

Details

Attachments

(2 files)

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

Blocks: 1764850
Component: Untriaged → CSS Parsing and Computation
Product: Firefox → Core
Status: UNCONFIRMED → NEW
Ever confirmed: true

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
Severity: -- → S3
Summary: round(up) and round(down) incorrect values → CSS round(up, N, N) resolves to 2N rather than to N (and similarly, round(down, -N, -N) produces -2N rather than -N)

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.

Summary: CSS round(up, N, N) resolves to 2N rather than to N (and similarly, round(down, -N, -N) produces -2N rather than -N) → CSS round(up, N, N) resolves to 2N rather than to N (and similarly, round(down, -N, N) produces -2N rather than -N)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Pushed by jkew@mozilla.com:
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
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/42951 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 121 Branch
Upstream PR merged by moz-wptsync-bot

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.

Flags: needinfo?(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
Flags: needinfo?(jfkthame)
Attachment #9361901 - Flags: approval-mozilla-beta?
Attachment #9361953 - Flags: approval-mozilla-beta?

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

Attachment #9361901 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9361953 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: in-testsuite+
Duplicate of this bug: 1864804
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: