Skip to content
6 changes: 3 additions & 3 deletions src/css/values/calc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ impl<V: CalcValue> Calc<V> {
return Err(i.new_custom_error(css::ParserError::invalid_value));
};

if to_angle && !rad.is_nan() {
if to_angle {
if let Some(val) = V::try_from_angle(Angle::Rad(rad)) {
return Ok(Calc::Value(Box::new(val)));
}
Expand Down Expand Up @@ -1609,10 +1609,10 @@ impl CalcValue for Percentage {
fn into_calc(self) -> Calc<Self> {
Calc::Value(Box::new(self))
}
fn from_calc(c: Calc<Self>, _input: &mut css::Parser) -> CssResult<Self> {
fn from_calc(c: Calc<Self>, input: &mut css::Parser) -> CssResult<Self> {
match c {
Calc::Value(v) => Ok(*v),
_ => Ok(Percentage { v: f32::NAN }),
_ => Err(input.new_custom_error(css::ParserError::invalid_value)),
}
}
}
Expand Down
31 changes: 20 additions & 11 deletions src/css/values/color.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1955,10 +1955,12 @@ impl ComponentParser {
}

if let Ok(value) = input.try_parse(CSSNumberFns::parse) {
Ok(NumberOrPercentage::Number { value })
Ok(NumberOrPercentage::Number {
value: censor_nan(value),
})
} else if let Ok(value) = input.try_parse(Percentage::parse) {
Ok(NumberOrPercentage::Percentage {
unit_value: value.v,
unit_value: censor_nan(value.v),
})
} else if self.allow_none {
input.expect_ident_matching(b"none")?;
Expand All @@ -1982,10 +1984,12 @@ impl ComponentParser {

if let Ok(angle) = input.try_parse(Angle::parse) {
Ok(css::color::AngleOrNumber::Angle {
degrees: angle.to_degrees(),
degrees: censor_nan(angle.to_degrees()),
})
} else if let Ok(value) = input.try_parse(CSSNumberFns::parse) {
Ok(css::color::AngleOrNumber::Number { value })
Ok(css::color::AngleOrNumber::Number {
value: censor_nan(value),
})
} else if self.allow_none {
input.expect_ident_matching(b"none")?;
Ok(css::color::AngleOrNumber::Number { value: f32::NAN })
Expand All @@ -2003,7 +2007,7 @@ impl ComponentParser {
}

if let Ok(val) = input.try_parse(Percentage::parse) {
Ok(val.v)
Ok(censor_nan(val.v))
} else if self.allow_none {
input.expect_ident_matching(b"none")?;
Ok(f32::NAN)
Expand All @@ -2020,7 +2024,7 @@ impl ComponentParser {
}

if let Ok(val) = input.try_parse(CSSNumberFns::parse) {
Ok(val)
Ok(censor_nan(val))
} else if self.allow_none {
input.expect_ident_matching(b"none")?;
Ok(f32::NAN)
Expand Down Expand Up @@ -2114,7 +2118,7 @@ impl RelativeComponentParser {
}
}) {
return Ok(css::color::AngleOrNumber::Angle {
degrees: value.to_degrees(),
degrees: censor_nan(value.to_degrees()),
});
}

Expand Down Expand Up @@ -2148,7 +2152,7 @@ impl RelativeComponentParser {
}
}) {
return Ok(NumberOrPercentage::Percentage {
unit_value: value.v,
unit_value: censor_nan(value.v),
});
}

Expand All @@ -2175,7 +2179,7 @@ impl RelativeComponentParser {
}
Err(i.new_custom_error(css::ParserError::invalid_value))
}) {
return Ok(value.v);
return Ok(censor_nan(value.v));
Comment on lines 2179 to +2182

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 A stylesheet whose typed @ property initial-value holds a valid relative color such as oklch(from #ff876a calc(l - .08) c h) now fails the whole bundle; on the base branch it built. RelativeComponentParser::parse_percentage wraps the channel keyword as a Percentage (color.rs:2172), so calc(l - .08) becomes a percentage+number sum that Percentage::from_calc now rejects (calc.rs:1615). @ property has no unparsed fallback and error_recovery is never true. Fix: read the keyword as the spec's (l for oklab/oklch, l*100 for lab/lch/hsl/hwb) and scale back, so calc(keyword +/- number) reduces in every percentage-only channel reader; Bun.color() also returns null for these inputs. [also at: src/css/values/calc.rs:1615 - Bundling a stylesheet whose typed @ property initial-value is a valid relative color like hsl(from blue h s calc(l + 20)) now fails with "Unexpected token: hsl("; on the base it built.]

Why this was flagged

Input: @ property --p { syntax: "<color>"; inherits: false; initial-value: hsl(from blue h s calc(l + 20)); } or the oklch form above, both valid CSS (css-color-5: the l keyword is a , Chrome accepts them). The relative arm at color.rs:2169-2183 builds Calc::Value(Box::new(Percentage { v })) for the keyword (color.rs:2172), so Calc::add at calc.rs:264 calls rhs.into_value() on the Number term, and Percentage::from_calc now returns Err(invalid_value) (calc.rs:1615). The literal fallback at color.rs:2009 cannot parse the keyword either, so the color errors. For a color: declaration this is caught as an unparsed property, but @ property propagates the error (src/css/rules/property.rs:141), error_recovery is only ever false (css_parser.rs:2952), and the stylesheet parse returns Err at css_parser.rs:2627, which the bundler reports as a bundle error. On the base branch from_calc folded the sum to Percentage { v: NaN }, so the rule built (with a wrong none channel).

Verification: RelativeComponentParser::parse_percentage (src/css/values/color.rs:2169-2172) wraps l as Calc::Value(Box::new(Percentage { v })). Calc::add at calc.rs:258-265 calls Percentage::from_calc, changed to Err(invalid_value) (calc.rs:1612-1616). PropertyRule::parse has no fallback at src/css/rules/property.rs:141, and error_recovery is only ever false (css_parser.rs:2952). On the base the same input built.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same root as the thread on #45970: the error comes from Percentage::from_calc, which is the first commit of this stack.

I did not copy either into this PR. #45970 now says to land #45807 first, and its tests assert the as-written output for the valid value in a test.todo row. I leave this thread open for a maintainer.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

Err(input.new_error_for_next_token())
Expand Down Expand Up @@ -2220,10 +2224,10 @@ impl RelativeComponentParser {
}) {
// PERF: I don't like this redundant allocation
if let Calc::Value(v) = calc_val {
return Ok(*v);
return Ok(censor_nan(*v));
}
if let Calc::Number(n) = calc_val {
return Ok(n);
return Ok(censor_nan(n));
}
}
Err(input.new_custom_error(css::ParserError::invalid_value))
Expand Down Expand Up @@ -2258,6 +2262,11 @@ impl RelativeComponentParser {
}
}

/// A calculation that is NaN is 0 (css-values-4 #top-level-calculation). NaN in a color is `none`.
fn censor_nan(value: f32) -> f32 {
if value.is_nan() { 0.0 } else { value }
Comment thread
robobun marked this conversation as resolved.
Resolved
}

bitflags::bitflags! {
/// A channel type for a color space.
#[derive(Clone, Copy, PartialEq, Eq)]
Expand Down
8 changes: 4 additions & 4 deletions test/bundler/css/wpt/color-computed-rgb.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,12 @@ describe("color-computed-rgb", () => {
runTest("Green channel resolves negative infinity to zero", "rgb(0, calc(-infinity), 0)", "#000");
runTest("Blue channel resolves negative infinity to zero", "rgb(0, 0, calc(-infinity))", "#000");
runTest("Alpha channel resolves negative infinity to fully transparent", "rgba(0, 0, 0, calc(-infinity))", "#0000");
runTest("Red channel resolves NaN to zero", "rgb(calc(NaN), 0, 0)", "rgb(calc(NaN), 0, 0)");
runTest("Green channel resolves NaN to zero", "rgb(0, calc(NaN), 0)", "rgb(0, calc(NaN), 0)");
runTest("Blue channel resolves NaN to zero", "rgb(0, 0, calc(NaN))", "rgb(0, 0, calc(NaN))");
runTest("Red channel resolves NaN to zero", "rgb(calc(NaN), 0, 0)", "#000");
runTest("Green channel resolves NaN to zero", "rgb(0, calc(NaN), 0)", "#000");
runTest("Blue channel resolves NaN to zero", "rgb(0, 0, calc(NaN))", "#000");
runTest("Alpha channel resolves NaN to zero", "rgba(0, 0, 0, calc(NaN))", "#0000");
// TODO: do this later, requires a lot of machinery to change in calc parsing
// not necessary for spec compliance as this is technially browser behavior
// runTest("Alpha channel resolves NaN to zero", "rgba(0, 0, 0, calc(NaN))", "#0000");
// runTest(
// "Red channel resolves NaN equivalent calc statements to zero",
// "rgb(calc(0 / 0), 0, 0)",
Expand Down
50 changes: 50 additions & 0 deletions test/js/bun/css/color.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,56 @@ describe("rgb() channel order and legacy syntax", () => {
});
});

// A <number> cannot be added to a <percentage>, so this is not a color.
test("a calculation that adds a number to a percentage is null", () => {
expect(color("rgb(calc(10 + 10%) 0 0)", "css")).toBeNull();
});

// These are null: the parser reads a channel keyword as a <percentage> where the channel takes
// only one, and a <number> cannot be added to it. https://github.com/oven-sh/bun/pull/45362
test.todo.each([
["hsl(from blue h s calc(l + 20))", "#66f"],
["lab(from lab(50% 10 20) calc(l + 20) a b)", "lab(70% 10 20)"],
])("%s is %s", (input, expected) => {
expect(color(input, "css")).toBe(expected);
});

// CSS Values 4 "Infinities, NaN, and Signed Zero": a calculation that is NaN is 0.
// Only the keyword `none` is a missing component.
describe("a calculation that is NaN in a channel is 0", () => {
test.each([
["lab(50% calc(NaN) 0)", "lab(50% 0 0)"],
["lab(50% calc(sqrt(-1)) 0)", "lab(50% 0 0)"],
["lab(calc(NaN * 1%) 10 0)", "lab(0% 10 0)"],
["lab(50% 10 0 / calc(NaN))", "lab(50% 10 0 / 0)"],
["lch(50% 10 calc(NaN * 1deg))", "lch(50% 10 0)"],
["oklch(50% .1 asin(2))", "oklch(50% .1 0)"],
["color(srgb calc(NaN) 0 0)", "color(srgb 0 0 0)"],
["rgb(calc(NaN), 34, 56)", "#002238"],
["rgba(12, 34, 56, calc(NaN))", "#0c223800"],
["hsl(calc(NaN), 50%, 50%)", "#bf4040"],
["lab(from lab(50% 10 20) l calc(a * NaN) b)", "lab(50% 0 20)"],
["lab(50% 0e999 0)", "lab(50% 0 0)"],
])("%s is %s", (input, expected) => {
expect(color(input, "css")).toBe(expected);
});

// asin() is an <angle>, also when it is NaN. A <number> channel does not take it.
test.each(["rgb(asin(2), 0, 0)", "lab(50% asin(2) 0)", "lab(50% 10 0 / asin(2))"])("%s is null", input => {
expect(color(input, "css")).toBeNull();
});

test("in the other output formats", () => {
expect(color("rgb(calc(NaN), 34, 56)", "{rgb}")).toEqual({ r: 0, g: 34, b: 56 });
expect(color("rgba(12, 34, 56, calc(NaN))", "{rgba}")).toEqual({ r: 12, g: 34, b: 56, a: 0 });
expect(color("hsl(calc(NaN), 50%, 50%)", "hex")).toBe("#bf4040");
});

test.each(["lab(50% none 0)", "lch(50% 10 none)", "color(srgb none 0 0)"])("%s keeps its none", input => {
expect(color(input, "css")).toBe(input);
});
});

describe("conversions between color spaces", () => {
// Each case converts a color whose channels all differ, so a channel landing
// in another channel's place shows up in the output. Mixing a color with
Expand Down
Loading
Loading