[css-values] mod() and rem() don't handle signed zero well
Categories
(Core :: CSS Parsing and Computation, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox124 | --- | fixed |
People
(Reporter: Oriol, Assigned: Oriol)
References
Details
Attachments
(1 file)
This is one of the failures that I detected in https://github.com/web-platform-tests/wpt/pull/43430
<!DOCTYPE html>
<script>
var el = document.documentElement;
el.style.zIndex = "calc(1 / mod(-1, -1))";
el.textContent = getComputedStyle(el).zIndex;
</script>
Expected: a very small (large in magnitude) negative value
Actual: 2147483647
These are all the related problematic cases:
mod(-1, -1)should be -0, but it's +0.mod(1, -1)should be -0, but it's +0.rem(-1, -1)should be -0, but it's +0.rem(-1, 1)should be -0, but it's +0.
As per https://drafts.csswg.org/css-values/#round-func, mod(A, B) is a value "between zero and B", which is defined as
if B is positive the range starts at 0⁺, and if B is negative it starts at 0⁻
For rem(A, B), if one value is positive and the other is negative, then it chooses "between zero and -B". If both values are positive or both negative, it behaves like mod(A, B).
|
||
Comment 1•2 years ago
|
||
Presumably this is fixed by bug 1867569?
|
Assignee | |
Comment 2•2 years ago
|
||
Doesn't seem to be the case...
https://searchfox.org/mozilla-central/rev/956e25260926097a4d54d5aeb0e06347841616bf/servo/components/style/values/generics/calc.rs#1047-1048
ModRemOp::Mod => dividend - divisor * (dividend / divisor).floor(),
ModRemOp::Rem => dividend - divisor * (dividend / divisor).trunc(),
E.g. for mod(-1, -1), this produces (-1) - (-1) * floor((-1) / (-1)) = -1 + 1 = +0 instead of -0
|
||
Comment 3•2 years ago
|
||
I'm not sure if there's a way to adjust the math to get the correct sign, but I suppose we could adjust the sign of the result as necessary. Maybe something like this?
ModRemOp::Mod => (dividend - divisor * (dividend / divisor).floor()).copysign(divisor),
ModRemOp::Rem => (dividend - divisor * (dividend / divisor).trunc()).copysign(
if dividend.signum() == divisor.signum() { divisor } else { dividend }
),
|
|
Assignee | |
Comment 4•2 years ago
|
||
For ModRemOp::Rem it can just be copysign(dividend) (if the signs are the same, then it doesn't matter which one we pick).
I think that would be correct, but in general changing the sign of a modulo can be wrong, so first I would check that the result is 0.
And in fact I don't think the current formulas can produce -0 at all, so I would do something like
ModRemOp::Mod => {
let r = dividend - divisor * (dividend / divisor).floor();
if r == 0.0 && divisor.is_sign_negative() { -0.0 } else { r }
},
ModRemOp::Rem => {
let r = dividend - divisor * (dividend / divisor).trunc();
if r == 0.0 && dividend.is_sign_negative() { -0.0 } else { r }
}
|
|
Assignee | |
Updated•2 years ago
|
|
|
Assignee | |
Comment 5•1 year ago
|
||
|
||
Updated•1 year ago
|
|
||
Comment 7•1 year ago
|
||
| bugherder | ||
Description
•