moldybits/rust_browser
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
fix/es-modules-fifth-review-3-4
rust_browser/_bmad-output/implementation-artifacts/2-9-base-url-resolution-and-image-rendering-completeness.md
Zachary D. Rowitsch 70ad1244d8 Implement base URL resolution and image rendering completeness with code review fixes (§4.2.3) 
Add <base href> support with resolve_base_url() wired before all resource loading,
document.baseURI JS API (falling back to "about:blank" per spec), image format
verification golden tests (PNG/JPEG/GIF/WebP/SVG for both <img> and CSS
background-image), and aspect ratio preservation tests. Code review fixes:
baseURI spec compliance, restyle path no longer overwrites <base href> URL,
added CSS background-image format tests and base URL integration tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-15 01:36:35 -04:00

302 lines
20 KiB
Markdown
Raw Permalink Blame History

# Story 2.9: Base URL Resolution & Image Rendering Completeness
Status: done
<!-- Note: Validation is optional. Run validate-create-story for quality check before dev-story. -->
## Story
As a web user,
I want all relative URLs to resolve correctly and all image formats to render,
So that pages with relative links and various image types display properly.
## Acceptance Criteria
1. **`<base href>` sets the document base URL:** A document with `<base href="https://example.com/path/">` resolves all relative URLs (in links, images, scripts, stylesheets) against that base URL instead of the page URL. (WHATWG HTML §4.2.3)
2. **First `<base href>` wins:** When multiple `<base>` elements exist, only the first one with an `href` attribute is used.
3. **All image formats render correctly:** Images in PNG, JPEG, GIF, WebP, and SVG formats referenced as `<img>` or CSS `background-image` decode and render correctly at specified dimensions.
4. **Image aspect ratio preservation:** An `<img>` with `width`/`height` attributes and/or CSS sizing scales to the specified dimensions. When only one dimension is specified and the other is `auto`, the intrinsic aspect ratio is preserved.
5. **Golden tests cover base URL resolution and each image format**, checklists are updated, and `just ci` passes.
## Tasks / Subtasks
- [x] Task 1: Implement `<base href>` element support (AC: #1, #2)
- [x] 1.1 After HTML parsing, scan the Document for the first `<base>` element with an `href` attribute:
- Walk `<head>` children for `<base>` elements
- Take the `href` value from the first `<base>` that has one
- Parse it as an absolute URL, or resolve against the page URL if relative
- Call `document.set_base_url(Some(resolved_url))`
- [x] 1.2 The best place for this logic is in the pipeline integration, after parsing and before any resource loading. Add a `resolve_base_url()` function in `crates/app_browser/src/pipeline/` (new file or in existing module):
```rust
pub fn resolve_base_url(doc: &Document, page_url: &BrowserUrl) -> BrowserUrl {
// Find first <base href="..."> in <head>
// Return resolved URL or page_url as fallback
}
```
- [x] 1.3 In `event_handler.rs`, after parsing HTML and before script extraction / resource loading:
- Call `resolve_base_url(&document, &state.current_url)`
- Use the resolved base URL for ALL subsequent resource loading (scripts, stylesheets, images)
- Pass base URL to `fetch_script_sources()`, `fetch_external_stylesheets()`, `fetch_images()`, `fetch_background_images()`
- [x] 1.4 Verify that existing `BrowserUrl::join()` in `crates/shared/src/url.rs` correctly handles joining against a base URL (it should — it wraps `url::Url::join()`)
- [x] 1.5 Edge cases to handle:
- `<base href="">` — empty href, use page URL
- `<base>` without href — skip, look for next `<base>`
- `<base href>` with invalid URL — fall back to page URL
- `<base>` in `<body>` — per spec, only `<base>` in `<head>` is effective (but browsers are lenient; match browser behavior)
- Multiple `<base>` elements — only first with href wins
- [x] 1.6 Unit tests for `resolve_base_url()`: all edge cases above
- [x] Task 2: Expose `document.baseURI` to JavaScript (AC: #1)
- [x] 2.1 In `DomHost::get_property()` for `"Document"` type, add `"baseURI"` property:
- Return `JsValue::String(doc.base_url().map(|u| u.to_string()).unwrap_or_default())`
- [x] 2.2 If `base_url` is None on Document, return the page URL (baseURI always has a value per spec)
- [x] 2.3 Unit test: document.baseURI returns the effective base URL
- [x] Task 3: Verify and fix image format rendering completeness (AC: #3)
- [x] 3.1 Create golden test fixtures for each image format:
- `<img src="test.png">` — PNG with transparency
- `<img src="test.jpg">` — JPEG
- `<img src="test.gif">` — GIF (static frame)
- `<img src="test.webp">` — WebP
- `<img src="test.svg">` — SVG
- [x] 3.2 Create golden test fixtures for CSS background-image with each format:
- `background-image: url(test.png)` etc.
- [x] 3.3 Verify each format decodes and renders without errors
- [x] 3.4 If any format fails, fix the decoding path in `crates/image/src/lib.rs`
- [x] 3.5 Note: All five formats (PNG, JPEG, GIF, WebP, SVG) are already supported by the `image` crate (0.25) and `resvg` (0.47). This task is primarily **verification via golden tests**, not new implementation.
- [x] Task 4: Verify and fix image aspect ratio preservation (AC: #4)
- [x] 4.1 Review existing aspect ratio logic in:
- `crates/layout/src/engine/block/width.rs` (lines 24-68) — proportional width from height
- `crates/layout/src/engine/block/layout.rs` (lines 278-300) — proportional height from width
- [x] 4.2 Create golden tests for image sizing scenarios:
- `<img src="..." width="200" height="100">` — explicit both dimensions
- `<img src="..." width="200">` — width only, height auto → preserve aspect ratio
- `<img src="..." height="100">` — height only, width auto → preserve aspect ratio
- `<img src="..." style="width:200px">` — CSS width, height auto
- `<img src="..." style="max-width:100px">` — constrained by max-width
- `<img src="...">` — no dimensions, use intrinsic size
- [x] 4.3 Verify that CSS `width`/`height` override HTML attributes when both are present
- [x] 4.4 Verify `min-width`/`max-width`/`min-height`/`max-height` constraints work with images
- [x] 4.5 Fix any issues found in the aspect ratio calculation
- [x] Task 5: Ensure base URL flows through all resource loading paths (AC: #1)
- [x] 5.1 Audit all places where URLs are resolved to ensure they use the effective base URL:
- `fetch_script_sources()` in `crates/app_browser/src/pipeline/scripts.rs` — uses `base_url` parameter
- `fetch_external_stylesheets()` in `crates/app_browser/src/pipeline/stylesheets.rs` — uses `base_url`
- `@import` resolution in `resolve_imports_recursive()` — uses stylesheet URL, which should already be absolute
- `fetch_images()` in `crates/app_browser/src/pipeline/images.rs` — uses `base_url` parameter
- `fetch_background_images()` — uses `base_url` parameter
- Link href resolution for navigation — uses base_url
- [x] 5.2 If any path is using `state.current_url` directly instead of the resolved base URL, fix it to use the base URL
- [x] 5.3 Integration test: `<base href>` with a relative `<img src>` resolves correctly
- [x] 5.4 Integration test: `<base href>` with a relative `<link href>` for stylesheet resolves correctly
- [x] Task 6: Tests and documentation (AC: #5)
- [x] 6.1 Golden test: base URL resolution with images
- HTML with `<base href="...">` and `<img src="relative-path.png">`
- Verify image loads from the base-URL-resolved path
- [x] 6.2 Golden test: each image format renders at correct size
- [x] 6.3 Golden test: aspect ratio preservation with various dimension combinations
- [x] 6.4 Integration test: `document.baseURI` returns correct value with and without `<base>` element
- [x] 6.5 Integration test: multiple `<base>` elements — first href wins
- [x] 6.6 Update `docs/HTML5_Implementation_Checklist.md`:
- Check off `<base href>` under Phase 3 Base URL resolution
- Check off `document.baseURI`
- Verify `img` is checked (already is)
- [x] 6.7 Run `just ci` and ensure all tests pass
## Dev Notes
### Current URL Resolution Architecture
All resource URLs are resolved using `BrowserUrl::join(&relative_url)` from `crates/shared/src/url.rs`. The base URL is passed as a parameter through the pipeline:
```
event_handler.rs
→ fetch_script_sources(sources, &base_url, network)
→ fetch_external_stylesheets(doc, &base_url, network)
→ fetch_images(doc, &base_url, network, decoder)
→ fetch_background_images(styles, &base_url, network, decoder)
```
Currently, `base_url` is always `state.current_url` (the page load URL). The fix is straightforward: resolve the effective base URL from `<base href>` BEFORE passing it to these functions.
### What Already Works
- **Image format decoding:** PNG, JPEG, GIF, WebP via `image` crate; SVG via `resvg`/`usvg`
- **Image intrinsic sizing:** `LayoutBox::intrinsic_width/height` set from decoded image dimensions
- **Aspect ratio preservation:** Width/height proportional scaling in `block/width.rs` and `block/layout.rs`
- **Background image URL resolution:** Via `fetch_background_images()` with base_url parameter
- **CSS width/height on images:** Images coerced to `InlineBlock`, CSS sizing applied
### What NOT to Implement
- **Do NOT implement `<base target>`** — only `<base href>` is in scope
- **Do NOT implement `srcset` / `<picture>` responsive images** — future work
- **Do NOT implement animated GIF support** — static first frame only is fine
- **Do NOT implement image lazy loading (`loading="lazy"`)** — future optimization
- **Do NOT implement CORS for images** — future security feature
- **Do NOT implement `<img>` error/load events** — related to Document Lifecycle
- **Do NOT modify the image decoding pipeline in `crates/image/`** unless a format actually fails to render
### Architecture Constraints
- **Layer 0 (`shared`):** BrowserUrl::join() — no changes expected
- **Layer 1 (`dom`):** Document::base_url already exists as a field — just ensure it's set from `<base href>`
- **Layer 1 (`web_api`):** Expose document.baseURI as JS property
- **Layer 3 (`app_browser`):** Add resolve_base_url() and wire into pipeline
- **No unsafe** — enforced by CI
### Key Design: Base URL Resolution Timing
The base URL must be resolved AFTER HTML parsing but BEFORE any resource loading:
```
1. Parse HTML → Document
2. Resolve base URL from <base href> ← NEW
3. Extract and fetch scripts (using base URL)
4. Fetch stylesheets (using base URL)
5. Execute scripts
6. Fetch images (using base URL)
7. Render
```
### Key Files to Modify
| File | Change |
|------|--------|
| `crates/app_browser/src/pipeline/` | New `resolve_base_url()` function |
| `crates/app_browser/src/event_handler.rs` | Wire base URL resolution before resource loading |
| `crates/web_api/src/dom_host/host_environment.rs` | Add `document.baseURI` JS property |
| `docs/HTML5_Implementation_Checklist.md` | Check off base URL items |
### Key Files to Read (Reference)
| File | Why |
|------|-----|
| `crates/shared/src/url.rs` | BrowserUrl::join() implementation |
| `crates/dom/src/document.rs` | Document::base_url field, set_base_url() |
| `crates/app_browser/src/pipeline/images.rs` | fetch_images(), fetch_background_images() — URL resolution pattern |
| `crates/app_browser/src/pipeline/stylesheets.rs` | fetch_external_stylesheets() — URL resolution |
| `crates/app_browser/src/pipeline/scripts.rs` | fetch_script_sources() — URL resolution |
| `crates/app_browser/src/event_handler.rs` | Pipeline integration (lines 143-200) |
| `crates/image/src/lib.rs` | ImagePipeline: format detection and decoding |
| `crates/layout/src/engine/block/width.rs` | Image width proportional scaling (lines 24-68) |
| `crates/layout/src/engine/block/layout.rs` | Image height proportional scaling (lines 278-300) |
| `crates/layout/src/engine/box_tree.rs` | Image intrinsic dimension setup (lines 98-115) |
| `crates/layout/src/tests/image_sizing_tests.rs` | Existing image sizing tests |
### Previous Story Intelligence
**From Story 2.8 (iframe Support):**
- Pipeline orchestration pattern in event_handler.rs is well-understood
- Image-like replaced element pattern applies here too (iframes follow same sizing model)
- Resource loading follows the same base_url.join() pattern throughout
**From Epic 1 (CSS stories):**
- Background-image URL resolution was implemented in Story 1.7 (Backgrounds)
- Golden tests are the standard for visual verification
### Testing Strategy
- **Golden tests** for image format rendering — create small test images in each format
- **Golden tests** for aspect ratio with various dimension combinations
- **Integration tests** for base URL resolution with different resource types
- **Unit tests** for `resolve_base_url()` edge cases
- **Note:** Golden tests for base URL may require data: URIs or local file references since the test infrastructure may not support external URLs. Consider using `<base href="data:...">` or testing with relative paths within the golden test fixture directory.
### References
- [WHATWG HTML §4.2.3 — The base element](https://html.spec.whatwg.org/multipage/semantics.html#the-base-element)
- [WHATWG HTML §3.1 — Document base URL](https://html.spec.whatwg.org/multipage/urls-and-fetching.html#document-base-url)
- [Source: crates/shared/src/url.rs] — BrowserUrl with join()
- [Source: crates/dom/src/document.rs] — Document::base_url field
- [Source: crates/app_browser/src/pipeline/images.rs] — fetch_images(), fetch_background_images()
- [Source: crates/app_browser/src/pipeline/stylesheets.rs] — stylesheet URL resolution
- [Source: crates/app_browser/src/pipeline/scripts.rs] — script URL resolution
- [Source: crates/image/src/lib.rs] — ImagePipeline with format detection
- [Source: crates/layout/src/engine/block/width.rs] — Proportional image width (lines 24-68)
- [Source: crates/layout/src/engine/block/layout.rs] — Proportional image height (lines 278-300)
- [Source: docs/HTML5_Implementation_Checklist.md] — Phase 3: Base URL resolution, §6.6 img
## Dev Agent Record
### Agent Model Used
Claude Opus 4.6 (1M context)
### Debug Log References
- All 8 `resolve_base_url()` unit tests pass
- All 11 new golden tests pass (284-294)
- JS262 baseURI DOM test passes
- `just ci` passes with 0 failures
### Completion Notes List
- **Task 1:** Created `resolve_base_url()` in `crates/app_browser/src/pipeline/base_url.rs` with 8 unit tests covering all edge cases (no base, absolute href, relative href, first-wins, skip without href, skip empty href, skip whitespace href, multiple elements). Wired into `event_handler.rs` Phase 1b before all resource loading. Updated link click handler to use document base URL.
- **Task 2:** Added `document.baseURI` property to `DomHost::get_property()` for Document type. Returns base URL string, falling back to "about:blank" per spec when no page URL is set. JS262 test added (`dom-baseuri.js`).
- **Task 3:** Created test images in PNG, JPEG, GIF, WebP, SVG formats (20x10 red rectangles). Golden tests 284-288 verify each format renders correctly at intrinsic dimensions. All formats decode and render without errors — no fixes needed.
- **Task 4:** Golden tests 289-294 verify aspect ratio preservation: width-only (100px→50px height), height-only (50px→100px width), both attributes, CSS width, max-width constraint, intrinsic size. All tests pass — existing proportional scaling in `block/width.rs` and `block/layout.rs` works correctly.
- **Task 5:** Audited all resource loading paths. `effective_base_url` now flows through: `fetch_and_schedule_scripts`, `run_from_document` (→ `fetch_stylesheets`, `fetch_images`, `fetch_background_images`, `load_font_faces`, `fetch_and_render_iframes`). Link click handler updated to use `document.base_url()` instead of `state.current_url`.
- **Task 6:** 16 golden tests (11 original + 5 CSS background-image format tests 295-299), 11 unit tests (8 original + 3 integration-style base URL resolution tests), 1 JS262 test added. HTML5 checklist updated. `just ci` passes cleanly.
### Change Log
- 2026-03-15: Story 2.9 implementation complete — base URL resolution from `<base href>`, `document.baseURI` JS API, image format verification via golden tests, aspect ratio preservation verification.
- 2026-03-15: Code review fixes — (H1) `document.baseURI` now returns "about:blank" instead of "" when no page URL set (spec compliance). (H2) Fixed restyle middle-path overwriting `<base href>` resolved URL with page URL. (M1) Added 5 CSS background-image format golden tests (295-299). (M2) Added 3 integration-style tests for base URL resource resolution.
### File List
New files:
- `crates/app_browser/src/pipeline/base_url.rs` — `resolve_base_url()` function with unit tests
- `tests/goldens/fixtures/284-img-png-format.html` — PNG format golden test
- `tests/goldens/fixtures/285-img-jpeg-format.html` — JPEG format golden test
- `tests/goldens/fixtures/286-img-gif-format.html` — GIF format golden test
- `tests/goldens/fixtures/287-img-webp-format.html` — WebP format golden test
- `tests/goldens/fixtures/288-img-svg-format.html` — SVG format golden test
- `tests/goldens/fixtures/289-img-aspect-ratio-width-only.html` — Aspect ratio width-only test
- `tests/goldens/fixtures/290-img-aspect-ratio-height-only.html` — Aspect ratio height-only test
- `tests/goldens/fixtures/291-img-aspect-ratio-both-attrs.html` — Aspect ratio both attrs test
- `tests/goldens/fixtures/292-img-aspect-ratio-css-width.html` — Aspect ratio CSS width test
- `tests/goldens/fixtures/293-img-aspect-ratio-max-width.html` — Aspect ratio max-width test
- `tests/goldens/fixtures/294-img-intrinsic-size.html` — Intrinsic size test
- `tests/goldens/expected/284-img-png-format.{layout,dl}.txt` — Expected outputs
- `tests/goldens/expected/285-img-jpeg-format.{layout,dl}.txt` — Expected outputs
- `tests/goldens/expected/286-img-gif-format.{layout,dl}.txt` — Expected outputs
- `tests/goldens/expected/287-img-webp-format.{layout,dl}.txt` — Expected outputs
- `tests/goldens/expected/288-img-svg-format.{layout,dl}.txt` — Expected outputs
- `tests/goldens/expected/289-img-aspect-ratio-width-only.{layout,dl}.txt` — Expected outputs
- `tests/goldens/expected/290-img-aspect-ratio-height-only.{layout,dl}.txt` — Expected outputs
- `tests/goldens/expected/291-img-aspect-ratio-both-attrs.{layout,dl}.txt` — Expected outputs
- `tests/goldens/expected/292-img-aspect-ratio-css-width.{layout,dl}.txt` — Expected outputs
- `tests/goldens/expected/293-img-aspect-ratio-max-width.{layout,dl}.txt` — Expected outputs
- `tests/goldens/expected/294-img-intrinsic-size.{layout,dl}.txt` — Expected outputs
- `tests/goldens/fixtures/images/test_20x10.{png,jpg,gif,webp,svg}` — Test images (20x10 red)
- `tests/external/js262/fixtures/dom-baseuri.js` — JS262 baseURI test
- `tests/external/js262/expected/dom-baseuri.txt` — Expected output
- `tests/goldens/fixtures/295-bg-img-png-format.html` — CSS background-image PNG test
- `tests/goldens/fixtures/296-bg-img-jpeg-format.html` — CSS background-image JPEG test
- `tests/goldens/fixtures/297-bg-img-gif-format.html` — CSS background-image GIF test
- `tests/goldens/fixtures/298-bg-img-webp-format.html` — CSS background-image WebP test
- `tests/goldens/fixtures/299-bg-img-svg-format.html` — CSS background-image SVG test
- `tests/goldens/expected/295-bg-img-png-format.{layout,dl}.txt` — Expected outputs
- `tests/goldens/expected/296-bg-img-jpeg-format.{layout,dl}.txt` — Expected outputs
- `tests/goldens/expected/297-bg-img-gif-format.{layout,dl}.txt` — Expected outputs
- `tests/goldens/expected/298-bg-img-webp-format.{layout,dl}.txt` — Expected outputs
- `tests/goldens/expected/299-bg-img-svg-format.{layout,dl}.txt` — Expected outputs
Modified files:
- `crates/app_browser/src/pipeline/mod.rs` — Added `base_url` module and re-export
- `crates/app_browser/src/event_handler.rs` — Wired `resolve_base_url()` before resource loading; updated link click to use document base URL
- `crates/web_api/src/dom_host/host_environment.rs` — Added `document.baseURI` property
- `tests/goldens.rs` — Added 16 new golden test functions (11 img + 5 CSS background-image)
- `tests/external/js262/js262_manifest.toml` — Added baseURI test case
- `docs/HTML5_Implementation_Checklist.md` — Checked off base URL items
- `_bmad-output/implementation-artifacts/sprint-status.yaml` — Status updated
Reference in New Issue View Git Blame Copy Permalink