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-7-document-lifecycle.md
Zachary D. Rowitsch 626a1c517c Implement document lifecycle events with code review fixes (§8.4, §7.1) 
Add DOMContentLoaded, load, and readystatechange events with correct
readyState transitions (loading→interactive→complete). Includes Window
as a first-class event target, body onload spec quirk, and idempotency
guards to prevent double-firing. Code review hardened the API surface
by enforcing forward-only state transitions, eliminating a redundant
wrapper function, and requesting a redraw after load handler DOM mutations.

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

218 lines
14 KiB
Markdown
Raw Permalink Blame History

# Story 2.7: Document Lifecycle
Status: done
<!-- Note: Validation is optional. Run validate-create-story for quality check before dev-story. -->
## Story
As a web developer using JavaScript,
I want document lifecycle events to fire at the correct times,
So that scripts can initialize at the right point in page loading.
## Acceptance Criteria
1. **DOMContentLoaded fires after parsing:** A page with a `DOMContentLoaded` event listener on `document` fires the event after the HTML is fully parsed (but before external resources like images are fully loaded). (WHATWG HTML §8.4)
2. **Window load event fires after all resources:** A page with a `load` event listener on `window` fires the event after all resources (images, stylesheets, scripts) have finished loading.
3. **document.readyState transitions correctly:** JavaScript reading `document.readyState` gets `"loading"` during parsing, `"interactive"` after parsing completes (before load), and `"complete"` after all resources load.
4. **readystatechange event fires on transitions:** A `readystatechange` event listener on `document` receives an event each time `document.readyState` changes (loading→interactive and interactive→complete).
5. **Integration tests verify event timing and readyState transitions**, checklist is updated, and `just ci` passes.
## Tasks / Subtasks
- [x] Task 1: Add readyState property to Document (AC: #3)
- [x] 1.1 Add a `ready_state` field to `Document` in `crates/dom/src/document.rs`
- [x] 1.2 Initialize `ready_state: ReadyState::Loading` in `Document::new()`
- [x] 1.3 Add `pub fn ready_state(&self) -> ReadyState` getter and `pub fn set_ready_state(&mut self, state: ReadyState)` setter
- [x] 1.4 Expose `document.readyState` in JavaScript via `DomHost::get_property()` for `"Document"` type
- [x] 1.5 Unit tests: readyState default is "loading", setter works, JS property access returns correct string
- [x] Task 2: Add dispatch_document_event to WebApiFacade (AC: #1, #2, #4)
- [x] 2.1 Add `dispatch_lifecycle_event()` method to `WebApiFacade` — fires listeners directly on target without DOM propagation
- [x] 2.2 DOMContentLoaded fires on `EventTargetId::Document` (bubbles: true, cancelable: false)
- [x] 2.3 Load fires on `EventTargetId::Window` (bubbles: false, cancelable: false)
- [x] 2.4 Readystatechange fires on `EventTargetId::Document` on each transition
- [x] 2.5 Added `invoke_listeners_simple()` public wrapper in `event_dispatch.rs` for direct-target events
- [x] 2.6 Unit tests: verify DOMContentLoaded fires on document, load fires on window, readystatechange fires on document
- [x] Task 3: Wire lifecycle events into the page-load pipeline (AC: #1, #2, #3, #4)
- [x] 3.1 Updated pipeline in `event_handler.rs` with Phase 4 (interactive) and Phase 6 (complete) lifecycle events
- [x] 3.2 Used swap_document pattern for load event after rendering
- [x] 3.3 Added `fire_dom_content_loaded()` and `fire_load_event()` to `BrowserRuntime`
- [x] 3.4 `fire_dom_content_loaded()` called after scripts + inline handlers, before rendering
- [x] 3.5 `fire_load_event()` called after rendering completes
- [x] Task 4: Handle Window as an event target (AC: #2)
- [x] 4.1 Added `Window` variant to `EventTargetId` enum
- [x] 4.2 `addEventListener`/`removeEventListener` on Window works via `event_target_id_for()` mapping
- [x] 4.3 `<body onload="...">` now registers on `EventTargetId::Window` instead of body node
- [x] 4.4 Unit tests: addEventListener on window for "load" event registers correctly
- [x] Task 5: Tests and documentation (AC: #5)
- [x] 5.1 Integration test: DOMContentLoaded fires after scripts (verifies state=["script:loading","dcl:interactive"])
- [x] 5.2 Integration test: load fires after DOMContentLoaded (verifies order and readyState values)
- [x] 5.3 Integration test: readystatechange fires on each transition (verifies ["interactive","complete"])
- [x] 5.4 Integration test: inline `<body onload="...">` fires as window load event
- [x] 5.5 Updated `docs/HTML5_Implementation_Checklist.md` — Phase 4 items checked off
- [x] 5.6 Updated `docs/DOM_Implementation_Checklist.md` — readyState, DOMContentLoaded, readystatechange checked off
- [x] 5.7 `just ci` passes with all tests green
## Dev Notes
### Event System Infrastructure (What Exists)
The event system has solid foundations:
- **`DomEvent`** struct in `crates/web_api/src/event.rs` — event_type, target, phase, bubbles, cancelable, defaultPrevented, propagationStopped
- **`EventTargetId`** enum in `crates/web_api/src/event_target.rs` — currently `Node(NodeId)` and `Document` variants
- **`dispatch_event()`** in `crates/web_api/src/event_dispatch.rs` — full propagation (target → bubble) through DOM tree. Takes a `NodeId` target and builds path to Document
- **`EventListenerRegistry`** — addEventListener/removeEventListener with deduplication and snapshot-based dispatch
- **`install_inline_event_handlers()`** — scans `on*` attributes and registers listeners
- **`dispatch_click()`** on `WebApiFacade` — uses `dispatch_event()` for node-targeted events
### Key Design Decision: Direct-Target Events vs DOM Propagation
DOMContentLoaded, load, and readystatechange are NOT standard DOM propagation events. They do NOT bubble through DOM element nodes:
- **DOMContentLoaded** fires on `document` (bubbles: true, but only within document, not through element tree)
- **load** fires on `window` (bubbles: false)
- **readystatechange** fires on `document` (bubbles: false)
The existing `dispatch_event()` function expects a `NodeId` target and builds a propagation path through DOM elements. For lifecycle events, you need a **simpler dispatch** that fires listeners directly on a specific `EventTargetId` without building a DOM propagation path.
**Recommended approach:** Add a `dispatch_simple_event()` function that:
1. Takes `EventTargetId` + event type + bubbles/cancelable flags
2. Gets listeners from the registry for that target
3. Invokes them directly (no propagation path needed)
4. Drains microtask queue after dispatch
### What NOT to Change
- **Do NOT modify the existing `dispatch_event()` propagation logic** — it works correctly for node-targeted events like click
- **Do NOT add resource loading tracking** — for now, `load` fires after rendering is complete. True resource-complete tracking (waiting for all images/stylesheets) is a future optimization. The key behavior is that load fires AFTER DOMContentLoaded.
- **Do NOT implement `beforeunload` or `unload` events** — those are navigation events, not document lifecycle
- **Do NOT implement `document.DOMContentLoaded` as a property** — it's an event, not a property
- **Do NOT modify the HTML parser or tree builder**
### Architecture Constraints
- **Layer 0 (`dom`):** ReadyState type and Document field addition
- **Layer 1 (`web_api`):** Event dispatch for lifecycle events, JS property binding for readyState
- **Layer 2 (`browser_runtime`):** Public API methods for lifecycle events
- **Layer 3 (`app_browser`):** Wire lifecycle events into page-load pipeline
- **No unsafe** — enforced by CI
- **No upward dependencies** — `dom` does not import `web_api`; readyState is a DOM concept, not a web_api concept
### `<body onload>` Spec Quirk
Per HTML spec, the `onload` attribute on `<body>` is a **window event handler**, not a body event handler. The current `install_inline_event_handlers()` registers it on `EventTargetId::Node(body_id)`, which is incorrect. It should register `load` from `<body onload="...">` on `EventTargetId::Window` (or the equivalent). This is a known browser quirk that's important for compatibility.
Similarly, `onerror`, `onfocus`, `onblur`, `onscroll`, and `onresize` on `<body>` are also window event handlers per spec. For this story, only handle `onload` on `<body>` — the others can be addressed later.
### Key Files to Modify
| File | Change |
|------|--------|
| `crates/dom/src/document.rs` | Add `ReadyState` enum, `ready_state` field, getter/setter |
| `crates/web_api/src/event_target.rs` | Add `Window` variant to `EventTargetId` if needed |
| `crates/web_api/src/lib.rs` | Add `dispatch_document_event()` / `dispatch_window_event()`, expose readyState JS property |
| `crates/web_api/src/dom_host/host_environment.rs` | Add `readyState` property for Document type |
| `crates/browser_runtime/src/lib.rs` | Add `fire_dom_content_loaded()`, `fire_load_event()` |
| `crates/app_browser/src/event_handler.rs` | Wire lifecycle events into pipeline (lines 143-178) |
| `docs/HTML5_Implementation_Checklist.md` | Check off Phase 4 items |
| `docs/DOM_Implementation_Checklist.md` | Check off readyState, DOMContentLoaded, readystatechange |
### Key Files to Read (Reference)
| File | Why |
|------|-----|
| `crates/web_api/src/event.rs` | DomEvent struct, EventPhase |
| `crates/web_api/src/event_dispatch.rs` | Existing dispatch_event() — reference for how listeners are invoked |
| `crates/web_api/src/event_target.rs` | EventTargetId, EventListenerRegistry, EventListener |
| `crates/web_api/src/lib.rs` | WebApiFacade — dispatch_click pattern, install_inline_event_handlers |
| `crates/browser_runtime/src/lib.rs` | BrowserRuntime public API (dispatch_click pattern to follow) |
| `crates/app_browser/src/event_handler.rs` | Page-load pipeline (lines 143-178) |
### Previous Story Intelligence
**From Story 2.6 (Script Loading):**
- The page-load pipeline in `event_handler.rs` (lines 143-178) is the critical integration point
- Script execution happens after parsing, before rendering — lifecycle events slot between these phases
- The `runtime.prepare_for_navigation()` → `set_document()` → execute scripts → `take_document()` pattern must be preserved
**From Story 2.5 (DOM Query APIs):**
- JS property binding follows established pattern in `host_environment.rs` `get_property()` dispatch on `obj_type`
- Adding `"readyState"` follows the same pattern as other Document properties
**From Epic 1:**
- Always update both checklists (HTML5 and DOM) when applicable
- Code review catches edge cases — be thorough with event ordering
### Testing Strategy
- **Unit tests** in `crates/dom/src/document.rs` for ReadyState type and Document property
- **Unit tests** in `crates/web_api/src/lib.rs` for lifecycle event dispatch
- **Integration tests** using JS scripts that register event listeners and record execution order via array pushes
- **Key test pattern:** Scripts register listeners that push strings to a global array. After page load, verify the array contains the expected sequence.
### Spec References
- [WHATWG HTML §8.4 — Document lifecycle](https://html.spec.whatwg.org/multipage/parsing.html#the-end)
- [WHATWG HTML §7.1 — Document readiness states](https://html.spec.whatwg.org/multipage/dom.html#current-document-readiness)
- [DOM §2.7 — Event dispatch](https://dom.spec.whatwg.org/#dispatching-events)
### References
- [Source: crates/web_api/src/event.rs] — DomEvent struct, EventPhase enum
- [Source: crates/web_api/src/event_dispatch.rs] — dispatch_event(), build_propagation_path()
- [Source: crates/web_api/src/event_target.rs] — EventTargetId enum, EventListenerRegistry
- [Source: crates/web_api/src/lib.rs] — WebApiFacade (dispatch_click, install_inline_event_handlers)
- [Source: crates/browser_runtime/src/lib.rs] — BrowserRuntime public API
- [Source: crates/app_browser/src/event_handler.rs] — Page-load pipeline (lines 143-178)
- [Source: crates/dom/src/document.rs] — Document type
- [Source: docs/HTML5_Implementation_Checklist.md] — Phase 4: Core Document Lifecycle
- [Source: docs/DOM_Implementation_Checklist.md] — §3.4 Document Lifecycle Events
## Dev Agent Record
### Agent Model Used
Claude Opus 4.6 (1M context)
### Debug Log References
No errors or blocking issues encountered.
### Completion Notes List
- Implemented `ReadyState` enum (Loading/Interactive/Complete) in the `dom` crate with `as_str()` method
- Added `Window` variant to `EventTargetId` enum, enabling window as a first-class event target
- Created `dispatch_lifecycle_event()` on `WebApiFacade` for direct-target events (no DOM propagation)
- Added `invoke_listeners_simple()` public wrapper in `event_dispatch.rs`
- Wired `fire_dom_content_loaded()` and `fire_load_event()` through `BrowserRuntime` into the page-load pipeline
- Fixed `<body onload>` spec quirk: now registers on `EventTargetId::Window` per HTML spec
- Exposed `document.readyState` as a readable JS property
- All acceptance criteria satisfied with 5 unit tests + 4 integration tests + 8 lifecycle unit tests
### File List
- `crates/dom/src/document.rs` — Added `ReadyState` enum, `ready_state` field, getter/setter
- `crates/dom/src/lib.rs` — Re-exported `ReadyState`
- `crates/dom/src/tests/mod.rs` — Added `ready_state_tests` module
- `crates/dom/src/tests/ready_state_tests.rs` — NEW: 5 unit tests for ReadyState
- `crates/web_api/src/event_target.rs` — Added `Window` variant to `EventTargetId`
- `crates/web_api/src/event_dispatch.rs` — Made `invoke_listeners()` `pub(crate)` for lifecycle event dispatch
- `crates/web_api/src/lib.rs` — Added `dispatch_lifecycle_event()`, `fire_dom_content_loaded()`, `fire_load_event()`, body onload Window fix, 8 lifecycle unit tests
- `crates/web_api/src/dom_host/mod.rs` — Updated `event_target_id_for()` and `event_target_to_host_object()` for Window
- `crates/web_api/src/dom_host/host_environment.rs` — Added `document.readyState` JS property
- `crates/browser_runtime/src/lib.rs` — Added `fire_dom_content_loaded()` and `fire_load_event()` forwarding methods
- `crates/app_browser/src/event_handler.rs` — Wired lifecycle events into pipeline (Phase 4 + Phase 6)
- `tests/js_lifecycle.rs` — NEW: 4 integration tests for document lifecycle
- `Cargo.toml` — Added `[[test]]` entry for `js_lifecycle`
- `docs/HTML5_Implementation_Checklist.md` — Checked off Phase 4 readiness items
- `docs/DOM_Implementation_Checklist.md` — Checked off readyState, DOMContentLoaded, readystatechange
## Change Log
- 2026-03-15: Implemented document lifecycle events (DOMContentLoaded, load, readystatechange) with readyState transitions and Window event target support
- 2026-03-15: Code review fixes — removed invoke_listeners_simple wrapper (M1), changed dispatch_lifecycle_event to return () (M2), added window.request_redraw() after load handler DOM mutations (M3), added idempotency guards to lifecycle methods (M4), enforced forward-only readyState transitions (L1), removed no-change file list entry (L2), added TODO for other body-forwarded events (L3), clarified phase numbering comments (L4)
Reference in New Issue View Git Blame Copy Permalink