From 19ca2e9bf920f97095fb0f85d24105b44348b0e3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 18 Oct 2026 10:52:43 +0000 Subject: [PATCH 1/4] css: keep math functions over two different length units as written LengthValue's TryOp and TryOpTo impls converted self to px twice when the two operands had different units. round(), rem(), mod(), hypot() and atan2() over 1in and 10px were evaluated as f(96px, 96px), and a second operand with no px value (1em) did not stop the fold. atan2() now reads both operands in px. It has no step. round(), rem(), mod() and hypot() over two different units are not folded. The px value of a unit is rounded (1pt is 1.3333334px), and round(), rem() and mod() jump where the quotient of the operands crosses a whole number, so a fold on px values can be a whole step away from the value a browser computes. The function stays as written. --- src/css/values/length.rs | 15 ++------- test/js/bun/css/color.test.ts | 7 +++++ test/js/bun/css/css.test.ts | 57 +++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 13 deletions(-) diff --git a/src/css/values/length.rs b/src/css/values/length.rs index 62c8e3a275c8..61452164296d 100644 --- a/src/css/values/length.rs +++ b/src/css/values/length.rs @@ -681,20 +681,11 @@ impl protocol::TryMap for LengthValue { impl protocol::TryOp for LengthValue { #[inline] fn try_op(&self, rhs: &Self, ctx: C, f: impl Fn(C, f32, f32) -> f32) -> Option { - // `LengthValue::try_op` takes a 2-arg closure (ctx folded - // in by caller); the `protocol::TryOp` shape passes `C` by-value with - // no `Copy` bound, so we can't capture `ctx` in an `Fn` closure that - // might be called more than once. Inline the same-unit + px-convert - // dispatch here so `f` is invoked exactly once on the chosen path. if core::mem::discriminant(self) == core::mem::discriminant(rhs) { let v = f(ctx, self.value(), rhs.value()); return Some(self.map_value(|_| v)); } - // Intentionally calls `self.to_px()` for BOTH operands (sic) — kept - // for behavioral compatibility. - if let (Some(a), Some(b)) = (self.to_px(), self.to_px()) { - return Some(LengthValue::Px(f(ctx, a, b))); - } + // A px fold can land a whole step off: `to_px` rounds, and round()/rem()/mod() jump. None } } @@ -704,9 +695,7 @@ impl protocol::TryOpTo for LengthValue { if core::mem::discriminant(self) == core::mem::discriminant(rhs) { return Some(f(ctx, self.value(), rhs.value())); } - // Intentionally calls `self.to_px()` for BOTH operands (sic) — kept - // for behavioral compatibility. - if let (Some(a), Some(b)) = (self.to_px(), self.to_px()) { + if let (Some(a), Some(b)) = (self.to_px(), rhs.to_px()) { return Some(f(ctx, a, b)); } None diff --git a/test/js/bun/css/color.test.ts b/test/js/bun/css/color.test.ts index 64477fbebbc0..b466bf952b65 100644 --- a/test/js/bun/css/color.test.ts +++ b/test/js/bun/css/color.test.ts @@ -574,6 +574,13 @@ describe("input forms", () => { expect(color("#gg0000", "hex")).toBeNull(); }); + // atan2(96px, 10px) is 84.05deg. A round() over two units is not evaluated, so atan2() has + // no number to use and the color cannot be converted. + test("a hue from atan2() of lengths in different units", () => { + expect(color("hsl(atan2(1in, 10px) 50% 50%)", "css")).toBe("#8cbf40"); + expect(color("hsl(atan2(round(1in, 10px), 1px) 50% 50%)", "css")).toBeNull(); + }); + test("alpha survives the object and array forms", () => { expect(color("#f00", "{rgba}")).toEqual({ r: 255, g: 0, b: 0, a: 1 }); expect(color("#f00", "[rgba]")).toEqual([255, 0, 0, 255]); diff --git a/test/js/bun/css/css.test.ts b/test/js/bun/css/css.test.ts index 6e7b32d3b5e6..068140f3ff3f 100644 --- a/test/js/bun/css/css.test.ts +++ b/test/js/bun/css/css.test.ts @@ -233,6 +233,63 @@ describe("css tests", () => { ); // ideally -400% - 8vh + 3ic minify_test(`a { top: calc(100% - 1 * 2 - 8 * 2); }`, `a{top:calc(100% - 2 - 16)}`); // ideally 100% - 18 }); + describe("math functions with mixed length units", () => { + // round(), rem(), mod() and hypot() over two lengths in different units stay as written. + // The second length can have no px value (1rem). If it has one, its px value is rounded + // (1pt is 1.3333334px), and round(), rem() and mod() can then land a whole step away. + // From WPT css/css-values/round-mod-rem-computed.html: + minify_test(`a { width: round(100px, 1rem) }`, `a{width:round(100px,1rem)}`); + minify_test(`a { width: mod(100px, 1rem) }`, `a{width:mod(100px,1rem)}`); + minify_test(`a { width: rem(100px, 1rem) }`, `a{width:rem(100px,1rem)}`); + // From the lightningcss tests: + minify_test(`a { width: round(22px, 5vw) }`, `a{width:round(22px,5vw)}`); + minify_test(`a { width: rem(18px, 5vw) }`, `a{width:rem(18px,5vw)}`); + minify_test(`a { width: mod(18px, 5vw) }`, `a{width:mod(18px,5vw)}`); + minify_test(`a { width: hypot(1px, 2vw) }`, `a{width:hypot(1px,2vw)}`); + minify_test(`a { width: hypot(1px, 2px, 3vw) }`, `a{width:hypot(1px,2px,3vw)}`); + // From MDN: + minify_test(`a { margin: mod(1000px, 29rem) }`, `a{margin:mod(1000px,29rem)}`); + minify_test(`a { margin: rem(1000px, 29rem) }`, `a{margin:rem(1000px,29rem)}`); + minify_test(`a { width: round(1in, 1em) }`, `a{width:round(1in,1em)}`); + minify_test(`a { width: round(1em, 1in) }`, `a{width:round(1em,1in)}`); + // Two absolute units. + minify_test(`a { width: round(1in, 10px) }`, `a{width:round(1in,10px)}`); + minify_test(`a { width: round(down, 70mm, 1cm) }`, `a{width:round(down,70mm,1cm)}`); + minify_test(`a { width: rem(1in, 10px) }`, `a{width:rem(1in,10px)}`); + minify_test(`a { width: mod(4px, 1pt) }`, `a{width:mod(4px,1pt)}`); + minify_test(`a { width: hypot(1in, 10px) }`, `a{width:hypot(1in,10px)}`); + minify_test(`a { width: hypot(1in, 3px, 4px) }`, `a{width:hypot(1in,3px,4px)}`); + minify_test(`a { border-spacing: round(1in, 10px) }`, `a{border-spacing:round(1in,10px)}`); + // The function also stays inside an outer function and in a media query. + minify_test(`a { width: calc(round(1in, 10px) + 1px) }`, `a{width:calc(round(1in,10px) + 1px)}`); + minify_test(`a { width: min(round(down, 70mm, 1cm), 250px) }`, `a{width:min(round(down,70mm,1cm),250px)}`); + minify_test( + `@media (min-width: round(1in, 10px)) { a { color: red } }`, + `@media (width>=round(1in,10px)){a{color:red}}`, + ); + // One unit still folds. + minify_test(`a { width: round(18px, 5px) }`, `a{width:20px}`); + minify_test(`a { width: mod(18px, 5px) }`, `a{width:3px}`); + minify_test(`a { width: hypot(3in, 4in) }`, `a{width:5in}`); + minify_test(`a { width: round(calc(1in + 1px), 10px) }`, `a{width:100px}`); + // atan2() has no step, so it reads both lengths in px. (The angle is printed in whichever + // of deg/rad is shorter: 1.467rad is 84.0531deg.) + minify_test(`a { transform: rotate(atan2(1in, 0px)) }`, `a{transform:rotate(90deg)}`); + minify_test(`a { transform: rotate(atan2(1in, 10px)) }`, `a{transform:rotate(1.467rad)}`); + minify_test(`a { transform: rotate(atan2(10px, -1in)) }`, `a{transform:rotate(3.0378rad)}`); + minify_test(`a { rotate: atan2(1cm, 1mm) }`, `a{rotate:1.47113rad}`); + // A length with no px value leaves atan2() as written. + minify_test(`a { transform: rotate(atan2(1in, 1em)) }`, `a{transform:rotate(atan2(1in,1em))}`); + minify_test(`a { rotate: atan2(1px, -1vw) }`, `a{rotate:atan2(1px,-1vw)}`); + // An at-rule prelude cannot hold an atan2() that is left as written, so the rule is an error. + test.each([ + [`@media (min-width: calc(100px * tan(atan2(1in, 1em)))) { a { color: red } }`, "Invalid media query"], + [`@container (min-width: calc(100px * tan(atan2(1in, 1em)))) { a { color: red } }`, "Invalid media query"], + [`@property --x { syntax: ''; inherits: false; initial-value: atan2(1in, 1em) }`, "Unexpected token"], + ])("ERROR: %s", (source, message) => { + expect(() => minify_test_with_options(source, "")).toThrow(message); + }); + }); describe("border_spacing", () => { minify_test( ` From f64b45323f0c438c86cfbf2a2995435b6f6c4105 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:21:41 +0000 Subject: [PATCH 2/4] css: pin the exact atan2() error and refresh a stale comment --- src/css/values/calc.rs | 8 ++++---- test/js/bun/css/css.test.ts | 6 +++++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/css/values/calc.rs b/src/css/values/calc.rs index 45fc6c521542..fff760681451 100644 --- a/src/css/values/calc.rs +++ b/src/css/values/calc.rs @@ -1665,10 +1665,10 @@ calc_protocol_forwarders!(Length { try_sign: forward, try_map: forward, try_op: |this, rhs, ctx, f| { - // Delegate to `protocol::TryOp for LengthValue` (which inlines the - // same-unit/px-convert dispatch) rather than the inherent `Length::try_op` - // — the inherent takes a 2-arg `Fn` closure, and adapting our 3-arg `f` - // to that without `C: Copy` would only yield `FnOnce`. + // Delegate to `protocol::TryOp for LengthValue` (same unit only) rather + // than the inherent `Length::try_op`: the inherent takes a 2-arg `Fn` + // closure, and adapting our 3-arg `f` to that without `C: Copy` would + // only yield `FnOnce`. if let (Length::Value(a), Length::Value(b)) = (this, rhs) { return ::try_op(a, b, ctx, f).map(Length::Value); } diff --git a/test/js/bun/css/css.test.ts b/test/js/bun/css/css.test.ts index 068140f3ff3f..a32b38addef3 100644 --- a/test/js/bun/css/css.test.ts +++ b/test/js/bun/css/css.test.ts @@ -282,10 +282,14 @@ describe("css tests", () => { minify_test(`a { transform: rotate(atan2(1in, 1em)) }`, `a{transform:rotate(atan2(1in,1em))}`); minify_test(`a { rotate: atan2(1px, -1vw) }`, `a{rotate:atan2(1px,-1vw)}`); // An at-rule prelude cannot hold an atan2() that is left as written, so the rule is an error. + minify_test( + `@property --x { syntax: ''; inherits: false; initial-value: atan2(1in, 10px) }`, + `@property --x{syntax:"";inherits:false;initial-value:1.467rad}`, + ); test.each([ [`@media (min-width: calc(100px * tan(atan2(1in, 1em)))) { a { color: red } }`, "Invalid media query"], [`@container (min-width: calc(100px * tan(atan2(1in, 1em)))) { a { color: red } }`, "Invalid media query"], - [`@property --x { syntax: ''; inherits: false; initial-value: atan2(1in, 1em) }`, "Unexpected token"], + [`@property --x { syntax: ''; inherits: false; initial-value: atan2(1in, 1em) }`, "Unexpected token: atan2("], ])("ERROR: %s", (source, message) => { expect(() => minify_test_with_options(source, "")).toThrow(message); }); From a5e75eceee579451e05a67520aed4c407f107d0d Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:22:57 +0000 Subject: [PATCH 3/4] [autofix.ci] apply automated fixes --- test/js/bun/css/css.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/js/bun/css/css.test.ts b/test/js/bun/css/css.test.ts index a32b38addef3..694ede631695 100644 --- a/test/js/bun/css/css.test.ts +++ b/test/js/bun/css/css.test.ts @@ -289,7 +289,10 @@ describe("css tests", () => { test.each([ [`@media (min-width: calc(100px * tan(atan2(1in, 1em)))) { a { color: red } }`, "Invalid media query"], [`@container (min-width: calc(100px * tan(atan2(1in, 1em)))) { a { color: red } }`, "Invalid media query"], - [`@property --x { syntax: ''; inherits: false; initial-value: atan2(1in, 1em) }`, "Unexpected token: atan2("], + [ + `@property --x { syntax: ''; inherits: false; initial-value: atan2(1in, 1em) }`, + "Unexpected token: atan2(", + ], ])("ERROR: %s", (source, message) => { expect(() => minify_test_with_options(source, "")).toThrow(message); }); From 23a7c92e579f0ff5c49dafbf90d869696a92c45c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:38:49 +0000 Subject: [PATCH 4/4] css: drop a comment about a Length::try_op that no longer exists --- src/css/values/calc.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/css/values/calc.rs b/src/css/values/calc.rs index fff760681451..cda07c41a6af 100644 --- a/src/css/values/calc.rs +++ b/src/css/values/calc.rs @@ -1665,10 +1665,6 @@ calc_protocol_forwarders!(Length { try_sign: forward, try_map: forward, try_op: |this, rhs, ctx, f| { - // Delegate to `protocol::TryOp for LengthValue` (same unit only) rather - // than the inherent `Length::try_op`: the inherent takes a 2-arg `Fn` - // closure, and adapting our 3-arg `f` to that without `C: Copy` would - // only yield `FnOnce`. if let (Length::Value(a), Length::Value(b)) = (this, rhs) { return ::try_op(a, b, ctx, f).map(Length::Value); }