rust_browser/_bmad-output/implementation-artifacts/1-5-table-layout-completeness.md

# Story 1.5: Table Layout Completeness

Status: done

## Story

As a web user,
I want HTML tables to render with correct cell sizing, borders, and captions,
so that tabular data displays correctly on real websites.

## Acceptance Criteria

1. **Given** a table with `border-collapse: collapse`, **When** the page is rendered, **Then** adjacent cell borders are merged using the conflict resolution rules per CSS 2.1 §17.6.2
2. **Given** a table with `border-collapse: separate` and `border-spacing`, **When** the page is rendered, **Then** cells are separated by the specified spacing
3. **Given** a table with `caption-side: top` or `caption-side: bottom`, **When** the page is rendered, **Then** the caption is placed above or below the table box respectively
4. **Given** a table with `empty-cells: hide` and cells containing no content, **When** the page is rendered, **Then** borders and backgrounds of empty cells are not drawn
5. **Given** a table with cells spanning rows/columns and percentage/fixed widths, **When** the page is rendered, **Then** the table layout algorithm distributes space correctly per CSS 2.1 §17.5
6. Golden tests cover each table feature, checklist is updated, and `just ci` passes

## Tasks / Subtasks

> **NOTE:** AC #1 (collapse), #2 (separate + spacing), and #5 (spanning + widths) are **already substantially implemented**. This story completes AC #3 (caption-side), #4 (empty-cells), and fills remaining gaps.

- [x] Task 1: `caption-side` property implementation (AC: #3)
  - [x] 1.1 Add `enum CaptionSide { Top, Bottom }` in `crates/style/src/types/text.rs` (alongside existing `BorderCollapse`, `TableLayout`)
  - [x] 1.2 Add `PropertyId::CaptionSide` in `crates/css/src/types.rs` (~line 348-487, PropertyId enum)
  - [x] 1.3 Add `CssValue` handling and parsing for `caption-side` in `crates/css/src/parser/mod.rs` — keywords: `top` (default), `bottom`
  - [x] 1.4 Add `caption_side: CaptionSide` to `ComputedStyles` in `crates/style/src/types/computed.rs` (default: `Top`)
  - [x] 1.5 Wire cascade resolution in `crates/style/src/resolver.rs`
  - [x] 1.6 Add `caption_side: CaptionSide` to `LayoutBox` in `crates/layout/src/types.rs`
  - [x] 1.7 Modify `layout_table_captions()` in `crates/layout/src/engine/table/mod.rs:301-322` to check `caption_side`:
    - `Top`: layout captions before rows (current behavior)
    - `Bottom`: layout captions after rows, offset by total row height
  - [x] 1.8 Modify `calculate_table_height()` in table mod.rs to account for bottom caption height
  - [x] 1.9 Unit tests for caption-side parsing and layout
  - [x] 1.10 Golden tests: caption-side top (verify existing), caption-side bottom

- [x] Task 2: `empty-cells` property implementation (AC: #4)
  - [x] 2.1 Add `enum EmptyCells { Show, Hide }` in `crates/style/src/types/text.rs`
  - [x] 2.2 Add `PropertyId::EmptyCells` in `crates/css/src/types.rs`
  - [x] 2.3 Add parsing for `empty-cells` — keywords: `show` (default), `hide`
  - [x] 2.4 Add `empty_cells: EmptyCells` to `ComputedStyles` (default: `Show`) and `LayoutBox`
  - [x] 2.5 Wire cascade resolution
  - [x] 2.6 Implement empty cell detection: a cell is "empty" if it has no content (no child nodes, or only whitespace text nodes) per CSS 2.1 §17.6.1.1
  - [x] 2.7 In display list builder (`crates/display_list/src/builder.rs`), when rendering a TableCell with `empty-cells: hide` in `border-collapse: separate` model, skip painting borders and backgrounds
  - [x] 2.8 Note: `empty-cells: hide` only applies in `border-collapse: separate` — in collapsed model, borders are shared and always painted
  - [x] 2.9 Unit tests for empty cell detection
  - [x] 2.10 Golden test: empty-cells show vs hide with border-spacing

- [x] Task 3: Collapsed border completeness (AC: #1 — fill remaining gaps)
  - [x] 3.1 Audit border style priority in `crates/layout/src/engine/table/collapsed.rs` against CSS 2.1 §17.6.2.1 conflict resolution rules:
    - Rule 1: `border-style: hidden` always wins
    - Rule 2: Wider borders win over narrower
    - Rule 3: Style priority: double > solid > dashed > dotted > ridge > outset > groove > inset > none
    - Rule 4: Cell > row > row-group > column > column-group > table
  - [x] 3.2 Verify all border styles are handled in conflict resolution (currently limited to none/solid/dashed/dotted — add groove/ridge/inset/outset if missing)
  - [x] 3.3 Support `table-layout: fixed` with `border-collapse: collapse` — currently falls back to auto (noted at `mod.rs:32-33`). Implement the fixed+collapsed combination per §17.5.2.1
  - [x] 3.4 Unit tests for border conflict resolution with all styles
  - [x] 3.5 Golden test: collapsed borders with mixed styles (groove, ridge, outset)

- [x] Task 4: Table border style rendering (AC: #1 support)
  - [x] 4.1 Check which border styles are painted in `crates/display_list/src/builder.rs` and `crates/render/` — verify `groove`, `ridge`, `inset`, `outset` are rendered with 3D-effect colors per CSS 2.1 §17.6.2
  - [x] 4.2 If missing: implement `groove` (etched-in), `ridge` (etched-out), `inset` (embedded), `outset` (extruded) using lighter/darker shades of the border color
  - [x] 4.3 Golden test for each 3D border style on table cells

- [x] Task 5: Golden tests and checklist update (AC: #6)
  - [x] 5.1 Add golden tests: caption-side bottom, empty-cells hide, collapsed borders with groove/ridge, fixed+collapsed combination
  - [x] 5.2 Verify all 20 existing table golden tests pass (078-090, 094, 143, 158, 170, 173, 187, 189)
  - [x] 5.3 Update `docs/CSS2.1_Implementation_Checklist.md` — check off: caption-side, empty-cells, remaining border styles in collapsed model
  - [x] 5.4 Run `just ci` and ensure all tests pass

## Dev Notes

### Current Implementation Status

Table layout is **substantially implemented**. What works:
- **`display: table/table-row/table-cell/table-row-group/table-header-group/table-footer-group/table-caption`** — all display types fully mapped and handled
- **`table-layout: auto`** — intrinsic content sizing, column width distribution via `width.rs` (1500+ lines)
- **`table-layout: fixed`** — column widths from `<col>` and first row, fast path
- **`border-collapse: separate`** — cells separated by `border-spacing`, fully working
- **`border-collapse: collapse`** — border conflict resolution in `collapsed.rs` (1100+ lines), half-border calculations
- **`border-spacing`** — single and two-value forms, UA stylesheet default 2px for `<table>`
- **`vertical-align` in cells** — top, middle, bottom, baseline (4 values)
- **Colspan/rowspan** — grid building, width distribution, height distribution all working
- **Anonymous box generation** — CSS 2.1 §17.2.1 anonymous row/cell creation
- **`<col>`/`<colgroup>` width hints** — span attribute, width column hints
- **HTML attribute mapping** — `cellspacing`, `cellpadding`, `align`, `valign`, `colspan`, `rowspan`, `width`
- **Nested tables** — fully supported
- **Caption layout** — positions above table (but always top, no `caption-side` support)

What is **missing or incomplete** (this story's scope):
- **`caption-side` property** — NOT IMPLEMENTED, captions always above
- **`empty-cells` property** — NOT IMPLEMENTED, empty cells always render
- **`table-layout: fixed` + `border-collapse: collapse` combination** — falls back to auto layout
- **Border styles `groove`/`ridge`/`inset`/`outset`** — may not be rendered with 3D effects
- **`visibility: collapse` for table rows/columns** — not implemented (lower priority)
- **Baseline alignment** — box baselines used, not text baselines

### Key Code Locations

| Component | File | Key Functions/Lines |
|---|---|---|
| Table layout entry | `crates/layout/src/engine/table/mod.rs:93-223` | `calculate_table_layout()` — main dispatcher |
| Fixed layout | `crates/layout/src/engine/table/mod.rs:230-299` | `calculate_table_layout_fixed()` |
| Caption layout | `crates/layout/src/engine/table/mod.rs:301-322` | `layout_table_captions()` — **modify for caption-side** |
| Table height calc | `crates/layout/src/engine/table/mod.rs` | `calculate_table_height()` |
| Grid cell struct | `crates/layout/src/engine/table/mod.rs:65-72` | `GridCell { cell_index, row_index, is_origin }` |
| Safety limits | `crates/layout/src/engine/table/mod.rs` | `MAX_TABLE_COLS=1000`, `MAX_TABLE_ROWS=10000` |
| Separate model layout | `crates/layout/src/engine/table/layout.rs` | Cell layout, vertical-align, row heights (596 lines) |
| Column width calc | `crates/layout/src/engine/table/width.rs` | `measure_table_intrinsic_width()`, `calculate_column_widths()` (1500+ lines) |
| Grid building | `crates/layout/src/engine/table/grid.rs` | `build_table_grid()`, `collect_table_row_indices()`, normalization (969 lines) |
| Collapsed borders | `crates/layout/src/engine/table/collapsed.rs` | `calculate_table_layout_collapsed()`, border conflict resolution (1100+ lines) |
| BorderCollapse enum | `crates/style/src/types/text.rs` | `BorderCollapse { Separate, Collapse }` |
| TableLayout enum | `crates/style/src/types/text.rs` | `TableLayout { Auto, Fixed }` |
| ComputedStyles fields | `crates/style/src/types/computed.rs:122-125` | `border_collapse`, `border_spacing`, `table_layout`, `vertical_align` |
| LayoutBox table fields | `crates/layout/src/types.rs:260-263` | `border_collapse`, `border_spacing`, `table_layout`, `vertical_align`, `colspan`, `rowspan` |
| Display enum | `crates/style/src/types/primitives.rs` | `Table`, `TableRow`, `TableCell`, `TableRowGroup`, etc. |
| UA stylesheet | `crates/style/src/ua_stylesheet.rs` | `<table>` defaults: `border-spacing: 2px`, `border-collapse: separate` |
| Style tests | `crates/style/src/tests/table.rs` | 40+ tests covering table property parsing/inheritance (906 lines) |
| Grid tests | `crates/layout/src/engine/table/grid.rs` | 65 tests for grid building and normalization |
| Collapsed tests | `crates/layout/src/engine/table/collapsed.rs` | 30+ tests for border conflict resolution |
| Display list | `crates/display_list/src/builder.rs` | Table cell painting — **modify for empty-cells** |

### Existing Golden Tests (Do NOT Regress — 20 tests)

| Fixture | Coverage |
|---|---|
| `078-table-basic.html` | Basic 2x2 table |
| `079-table-headers.html` | th elements |
| `080-table-sections.html` | thead/tbody/tfoot |
| `081-table-colspan.html` | Column spanning |
| `082-table-rowspan.html` | Row spanning |
| `083-table-mixed-spans.html` | Combined colspan/rowspan |
| `084-table-column-widths.html` | Column width distribution |
| `085-table-cell-padding.html` | Cell padding + spacing |
| `086-table-backgrounds.html` | Cell backgrounds |
| `087-table-width.html` | Table width sizing |
| `088-table-nested.html` | Nested tables |
| `089-table-text-wrap.html` | Text wrapping |
| `090-table-empty-cells.html` | Empty cells (no property test) |
| `094-table-vertical-align.html` | Vertical alignment |
| `143-img-link-in-table.html` | Images/links in cells |
| `158-table-with-caption.html` | Caption element |
| `170-table-implicit-close.html` | Implicit tag closing |
| `173-table-anonymous-row.html` | Anonymous row generation |
| `187-table-anonymous-objects.html` | Anonymous object generation |
| `189-table-col-width.html` | `<col>` width hints |

### Implementation Approach

**Task 1 (caption-side):**
Standard CSS property pipeline: parse → compute → layout. The layout change is in `layout_table_captions()` at `mod.rs:301-322`. Currently captions are always laid out first (before rows). For `caption-side: bottom`:
1. Skip captions in the initial pass
2. After laying out all rows, position captions below the final row
3. Add caption height to total table height

The `caption_side` property inherits (per CSS 2.1), so set on the table element and inherited by the caption. Check if the caption's own `caption-side` should be used (it should — the property applies to `table-caption` display elements).

**Task 2 (empty-cells):**
The `empty-cells` property only affects `border-collapse: separate`. Detection logic: check if a TableCell has zero children, or only whitespace text children. In the display list builder, when painting a cell with `empty-cells: hide`:
- Skip background painting for that cell
- Skip border painting for that cell
- Cell still occupies space (dimensions unchanged) — only visual rendering suppressed

**Task 3 (collapsed border completeness):**
The existing `collapsed.rs` (1100+ lines) implements border conflict resolution. Audit against §17.6.2.1 rules. The main gap is likely border style priority — the style precedence order is: `double > solid > dashed > dotted > ridge > outset > groove > inset > none`. Verify the existing `resolve_border_conflict()` function handles all styles. Also address the fixed+collapsed fallback at `mod.rs:32-33`.

**Task 4 (3D border styles):**
`groove`, `ridge`, `inset`, `outset` borders use two colors derived from the base border color:
- Lighter shade: blend toward white (or use a fixed lighter factor)
- Darker shade: blend toward black
- `groove`: dark-top/left, light-bottom/right (opposite of `ridge`)
- `ridge`: light-top/left, dark-bottom/right (opposite of `groove`)
- `inset`: dark-top/left, light-bottom/right
- `outset`: light-top/left, dark-bottom/right

### Architecture Constraints

- **Layer rule:** Changes span `css` (Layer 1), `style` (Layer 1), `layout` (Layer 1), `display_list` (Layer 1) — all horizontal Layer 1 deps
- **No unsafe:** All affected crates have `unsafe_code = "forbid"`
- **CSS Property Implementation Order:** Parse in `css/` → computed in `style/` → layout effect in `layout/` → paint effect in `display_list/` → golden tests → checklist → `just ci`
- **Safety limits:** Respect `MAX_TABLE_COLS=1000` and `MAX_TABLE_ROWS=10000` in all table code paths

### Previous Story Intelligence

**From Stories 1.1-1.4:**
- Golden test infrastructure: fixtures in `tests/goldens/fixtures/`, expected in `tests/goldens/expected/`
- CSS Property Implementation Order is consistently: parse → style → layout → paint → test → docs
- Checklist update at `docs/CSS2.1_Implementation_Checklist.md` is mandatory
- `just ci` is the single validation gate (~1 minute, run once)
- New enum types go in `crates/style/src/types/text.rs` alongside existing `BorderCollapse`, `TableLayout`

### Testing Strategy

1. **CSS parser tests** for new properties (caption-side, empty-cells) — extend `crates/style/src/tests/table.rs`
2. **Unit tests** for empty cell detection logic
3. **Unit tests** for border conflict resolution with all border styles
4. **Golden tests** — use next available fixture numbers:
   - Caption-side bottom
   - Empty-cells hide with border-spacing
   - Collapsed borders with groove/ridge/outset
   - Fixed + collapsed combination
5. **Regression verification** — all 20 existing table golden tests must pass
6. Run `just ci` at the end

### CSS 2.1 Spec References

- **§17.2** — Table box model: table wrapper box, caption box, table grid box
- **§17.2.1** — Anonymous table objects: missing row/cell generation
- **§17.4** — Table captions: caption-side property
- **§17.5** — Table layout: auto and fixed algorithms
- **§17.5.2** — Table width algorithms
- **§17.5.2.1** — Fixed table layout algorithm
- **§17.5.2.2** — Automatic table layout algorithm
- **§17.6** — Table borders
- **§17.6.1** — Separate borders model: border-spacing, empty-cells
- **§17.6.1.1** — `empty-cells` property definition
- **§17.6.2** — Collapsing border model
- **§17.6.2.1** — Border conflict resolution algorithm

### Project Structure Notes

- New CSS properties (`caption-side`, `empty-cells`) follow standard pipeline: parse → compute → layout/paint
- New enum types added to `crates/style/src/types/text.rs` (existing home for table enums)
- Empty-cells paint suppression goes in `crates/display_list/src/builder.rs`
- All table layout code in `crates/layout/src/engine/table/` — 5 files, well-organized
- New golden test fixtures in `tests/goldens/fixtures/` with next available number

### References

- [Source: crates/layout/src/engine/table/mod.rs] — Table layout orchestrator, caption layout, anonymous box generation
- [Source: crates/layout/src/engine/table/layout.rs] — Separate border model, vertical-align, row heights
- [Source: crates/layout/src/engine/table/width.rs] — Column width calculation (auto and fixed)
- [Source: crates/layout/src/engine/table/grid.rs] — Grid building, row collection
- [Source: crates/layout/src/engine/table/collapsed.rs] — Collapsed border model, conflict resolution
- [Source: crates/style/src/types/text.rs] — BorderCollapse, TableLayout enums
- [Source: crates/style/src/types/computed.rs#122-125] — Table computed style fields
- [Source: crates/layout/src/types.rs] — LayoutBox table fields (colspan, rowspan, etc.)
- [Source: crates/style/src/tests/table.rs] — Table style tests (906 lines)
- [Source: crates/display_list/src/builder.rs] — Table cell painting (modify for empty-cells)
- [Source: docs/CSS2.1_Implementation_Checklist.md#Phase-13] — Table layout checklist

## Dev Agent Record

### Agent Model Used

Claude Opus 4.6 (1M context)

### Debug Log References

- WPT regression in `wpt-css-css-tables-caption-relative-positioning` was caused by not handling bottom captions in the empty-table code path. Fixed by adding `layout_table_bottom_captions()` calls to all three empty-table branches (auto, fixed, collapsed).

### Completion Notes List

- **Task 1 (caption-side):** Full CSS property pipeline implemented: parse → compute → inherit → layout. `layout_table_captions()` now only handles `caption-side: top`, and a new `layout_table_bottom_captions()` handles `caption-side: bottom` after rows are laid out. All three table layout paths (auto, fixed, collapsed) updated for both non-empty and empty tables.
- **Task 2 (empty-cells):** Full CSS property pipeline. In display list builder, `is_cell_empty()` checks for no children and no non-whitespace inline content. Background and border painting suppressed for empty cells when `empty-cells: hide` in `border-collapse: separate` model only.
- **Task 3 (collapsed border completeness):** Audit confirmed `resolve_border_conflict()` already handles all 10 border styles with correct priority per §17.6.2.1. `table-layout: fixed` + `border-collapse: collapse` combination implemented by using `calculate_fixed_column_widths()` when `table-layout: fixed` with explicit width.
- **Task 4 (3D border styles):** Already fully implemented in render crate with `lighten()`/`darken()` color adjustments for groove, ridge, inset, outset.
- **Task 5 (golden tests + checklist):** 4 new golden tests added (231-234). All 227 golden tests pass. CSS2.1 checklist updated. `just ci` passes.

### Change Log

- 2026-03-13: Implement caption-side, empty-cells, fixed+collapsed layout, golden tests, checklist update (Story 1.5)
- 2026-03-13: Code review fixes — `resolve_border_conflict()` hidden-beats-wider bug (§17.6.2.1 Rule 1), `is_cell_empty()` whitespace-only anonymous children, 5 new border conflict tests

### File List

- `crates/style/src/types/text.rs` — Added `CaptionSide` and `EmptyCells` enums
- `crates/style/src/types/mod.rs` — Re-export new types
- `crates/style/src/types/computed.rs` — Added fields, cascade resolution, inheritance
- `crates/style/src/lib.rs` — Re-export new types
- `crates/style/src/tests/table.rs` — 8 new unit tests for caption-side and empty-cells
- `crates/css/src/types.rs` — Added `PropertyId::CaptionSide`, `PropertyId::EmptyCells`, is_inherited
- `crates/layout/src/types.rs` — Added fields to `LayoutBox`
- `crates/layout/src/engine/box_tree.rs` — Wire computed styles to layout box
- `crates/layout/src/engine/table/mod.rs` — Split caption layout into top/bottom, added `layout_table_bottom_captions()`
- `crates/layout/src/engine/table/collapsed.rs` — Fixed+collapsed column width dispatch, bottom caption support
- `crates/display_list/src/builder.rs` — `is_cell_empty()` helper, empty-cells paint suppression
- `tests/goldens.rs` — 4 new golden test registrations
- `tests/goldens/fixtures/231-table-caption-side-bottom.html` — New fixture
- `tests/goldens/fixtures/232-table-empty-cells-hide.html` — New fixture
- `tests/goldens/fixtures/233-table-collapsed-groove-ridge.html` — New fixture
- `tests/goldens/fixtures/234-table-fixed-collapsed.html` — New fixture
- `tests/goldens/expected/231-table-caption-side-bottom.{layout,dl}.txt` — Expected output
- `tests/goldens/expected/232-table-empty-cells-hide.{layout,dl}.txt` — Expected output
- `tests/goldens/expected/233-table-collapsed-groove-ridge.{layout,dl}.txt` — Expected output
- `tests/goldens/expected/234-table-fixed-collapsed.{layout,dl}.txt` — Expected output
- `docs/CSS2.1_Implementation_Checklist.md` — Updated Phase 13 checkboxes