rust_browser/_bmad-output/implementation-artifacts/1-12-media-rules.md

# Story 1.12: Media Rules

Status: done

<!-- Note: Validation is optional. Run validate-create-story for quality check before dev-story. -->

## 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

- [x] Task 1: Implement @import media query filtering (AC: #2)
  - [x] 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
  - [x] 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)
  - [x] 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
  - [x] 1.4 Thread `Viewport` through the import resolution pipeline: `resolve_imports()` and `resolve_imports_recursive()` need access to the viewport
  - [x] 1.5 If media queries are empty → fetch unconditionally (existing behavior). If media queries present but don't match viewport → skip fetch entirely
  - [x] 1.6 Add unit tests for @import with media query filtering

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

- [x] Task 3: Verify @import cascade ordering (AC: #1)
  - [x] 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)
  - [x] 3.2 Write a test: stylesheet A imports B, both define same property → A's value wins (later in cascade)
  - [x] 3.3 Verify import depth limit (5) and total import limit (32) are enforced
  - [x] 3.4 Verify cycle detection prevents infinite import loops

- [x] Task 4: Golden tests (AC: #5)
  - [x] 4.1 Create golden test fixture for `@media screen { ... }` applying styles
  - [x] 4.2 Create golden test fixture for `@media print { ... }` NOT applying styles
  - [x] 4.3 Create golden test fixture for `@import url(...)` loading and applying an external stylesheet (requires test HTTP server or file:// URL in fixture)
  - [x] 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`
  - [x] 4.5 Verify all existing @import and @media tests still pass

- [x] Task 5: Documentation and checklist (AC: #5)
  - [x] 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
  - [x] 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.rs` — `parse_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 update** — `docs/CSS2.1_Implementation_Checklist.md`
8. **CI validation** — `just 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`:
```rust
pub struct ImportDirective {
    pub url: String,
    pub media_queries: Vec<MediaQuery>,
}
```

2. **Parser change** — `parse_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.

3. **Resolution change** — In `resolve_imports_recursive()`, before fetching an import:
```rust
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
        }
    }
}
```

4. **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