css: harden media-query evaluator and @media boundary

Address review feedback on PR #2478:

- MediaQuery.zig: strip CSS `/* ... */` comments before tokenization so
  `screen and /*x*/ (min-width: 1px)` evaluates the same as without the
  comment.
- MediaQuery.zig: bound-check `em` / `rem` multiplication via
  `std.math.mul` so a u32-overflowing length (e.g. `268435456em`) fails
  closed instead of panicking in debug or wrapping in release.
- StyleManager.zig: prelude brace search skips `/* ... */` comments, so
  `@media /* { fake */ screen { ... }` splits at the real opening brace
  rather than the one inside the comment.
- Tests: unit tests for stripped comments, em/rem overflow, and
  unimplemented units (cm/mm/pt/in/vw). HTML fixtures cover commented
  preludes/queries and the `replaceSync` cascade path.
This commit is contained in:
Navid EMAD
2026-05-15 20:49:45 +02:00
parent 68bd1441af
commit dd5e335262
4 changed files with 155 additions and 8 deletions

View File

@@ -125,7 +125,11 @@ fn applyMediaAtRule(self: *StyleManager, text: []const u8) !void {
if (!std.ascii.eqlIgnoreCase(text[0.."@media".len], "@media")) return;
var rest = text["@media".len..];
const open = std.mem.indexOfScalar(u8, rest, '{') orelse return;
// Use a comment-aware brace finder; a `/* { */` in the prelude would
// otherwise split the rule at the wrong place. The inner block's
// contents are re-parsed by CssParser below, which has its own trivia
// handling, so only this outer boundary needs the special-case scan.
const open = indexOfOpenBraceSkippingComments(rest) orelse return;
const query = std.mem.trim(u8, rest[0..open], &std.ascii.whitespace);
if (rest.len == 0 or rest[rest.len - 1] != '}') return;
@@ -146,6 +150,22 @@ fn applyMediaAtRule(self: *StyleManager, text: []const u8) !void {
}
}
/// Find the first `{` in `s` that is not inside a CSS `/* ... */` comment.
/// An unclosed comment returns `null` (treat the whole rule as malformed).
fn indexOfOpenBraceSkippingComments(s: []const u8) ?usize {
var i: usize = 0;
while (i < s.len) {
if (i + 1 < s.len and s[i] == '/' and s[i + 1] == '*') {
const close = std.mem.indexOf(u8, s[i + 2 ..], "*/") orelse return null;
i = i + 2 + close + 2;
continue;
}
if (s[i] == '{') return i;
i += 1;
}
return null;
}
fn addRawRule(self: *StyleManager, selector_text: []const u8, block_text: []const u8) !void {
if (selector_text.len == 0) return;

View File

@@ -55,7 +55,12 @@ pub const Viewport = struct {
/// Returns true if `query` matches the given viewport. Comma-separated
/// queries are evaluated independently and combined with OR.
pub fn matches(query: []const u8, viewport: Viewport) bool {
var rest = std.mem.trim(u8, query, &std.ascii.whitespace);
// Strip CSS `/* ... */` comments up-front so a comment in the prelude or
// inside a feature parenthesis can't shadow a brace, comma, or paren.
var buf: [4096]u8 = undefined;
const stripped = stripComments(query, &buf);
var rest = std.mem.trim(u8, stripped, &std.ascii.whitespace);
if (rest.len == 0) return false;
while (rest.len > 0) {
@@ -68,6 +73,31 @@ pub fn matches(query: []const u8, viewport: Viewport) bool {
return false;
}
/// Strip CSS comments (`/* ... */`) from `query` into `buf`, returning the
/// stripped slice. Each comment is replaced with a single space so token
/// boundaries are preserved (`a/*x*/b` becomes `a b`, not `ab`). An unclosed
/// `/* ...` returns the original input so callers fall through to the normal
/// parser error path. If the query is larger than the buffer, returns the
/// original input — sane media queries are ~tens of bytes so 4 KiB is plenty.
fn stripComments(query: []const u8, buf: []u8) []const u8 {
if (query.len > buf.len) return query;
var out: usize = 0;
var i: usize = 0;
while (i < query.len) {
if (i + 1 < query.len and query[i] == '/' and query[i + 1] == '*') {
const close = std.mem.indexOf(u8, query[i + 2 ..], "*/") orelse return query;
buf[out] = ' ';
out += 1;
i = i + 2 + close + 2;
continue;
}
buf[out] = query[i];
out += 1;
i += 1;
}
return buf[0..out];
}
fn nextTopLevelComma(s: []const u8) usize {
var depth: usize = 0;
for (s, 0..) |c, i| {
@@ -246,8 +276,11 @@ fn parseLengthPx(value: []const u8) ?u32 {
return if (num == 0) 0 else null;
}
if (std.ascii.eqlIgnoreCase(unit, "px")) return num;
if (std.ascii.eqlIgnoreCase(unit, "em")) return num * 16;
if (std.ascii.eqlIgnoreCase(unit, "rem")) return num * 16;
// `em` / `rem`: 1em = 16px. `std.math.mul` returns an error on u32 overflow
// (e.g. `268435456em` would otherwise wrap or panic in debug); treat that
// as an unparseable length so the query fails closed per MQ4.
if (std.ascii.eqlIgnoreCase(unit, "em")) return std.math.mul(u32, num, 16) catch null;
if (std.ascii.eqlIgnoreCase(unit, "rem")) return std.math.mul(u32, num, 16) catch null;
return null;
}
@@ -424,10 +457,43 @@ test "MediaQuery: not print is true on screen viewport" {
try testing.expect(matches("not print", v));
}
test "MediaQuery: complex real-world responsive pattern" {
// Decidim / Spree pattern: hide mobile CTA above breakpoint.
const v = Viewport.default(); // 1920×1080 — desktop
test "MediaQuery: common responsive breakpoint" {
// Pattern: hide one of mobile/desktop CTA duplicates above a breakpoint.
const v = Viewport.default(); // 1920×1080 — desktop side.
try testing.expect(matches("(min-width: 768px)", v));
// Inverse pattern: mobile-only CTA below breakpoint.
try testing.expect(!matches("(max-width: 767px)", v));
}
test "MediaQuery: comments are stripped" {
const v = Viewport.default();
// Comment between tokens.
try testing.expect(matches("screen and /*hidden*/ (min-width: 1px)", v));
// Comment at the start.
try testing.expect(matches("/* leading */ screen", v));
// Comment that would otherwise change parens depth.
try testing.expect(matches("(min-width: /* hi */ 600px)", v));
// Comment containing a comma — must not split the query.
try testing.expect(matches("screen /*, print*/", v));
// Unclosed comment falls through to the parser, which fails closed.
try testing.expect(!matches("/* unterminated", v));
}
test "MediaQuery: em / rem overflow fails closed" {
const v = Viewport.default();
// 268435456 × 16 overflows u32 (would wrap to 0); the evaluator must
// treat the length as unparseable and the query as non-matching.
try testing.expect(!matches("(min-width: 268435456em)", v));
try testing.expect(!matches("(min-width: 268435456rem)", v));
// Just below the overflow threshold still parses (but doesn't match
// because 268435455 × 16 > viewport width).
try testing.expect(!matches("(min-width: 268435455em)", v));
}
test "MediaQuery: unimplemented units fail closed" {
const v = Viewport.default();
try testing.expect(!matches("(min-width: 5cm)", v));
try testing.expect(!matches("(min-width: 50mm)", v));
try testing.expect(!matches("(min-width: 10pt)", v));
try testing.expect(!matches("(min-width: 1in)", v));
try testing.expect(!matches("(min-width: 50vw)", v));
}

View File

@@ -53,6 +53,15 @@
@media (min-width: 1px) {
#visibility-hidden-cascade { visibility: hidden; }
}
/* Comment-in-prelude: the brace inside the comment must not be mistaken
for the rule's opening brace. */
@media /* { fake */ (min-width: 1px) {
#commented-prelude { display: none; }
}
/* Comment-in-query: the prelude includes a comment between tokens. */
@media screen /*x*/ and (min-width: 1px) {
#commented-query { display: none; }
}
</style>
<body>
@@ -65,8 +74,12 @@
<div id="hidden-by-nested">nested-hit</div>
<div id="not-hidden-unsupported">unsupported</div>
<div id="visibility-hidden-cascade">visibility</div>
<div id="commented-prelude">commented-prelude</div>
<div id="commented-query">commented-query</div>
<div id="dyn-target">dyn-target</div>
<div id="dyn2-target">dyn2-target</div>
<style id="dyn"></style>
<style id="dyn2"></style>
</body>
<script id=cascade_matching_query_hides>
@@ -150,3 +163,28 @@
testing.expectEqual(4, rule.type);
}
</script>
<script id=cascade_commented_prelude>
{
// `@media /* { fake */ (min-width: 1px)` — the brace inside the comment
// would otherwise fool a naive prelude/inner split.
testing.expectFalse($('#commented-prelude').checkVisibility());
}
</script>
<script id=cascade_commented_query>
{
// Comments between query tokens must be stripped before evaluation.
testing.expectFalse($('#commented-query').checkVisibility());
}
</script>
<script id=cascade_replaceSync_matching>
{
// The _css_rules path also fires through replaceSync, not just insertRule.
$('#dyn2').sheet.replaceSync(
'@media (min-width: 600px) { #dyn2-target { display: none; } }'
);
testing.expectFalse($('#dyn2-target').checkVisibility());
}
</script>

View File

@@ -101,6 +101,29 @@
testing.expectEqual(true, window.matchMedia('(min-width: 30em)').matches);
// 121em = 1936px > 1920px → does not match.
testing.expectEqual(false, window.matchMedia('(min-width: 121em)').matches);
// u32-overflowing em / rem fails closed instead of panicking.
testing.expectEqual(false, window.matchMedia('(min-width: 268435456em)').matches);
testing.expectEqual(false, window.matchMedia('(min-width: 268435456rem)').matches);
}
</script>
<script id=matchMedia_comments_stripped>
{
// CSS `/* ... */` comments between query tokens are stripped per spec.
testing.expectEqual(true, window.matchMedia('screen and /*x*/ (min-width: 1px)').matches);
testing.expectEqual(true, window.matchMedia('/* leading */ screen').matches);
testing.expectEqual(true, window.matchMedia('(min-width: /* hi */ 600px)').matches);
}
</script>
<script id=matchMedia_unimplemented_units>
{
// Units the evaluator doesn't claim to support (cm/mm/pt/in/vw/vh) fail closed.
testing.expectEqual(false, window.matchMedia('(min-width: 5cm)').matches);
testing.expectEqual(false, window.matchMedia('(min-width: 50mm)').matches);
testing.expectEqual(false, window.matchMedia('(min-width: 10pt)').matches);
testing.expectEqual(false, window.matchMedia('(min-width: 1in)').matches);
testing.expectEqual(false, window.matchMedia('(min-width: 50vw)').matches);
}
</script>