diff --git a/src/css/values/calc.rs b/src/css/values/calc.rs index 45fc6c521542..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` (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`. if let (Length::Value(a), Length::Value(b)) = (this, rhs) { return ::try_op(a, b, ctx, f).map(Length::Value); } 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..694ede631695 100644 --- a/test/js/bun/css/css.test.ts +++ b/test/js/bun/css/css.test.ts @@ -233,6 +233,70 @@ 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. + 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: atan2(", + ], + ])("ERROR: %s", (source, message) => { + expect(() => minify_test_with_options(source, "")).toThrow(message); + }); + }); describe("border_spacing", () => { minify_test( `