Closed Bug 1867908 Opened 7 months ago Closed 6 months ago

[css-values] round() with a negative step is wrong

Categories

(Core :: CSS Parsing and Computation, defect, P3)

defect

Tracking

()

RESOLVED FIXED
123 Branch
Tracking Status
firefox123 --- fixed

People

(Reporter: Oriol, Assigned: eliotj12, Mentored)

References

Details

(Keywords: good-first-bug)

Attachments

(1 file, 1 obsolete file)

<!DOCTYPE html>
<div style="z-index: round(nearest, 2.5, -1)"></div>
<div style="z-index: round(up, 2.5, -1)"></div>
<div style="z-index: round(down, 2.5, -1)"></div>
<div style="z-index: round(to-zero, 2.5, -1)"></div>
<script>
for (let el of document.querySelectorAll("div")) {
  el.textContent = getComputedStyle(el).zIndex;
}
</script>

Expected:

3
3
2
2

Actual:

2
2
3
2

Blink and WebKit are right.

https://searchfox.org/mozilla-central/rev/956e25260926097a4d54d5aeb0e06347841616bf/servo/components/style/values/generics/calc.rs#994-996

let div = value / step;
let lower_bound = div.floor() * step;
let upper_bound = div.ceil() * step;

If step < 0, then lower_bound > upper_bound, breaking the logic.

Note that the sign of the step shouldn't matter, so just doing let step = step.unitless_value().abs(); should be enough.

Keywords: good-first-bug
Mentor: emilio
Severity: -- → S3
Priority: -- → P3
Assignee: nobody → eliotj12
Status: NEW → ASSIGNED

(In reply to Oriol Brufau [:Oriol] from comment #1)

Note that the sign of the step shouldn't matter, so just doing let step = step.unitless_value().abs(); should be enough.

I've made that change. Have built and tested, and am now getting correct results (3,3,2,2 - per your example).

Attachment #9369931 - Attachment is obsolete: true
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/789ecd85405c
Fix round() with negative step returning incorrect value r=emilio
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/43790 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 123 Branch
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: