Files
rust_browser/_bmad-output/implementation-artifacts/1-12-media-rules.md
Zachary D. Rowitsch 029e3c83bf Implement CSS 2.1 @import media query filtering and verify @media rules with code review fixes (§6.3, §7.2.1, §7.3)
Add ImportDirective type pairing @import URLs with parsed media queries,
enabling viewport-based filtering during import resolution so that e.g.
`@import "print.css" print` is skipped on screen. Thread Viewport through
the resolve_imports pipeline, strip CSS Cascade 5 layer/supports annotations
before media parsing, and add golden tests for @media screen/print behavior.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-14 11:56:46 -04:00

18 KiB

Story 1.12: Media Rules

Status: done

Story

As a web user, I want stylesheets imported via @import and conditional @media rules to apply correctly, So that pages load external stylesheets and respond to media types.

Acceptance Criteria

  1. Given a stylesheet with @import url("other.css") When the page is rendered Then the imported stylesheet's rules are fetched and applied with correct cascade order per CSS 2.1 §6.3

  2. Given a stylesheet with @import url("print.css") print When rendering for screen media type Then the imported stylesheet is not applied

  3. Given a @media screen { ... } block When rendering for screen media type Then the rules inside the block are applied

  4. Given a @media print { ... } block When rendering for screen media type Then the rules inside the block are not applied

  5. Golden tests cover @import and @media rules, checklist is updated, and just ci passes.

Tasks / Subtasks

  • Task 1: Implement @import media query filtering (AC: #2)

    • 1.1 In crates/css/src/parser/mod.rs (~line 163, parse_at_import()): change from ignoring the media list to storing it alongside the import URL
    • 1.2 Update Stylesheet.imports type from Vec<String> to Vec<ImportDirective> where ImportDirective { url: String, media_queries: Vec<MediaQuery> } (in crates/css/src/types.rs ~line 1084)
    • 1.3 In crates/app_browser/src/pipeline/stylesheets.rs (~line 108, resolve_imports_recursive()): when resolving an import, check if its media queries match the current viewport before fetching/merging
    • 1.4 Thread Viewport through the import resolution pipeline: resolve_imports() and resolve_imports_recursive() need access to the viewport
    • 1.5 If media queries are empty → fetch unconditionally (existing behavior). If media queries present but don't match viewport → skip fetch entirely
    • 1.6 Add unit tests for @import with media query filtering
  • Task 2: Verify @media rule filtering works end-to-end (AC: #3, #4)

    • 2.1 Confirm Rule.matches_viewport() in crates/css/src/types.rs (~line 1030) correctly evaluates @media screen and @media print
    • 2.2 Confirm crates/style/src/context.rs (~line 225) filters rules via matches_viewport() during cascade
    • 2.3 Verify that MediaType::Screen returns true and MediaType::Print returns false for the screen renderer
    • 2.4 Verify nested @media rules (AND semantics across nesting levels) work correctly
    • 2.5 Write integration test exercising the full pipeline: HTML with <style>@media print { ... }</style> → verify those rules don't apply
  • Task 3: Verify @import cascade ordering (AC: #1)

    • 3.1 Confirm resolve_imports_recursive() in crates/app_browser/src/pipeline/stylesheets.rs (~line 173) correctly prepends imported rules before the importing stylesheet's own rules (per CSS 2.1 §6.3)
    • 3.2 Write a test: stylesheet A imports B, both define same property → A's value wins (later in cascade)
    • 3.3 Verify import depth limit (5) and total import limit (32) are enforced
    • 3.4 Verify cycle detection prevents infinite import loops
  • Task 4: Golden tests (AC: #5)

    • 4.1 Create golden test fixture for @media screen { ... } applying styles
    • 4.2 Create golden test fixture for @media print { ... } NOT applying styles
    • 4.3 Create golden test fixture for @import url(...) loading and applying an external stylesheet (requires test HTTP server or file:// URL in fixture)
    • 4.4 Note on @import golden tests: Golden tests run via cargo test without a network stack. If the golden test harness doesn't support external stylesheet resolution, create a unit/integration test instead that exercises the full pipeline with tiny_http
    • 4.5 Verify all existing @import and @media tests still pass
  • Task 5: Documentation and checklist (AC: #5)

    • 5.1 Update docs/CSS2.1_Implementation_Checklist.md — mark @import as complete (including media list filtering) and @media as complete for CSS 2.1 media types
    • 5.2 Run just ci 2>&1 | tee /tmp/ci-output.txt and fix any issues

Dev Notes

CSS 2.1 Spec References

  • §6.3 @import rules: must precede all other rules except @charset. Imported stylesheets' rules are inserted at the point of the @import rule. Media-dependent imports: @import url("print.css") print; — import is conditional on the media type.
  • §7.2.1 @media rule: groups rules under a media condition. Multiple media types comma-separated (OR semantics). Rules inside @media only apply when the media condition matches.
  • §7.3 Media types: CSS 2.1 defines all, screen, print, and several others. For a screen renderer, only all and screen match.

Current @import/@media Pipeline State

Already working (DO NOT reimplement):

  • parse_at_import() in crates/css/src/parser/mod.rs (~line 163) — extracts URL from @import "url" and @import url("url") forms
  • Stylesheet.imports: Vec<String> stores raw import URLs
  • resolve_imports() / resolve_imports_recursive() in crates/app_browser/src/pipeline/stylesheets.rs — recursive fetch with depth limit (5), total limit (32), cycle detection via HashSet<String>
  • Imported rules prepended before importing stylesheet's rules (correct CSS cascade order)
  • parse_at_media() in crates/css/src/parser/mod.rs (~line 226) — parses @media blocks with media types + features
  • MediaType enum: All, Screen, Print, Unknown
  • MediaQuery struct with negated, media_type, features fields
  • MediaFeature enum: MinWidth, MaxWidth, MinHeight, MaxHeight, CSS Level 4 range syntax, PrefersColorScheme
  • Rule.media_conditions: Vec<Rc<Vec<MediaQuery>>> — tracks enclosing @media nesting levels
  • Rule.matches_viewport() — AND across nesting levels, OR within each level
  • MediaQuery.matches() — type matching + feature matching
  • Style cascade in crates/style/src/context.rs (~line 225) skips rules where !rule.matches_viewport(vp)
  • Viewport struct with width, height, prefers_color_scheme
  • Max media nesting depth: 10
  • Extensive test coverage: ~300+ test cases across at_rules.rs, media_tests.rs, media_query.rs

Must be added:

  • @import media query filtering — currently @import "x.css" print fetches x.css regardless of media type
  • ImportDirective type to pair URL with media queries
  • Threading Viewport through resolve_imports() pipeline
  • Golden tests proving @media screen/print behavior end-to-end

Key Code Locations

Component File What to modify
@import parsing crates/css/src/parser/mod.rs (~line 163) Store media queries alongside URL
Stylesheet imports type crates/css/src/types.rs (~line 1084) Change Vec<String> to Vec<ImportDirective>
Import resolution crates/app_browser/src/pipeline/stylesheets.rs (~line 89) Thread Viewport, filter by media
Recursive import crates/app_browser/src/pipeline/stylesheets.rs (~line 108) Check media queries before fetch
@media parsing crates/css/src/parser/mod.rs (~line 226) Already complete — verify only
Media type matching crates/css/src/types.rs (~line 954) Already complete — verify only
Style cascade filter crates/style/src/context.rs (~line 225) Already complete — verify only
Viewport type crates/css/src/types.rs (~line 873) Already complete — reuse for import filtering
@import parser tests crates/css/src/tests/parser_tests/at_rules.rs (~line 291) Add media query storage tests
@media evaluation tests crates/css/src/tests/media_tests.rs Verify existing, add if needed
Style integration tests crates/style/src/tests/media_query.rs Add end-to-end test
Pipeline stylesheet tests crates/app_browser/src/pipeline/tests/stylesheet_tests.rs Add import media filter test
Golden fixtures tests/goldens/fixtures/ New HTML fixtures for @media
Golden expected tests/goldens/expected/ Expected layout/display list outputs
Checklist docs/CSS2.1_Implementation_Checklist.md (~line 27) Update @import/@media items

Architecture Constraints

  • Layer boundary: @import resolution lives in app_browser (Layer 3) because it requires NetworkStack. The CSS parser (Layer 1) only extracts import directives; resolution happens at Layer 3.
  • No unsafe code — all changes are in safe Rust
  • Phase-based pipeline: Import resolution happens before style computation. Resolved stylesheets are fully merged before entering the cascade.
  • Viewport threading: The Viewport is already created in the rendering pipeline before style computation. It needs to be passed one step earlier, into import resolution.

Implementation Pattern (Follow Exactly)

  1. Type change in css/types.rs — define ImportDirective { url: String, media_queries: Vec<MediaQuery> }, update Stylesheet.imports
  2. Parser update in css/parser/mod.rsparse_at_import() stores parsed media queries in the directive (reuse existing parse_media_queries())
  3. Import resolution in app_browser/pipeline/stylesheets.rs — thread Option<&Viewport> through resolve_imports()resolve_imports_recursive(), skip imports whose media queries don't match
  4. Pipeline wiring in app_browser/pipeline/execution.rs — pass Viewport to import resolution (create viewport before import resolution step)
  5. Verify @media — write tests confirming existing @media filtering works (should pass already)
  6. Golden tests in tests/goldens/ — @media screen/print fixtures
  7. Checklist updatedocs/CSS2.1_Implementation_Checklist.md
  8. CI validationjust ci 2>&1 | tee /tmp/ci-output.txt

@import Media Query Implementation Strategy

The key change is small but touches the type boundary between CSS parser (Layer 1) and app_browser (Layer 3):

  1. New type ImportDirective:
pub struct ImportDirective {
    pub url: String,
    pub media_queries: Vec<MediaQuery>,
}
  1. Parser changeparse_at_import() already reads past the URL and sees the media list tokens. Currently it ignores them. Change to call parse_media_queries() on the remaining tokens before the semicolon.

  2. Resolution change — In resolve_imports_recursive(), before fetching an import:

if !directive.media_queries.is_empty() {
    if let Some(vp) = viewport {
        if !MediaQuery::evaluate_list(&directive.media_queries, vp) {
            continue; // Skip import — media doesn't match
        }
    }
}
  1. Viewport creation — The viewport is currently created from winit window dimensions. For import resolution, the viewport must be available before style computation. Check if execution.rs creates it early enough; if not, move viewport construction earlier in the pipeline.

Previous Story Learnings

From story 1-11 (Overflow):

  • Follow established patterns — don't reinvent existing infrastructure
  • Golden test fixtures: tests/goldens/fixtures/NNN-descriptive-name.html (sequential numbering)
  • Regen goldens: cargo test -p rust_browser --test regen_goldens -- --nocapture
  • just ci must pass clean at the end
  • When modifying types used across crate boundaries, ensure all consumers compile

Golden Test Considerations

@media tests can work in golden harness:

  • Inline <style>@media screen { .box { background: red } } @media print { .box { background: blue } }</style> — screen styles apply, print styles don't
  • No network needed — purely CSS cascade logic

@import tests may NOT work in golden harness:

  • Golden tests construct DOM + stylesheets in-memory without networking
  • To test @import behavior end-to-end, use an integration test with tiny_http (see existing pattern in tests/)
  • Alternatively, if the golden harness supports multiple inline stylesheets, simulate import ordering

Git Intelligence

Recent relevant commits:

  • e0ccce1 — CDO/CDC token handling (CSS tokenizer improvement)
  • 5b23b4d — CSS intermediate debug dump tooling (shows cascade inspection pattern)
  • 7c8d2b8 — :link/:visited pseudo-class support (shows style computation pipeline)

Testing Strategy

  1. Unit tests for ImportDirective type and parse_at_import() media query extraction
  2. Unit tests for import resolution with media query filtering (mock network or use in-memory stylesheets)
  3. Verify existing tests — all ~300+ @import/@media tests should continue passing
  4. Golden tests — minimum 2 fixtures:
    • @media screen { ... } styles applied (visible change)
    • @media print { ... } styles NOT applied (no visible change)
  5. Integration test (if @import can't be golden-tested) — tiny_http server with @import url("x.css") print → verify x.css rules don't apply on screen
  6. Regression — existing stylesheet pipeline tests pass unchanged

Project Structure Notes

  • Main work spans Layer 1 (css) and Layer 3 (app_browser) — this is acceptable since @import resolution already lives in Layer 3
  • css crate change: type definition only (no upward dependency introduced)
  • app_browser crate change: resolution logic + viewport threading
  • No new external dependencies needed
  • Existing parse_media_queries() function reused for @import media parsing

References

  • [Source: CSS 2.1 §6.3 — @import cascade ordering]
  • [Source: CSS 2.1 §7.2.1 — @media rule definition]
  • [Source: CSS 2.1 §7.3 — Media types]
  • [Source: crates/css/src/parser/mod.rs — @import parsing (~line 163), @media parsing (~line 226)]
  • [Source: crates/css/src/types.rs — MediaType, MediaQuery, MediaFeature, Rule.media_conditions (~line 873-1040)]
  • [Source: crates/app_browser/src/pipeline/stylesheets.rs — Import resolution (~line 89-186)]
  • [Source: crates/style/src/context.rs — Viewport-based rule filtering (~line 225)]
  • [Source: crates/css/src/tests/parser_tests/at_rules.rs — @import/@media parser tests]
  • [Source: crates/css/src/tests/media_tests.rs — Media query evaluation tests]
  • [Source: crates/style/src/tests/media_query.rs — Style integration tests for media]
  • [Source: docs/CSS2.1_Implementation_Checklist.md — @import/@media status (~line 27)]
  • [Source: _bmad-output/project-context.md — Full project rules and patterns]

Dev Agent Record

Agent Model Used

Claude Opus 4.6 (1M context)

Debug Log References

No debug issues encountered.

Completion Notes List

  • Defined ImportDirective { url, media_queries } type in crates/css/src/types.rs with PartialEq<&str> for backward-compatible test assertions
  • Updated parse_at_import() in crates/css/src/parser/mod.rs to parse media queries from @import url(...) <media>; directives using existing parse_media_queries()
  • Changed Stylesheet.imports from Vec<String> to Vec<ImportDirective>
  • Added make_viewport() helper to Pipeline to create viewport consistently
  • Threaded Option<&Viewport> through resolve_imports()resolve_imports_recursive() in crates/app_browser/src/pipeline/stylesheets.rs
  • Import resolution now skips imports whose media queries don't match the viewport (e.g., @import "print.css" print; is not fetched on screen)
  • Verified all existing @media filtering works correctly (128+ existing media tests pass)
  • Added 4 new unit tests for @import media query parsing and evaluation
  • Added 5 new integration tests for @import cascade ordering and media filtering
  • Created 2 golden tests (268-media-screen-applies, 269-media-print-ignored)
  • Updated CSS2.1 Implementation Checklist
  • All 4,663 tests pass; 7 pre-existing golden test failures (background image tests 241-248) are unrelated
  • just fmt and just lint pass clean

Code review fixes (2026-03-14):

  • [H1] Fixed @import regression: layer/layer(...) and supports(...) annotations are now stripped before parsing media queries, preventing incorrect import filtering
  • [M1] Removed unused ImportDirective::unconditional() dead code
  • [M2] Strengthened test_import_with_screen_media_applied to actually verify the imported stylesheet is fetched
  • [M3] Added doc comment warning on PartialEq<&str> impl for ImportDirective
  • [L1/L2] Added tests for negated (not print) and explicit all import media queries

File List

New files:

  • tests/goldens/fixtures/268-media-screen-applies.html
  • tests/goldens/fixtures/269-media-print-ignored.html
  • tests/goldens/expected/268-media-screen-applies.dl.txt
  • tests/goldens/expected/268-media-screen-applies.layout.txt
  • tests/goldens/expected/269-media-print-ignored.dl.txt
  • tests/goldens/expected/269-media-print-ignored.layout.txt

Modified files:

  • crates/css/src/types.rs — Added ImportDirective struct, updated Stylesheet.imports type, updated dump()
  • crates/css/src/parser/mod.rs — Updated parse_at_import() to return ImportDirective with media queries, added strip_import_annotations() to handle layer/supports() keywords
  • crates/css/src/lib.rs — Exported ImportDirective
  • crates/css/src/tests/parser_tests/mod.rs — Added MediaQuery, MediaType, Viewport to test prelude
  • crates/css/src/tests/parser_tests/at_rules.rs — Enhanced import tests, added tests for layer/supports stripping, negated/all media queries
  • crates/app_browser/src/pipeline/mod.rs — Added make_viewport() helper
  • crates/app_browser/src/pipeline/execution.rs — Updated resolve_imports() calls to pass viewport, use make_viewport()
  • crates/app_browser/src/pipeline/stylesheets.rs — Added viewport parameter and media query filtering to import resolution
  • crates/app_browser/src/pipeline/tests/stylesheet_tests.rs — Added 5 integration tests for import cascade/media filtering
  • tests/goldens.rs — Registered golden tests 268, 269
  • docs/CSS2.1_Implementation_Checklist.md — Marked @import and @media as complete