From 0fbbf52ba7deb7569b53d07ab9f777982f902e1e Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 9 Sep 2026 18:12:38 +0700 Subject: [PATCH 01/18] fix(layout): size input[type=number] like the other text inputs The type list that grants the 300px-by-line-height intrinsic content box omitted "number", while the list in layout::construct that decides which inputs get a text editor included it. A number input therefore got an editor and a zero content box, measuring 6x6 (its padding and border) against 306x25.2 for every other text-like type. --- packages/blitz-dom/src/layout/mod.rs | 12 ++++- .../blitz-tests/tests/number_input_sizing.rs | 46 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 tests/blitz-tests/tests/number_input_sizing.rs diff --git a/packages/blitz-dom/src/layout/mod.rs b/packages/blitz-dom/src/layout/mod.rs index b6c88efc..67f3fa0c 100644 --- a/packages/blitz-dom/src/layout/mod.rs +++ b/packages/blitz-dom/src/layout/mod.rs @@ -519,7 +519,17 @@ impl BaseDocument { }, ); } - None | Some("text" | "password" | "email" | "tel" | "url" | "search") => { + // Kept in step with the list in + // `layout::construct::collect_layout_children`, which + // decides which inputs get a text editor. A type that + // is on that list and not this one gets an editor and + // no content box: `number` measured 6x6, its padding + // and border alone, against 306x25.2 for every other + // text-like type. + None + | Some( + "text" | "password" | "email" | "number" | "tel" | "url" | "search", + ) => { return compute_leaf_layout( inputs, node.style(), diff --git a/tests/blitz-tests/tests/number_input_sizing.rs b/tests/blitz-tests/tests/number_input_sizing.rs new file mode 100644 index 00000000..6a41e285 --- /dev/null +++ b/tests/blitz-tests/tests/number_input_sizing.rs @@ -0,0 +1,46 @@ +//! `input[type=number]` gets a text editor like every other text-like input, so +//! it has to get the same intrinsic content box. It did not: the type list in +//! `layout::construct` included `number` and the one in `layout::mod` did not, +//! so a bare number input measured its padding and border and nothing else. + +use blitz_dom::DocumentConfig; +use blitz_html::{HtmlDocument, HtmlProvider}; +use blitz_traits::shell::{ColorScheme, Viewport}; +use std::sync::Arc; + +fn size_of(doc: &HtmlDocument, selector: &str) -> (f32, f32) { + let node_id = doc + .query_selector(selector) + .unwrap() + .unwrap_or_else(|| panic!("no node matching {selector}")); + let layout = doc.get_node(node_id).unwrap().final_layout(); + (layout.size.width, layout.size.height) +} + +#[test] +fn a_number_input_is_sized_like_the_other_text_inputs() { + let mut doc = HtmlDocument::from_html( + r#" +
+
+ "#, + DocumentConfig { + viewport: Some(Viewport::new(800, 600, 1.0, ColorScheme::Light)), + html_parser_provider: Some(Arc::new(HtmlProvider) as _), + ..Default::default() + }, + ); + doc.resolve(0.0); + + let text = size_of(&doc, "#text"); + let number = size_of(&doc, "#number"); + + assert!( + number.0 > 100.0 && number.1 > 10.0, + "a number input collapsed to its padding and border: {number:?}" + ); + assert_eq!( + number, text, + "a number input must measure the same as a text input" + ); +} From 01b484eed4ce0be11166ddd5dff26acaeb4a8d36 Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 9 Sep 2026 18:15:09 +0700 Subject: [PATCH 02/18] fix(dom): prune removed nodes from a stacking context before resolving it flush_styles_to_layout returns early on a display:none subtree, so a stacking-context host hidden after its child list was built never rebuilds that list. resolve_hoisted_positions walks every node that has a context, hidden or not, and indexed the slab directly, so removing one of the listed children panicked with "invalid SlotMap key used" at resolve.rs:863 on the next resolve. Reported 6 times out of 6 from a signup page whose header menu switched language and then opened a chat launcher. --- packages/blitz-dom/src/resolve.rs | 23 +++++ .../tests/hoisted_child_survives_removal.rs | 83 +++++++++++++++++++ 2 files changed, 106 insertions(+) create mode 100644 tests/blitz-tests/tests/hoisted_child_survives_removal.rs diff --git a/packages/blitz-dom/src/resolve.rs b/packages/blitz-dom/src/resolve.rs index 61436d38..b5a6141f 100644 --- a/packages/blitz-dom/src/resolve.rs +++ b/packages/blitz-dom/src/resolve.rs @@ -859,6 +859,29 @@ impl BaseDocument { continue; }; + // A stacking context is assembled during a style flush and then + // outlives it. `flush_styles_to_layout` returns early on a + // `display: none` subtree, so a host hidden after its list was + // built never rebuilds that list, while this pass walks every node + // that has a context, hidden or not. Removing one of the listed + // children left a freed key behind, and the next resolve panicked + // with "invalid SlotMap key used" rather than painting one frame + // stale. Prune here, where the whole list is already in hand. + if context + .children + .iter() + .any(|child| !self.nodes.contains_key(child.node_id)) + { + context + .children + .retain(|child| self.nodes.contains_key(child.node_id)); + context.negative_z_count = context + .children + .iter() + .take_while(|child| child.z_index < 0) + .count() as u32; + } + for child in context.children.iter_mut() { let node = &self.nodes[child.node_id]; if node diff --git a/tests/blitz-tests/tests/hoisted_child_survives_removal.rs b/tests/blitz-tests/tests/hoisted_child_survives_removal.rs new file mode 100644 index 00000000..8471a159 --- /dev/null +++ b/tests/blitz-tests/tests/hoisted_child_survives_removal.rs @@ -0,0 +1,83 @@ +//! A stacking context must not outlive the nodes it lists. +//! +//! `flush_styles_to_layout` returns early on a `display: none` subtree, so a +//! stacking-context host inside one keeps the child list it was given on the +//! frame before it was hidden. `resolve_hoisted_positions` walks every node +//! that has a stacking context, hidden or not, and indexed the slab directly, +//! so removing one of those listed children panicked with +//! "invalid SlotMap key used" at resolve.rs:863 on the very next resolve. +//! +//! Reported from a signup page whose header menu switched language English, +//! Spanish, English and then opened a chat launcher: 6 crashes out of 6 runs. +//! The shape reproduced here is that sequence with the site removed. + +use blitz_dom::DocumentConfig; +use blitz_html::{HtmlDocument, HtmlProvider}; +use blitz_traits::shell::{ColorScheme, Viewport}; +use markup5ever::{QualName, local_name, ns}; +use std::sync::Arc; + +const HTML: &str = r#" + + + + +"#; + +#[test] +fn removing_a_hoisted_child_of_a_hidden_stacking_context_does_not_panic() { + let mut doc = HtmlDocument::from_html( + HTML, + DocumentConfig { + viewport: Some(Viewport::new(600, 400, 1.0, ColorScheme::Light)), + html_parser_provider: Some(Arc::new(HtmlProvider) as _), + ..Default::default() + }, + ); + doc.resolve(0.0); + + let menu = doc.query_selector("#menu").unwrap().expect("menu"); + let panel = doc.query_selector("#panel").unwrap().expect("panel"); + let raised = doc.query_selector("#raised").unwrap().expect("raised"); + + assert!( + doc.get_node(panel) + .unwrap() + .stacking_context + .as_ref() + .is_some_and(|context| context.children.iter().any(|child| child.node_id == raised)), + "fixture must hoist #raised into #panel's stacking context" + ); + + // Hide the menu. The walk stops at it, so #panel keeps the child list it + // was handed before it went away. + doc.mutate().set_attribute( + menu, + QualName::new(None, ns!(), local_name!("style")), + "display: none", + ); + doc.resolve(0.0); + + // Now drop the hoisted child, exactly as a re-render of the hidden subtree + // would. + doc.mutate().remove_and_drop_node(raised); + + // Panicked here before: nothing pruned #panel's list. + doc.resolve(0.0); + + assert!( + doc.get_node(panel) + .unwrap() + .stacking_context + .as_ref() + .is_none_or(|context| context.children.iter().all(|child| child.node_id != raised)), + "the removed child must not stay in the stacking context" + ); +} From 594d2986991880b9800fb2857239a4cda3781c62 Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 9 Sep 2026 18:30:13 +0700 Subject: [PATCH 03/18] fix(dom): make implicit form submission safe and spec-correct Two defects on the same walk of controls_to_form. The map is never pruned when a control leaves the document, so a framework that re-renders a field leaves a freed id in it. The walk indexed the slab for every id it found, so setting a value twice and then pressing Enter panicked with "invalid SlotMap key used" and took the host down. The walk now goes over the tree, which only yields live nodes, and removal prunes the map so it cannot grow without bound. The "more than one field that blocks implicit submission" rule is gated by the spec on the form having no submit button. It was applied unconditionally, so Enter never submitted any form with two explicitly typed fields even when the form had a Sign in button. Three sites hit this. --- packages/blitz-dom/src/document.rs | 6 + packages/blitz-dom/src/events/keyboard.rs | 64 +++++++- .../tests/implicit_form_submission.rs | 154 ++++++++++++++++++ 3 files changed, 215 insertions(+), 9 deletions(-) create mode 100644 tests/blitz-tests/tests/implicit_form_submission.rs diff --git a/packages/blitz-dom/src/document.rs b/packages/blitz-dom/src/document.rs index 5f731cb8..35f31c0c 100644 --- a/packages/blitz-dom/src/document.rs +++ b/packages/blitz-dom/src/document.rs @@ -1252,6 +1252,12 @@ impl BaseDocument { self.drag_mode = DragMode::None; } self.scrollbar_activity.remove(&node_id); + + // The form-owner map is keyed by control id and was never pruned, so a + // page that re-renders its fields grew an entry per render, every one + // of them a freed slot. Nothing dereferences those any more, but an + // unbounded map keyed on dead ids is a leak either way. + self.controls_to_form.remove(&node_id); } pub(crate) fn drop_node_ignoring_parent(&mut self, node_id: NodeId) -> Option { diff --git a/packages/blitz-dom/src/events/keyboard.rs b/packages/blitz-dom/src/events/keyboard.rs index 8f58edfe..3f1fd693 100644 --- a/packages/blitz-dom/src/events/keyboard.rs +++ b/packages/blitz-dom/src/events/keyboard.rs @@ -150,20 +150,66 @@ impl BaseDocument { } } -/// https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#field-that-blocks-implicit-submission +/// Whether this control is a submit button, and so a candidate for the form's +/// default button. +/// +/// +fn is_submit_button(element_data: &crate::ElementData) -> bool { + let type_attr = element_data.attr(local_name!("type")); + match element_data.name.local { + // A `button` with no type, or an unrecognised one, is in the Submit + // Button state. + local_name!("button") => !matches!(type_attr, Some("reset" | "button" | "menu")), + local_name!("input") => matches!(type_attr, Some("submit" | "image")), + _ => false, + } +} + +/// fn implicit_form_submission( doc: &BaseDocument, text_target: NodeId, mut dispatch_event: F, ) { - let Some(form_owner_id) = doc.controls_to_form.get(&text_target) else { + let Some(&form_owner_id) = doc.controls_to_form.get(&text_target) else { return; }; - if doc - .controls_to_form + + // Walked over the tree rather than over `controls_to_form` directly, for + // two reasons. Tree order is what names the default button, and the map is + // not a reliable source of live node ids: a control removed from the + // document leaves its key behind, so a framework that re-rendered a field + // left a freed id in it and indexing the slab for that id panicked with + // "invalid SlotMap key used", taking the host process down. The traverser + // only yields nodes that are still in the tree. + let controls: Vec = crate::traversal::TreeTraverser::new(doc) + .filter(|control_id| doc.controls_to_form.get(control_id) == Some(&form_owner_id)) + .collect(); + + // "If the form has a default button, then act as if that button was + // clicked." The multiple-fields rule below applies only when there is none, + // which is the whole of the gate: a login form with a user field, a + // password field and a Sign in button submits on Enter, and this returned + // early on it instead. + if let Some(default_button) = controls.iter().copied().find(|control_id| { + doc.get_node(*control_id) + .and_then(|node| node.element_data()) + .is_some_and(is_submit_button) + }) { + dispatch_event(DomEvent::new( + form_owner_id, + DomEventData::Submit(BlitzSubmitEvent { + form: form_owner_id.as_u64(), + submitter: default_button.as_u64(), + }), + )); + return; + } + + if controls .iter() - .filter(|(_control_id, form_id)| *form_id == form_owner_id) - .filter_map(|(control_id, _)| doc.nodes[*control_id].element_data()) + .filter_map(|control_id| doc.get_node(*control_id)) + .filter_map(|node| node.element_data()) .filter(|element_data| { element_data.attr(local_name!("type")).is_some_and(|t| { matches!( @@ -193,10 +239,10 @@ fn implicit_form_submission( // pressing Enter in the only text field of a form is a submission a page // handles, and a default action nothing can cancel is not one. dispatch_event(DomEvent::new( - *form_owner_id, + form_owner_id, DomEventData::Submit(BlitzSubmitEvent { - form: (*form_owner_id).as_u64(), - submitter: (*form_owner_id).as_u64(), + form: form_owner_id.as_u64(), + submitter: form_owner_id.as_u64(), }), )); } diff --git a/tests/blitz-tests/tests/implicit_form_submission.rs b/tests/blitz-tests/tests/implicit_form_submission.rs new file mode 100644 index 00000000..2d327867 --- /dev/null +++ b/tests/blitz-tests/tests/implicit_form_submission.rs @@ -0,0 +1,154 @@ +//! Pressing Enter in a form control must not dereference a control that has +//! been removed, and must submit when the form has a submit button. +//! +//! Two separate defects, both on the same walk of `controls_to_form`: +//! +//! 1. Setting a field's value re-creates its input node. The old id stayed in +//! `controls_to_form`, and `implicit_form_submission` indexed the slab for +//! every control it found there, so the second value-set followed by Enter +//! panicked with "invalid SlotMap key used" and took the host down with it. +//! +//! 2. The "more than one field that blocks implicit submission" rule is gated +//! on the form having no submit button +//! (). +//! Applying it unconditionally meant Enter never submitted any form with two +//! explicitly typed fields, submit button or not. Three sites hit this. + +use blitz_dom::{Document, DocumentConfig}; +use blitz_html::{HtmlDocument, HtmlProvider}; +use blitz_traits::{ + navigation::{NavigationOptions, NavigationProvider}, + shell::{ColorScheme, Viewport}, +}; +use keyboard_types::{Code, Key, Location, Modifiers}; + +use std::sync::{Arc, Mutex}; + +#[derive(Default)] +struct RecordingNavigation { + navigations: Mutex>, +} + +impl NavigationProvider for RecordingNavigation { + fn navigate_to(&self, options: NavigationOptions) { + self.navigations + .lock() + .unwrap() + .push(options.url.to_string()); + } +} + +fn doc_with(html: &str, navigation: Arc) -> HtmlDocument { + HtmlDocument::from_html( + html, + DocumentConfig { + viewport: Some(Viewport::new(800, 600, 1.0, ColorScheme::Light)), + html_parser_provider: Some(Arc::new(HtmlProvider) as _), + navigation_provider: Some(navigation), + base_url: Some("https://example.test/page".to_string()), + ..Default::default() + }, + ) +} + +fn press_enter(doc: &mut HtmlDocument, node_id: blitz_traits::node_id::NodeId) { + doc.set_focus_to(node_id); + doc.handle_ui_event(blitz_traits::events::UiEvent::KeyDown( + blitz_traits::events::BlitzKeyEvent { + key: Key::Enter, + code: Code::Enter, + modifiers: Modifiers::empty(), + location: Location::Standard, + is_auto_repeating: false, + is_composing: false, + state: blitz_traits::events::KeyState::Pressed, + text: None, + }, + )); +} + +const REBUILT: &str = r#" + +
+ +
+ +"#; + +#[test] +fn enter_after_two_value_sets_does_not_panic_on_a_recreated_control() { + let navigation = Arc::new(RecordingNavigation::default()); + let mut doc = doc_with(REBUILT, Arc::clone(&navigation)); + doc.resolve(0.0); + + let form = doc.query_selector("#form").unwrap().expect("form"); + + // Setting the field's value through a framework re-renders the form and + // gives the input a new node. Twice, because the first pass is what leaves + // the dead id behind and the second is what walks past it. + for value in ["one", "two"] { + let mut mutator = doc.mutate(); + mutator.set_inner_html(form, &format!(r#""#)); + drop(mutator); + doc.resolve(0.0); + } + + let input = doc.query_selector("#q").unwrap().expect("input"); + press_enter(&mut doc, input); + + assert_eq!( + navigation.navigations.lock().unwrap().len(), + 1, + "Enter in a single-field form must still submit it" + ); +} + +const TWO_TYPED_FIELDS_WITH_SUBMIT: &str = r#" + +
+ + + +
+ +"#; + +#[test] +fn enter_submits_a_multi_field_form_that_has_a_submit_button() { + let navigation = Arc::new(RecordingNavigation::default()); + let mut doc = doc_with(TWO_TYPED_FIELDS_WITH_SUBMIT, Arc::clone(&navigation)); + doc.resolve(0.0); + + let user = doc.query_selector("#user").unwrap().expect("user field"); + press_enter(&mut doc, user); + + assert_eq!( + navigation.navigations.lock().unwrap().len(), + 1, + "the spec gates the multi-field rule on the form having no submit button" + ); +} + +const TWO_TYPED_FIELDS_NO_SUBMIT: &str = r#" + +
+ + +
+ +"#; + +#[test] +fn enter_does_not_submit_a_multi_field_form_with_no_submit_button() { + let navigation = Arc::new(RecordingNavigation::default()); + let mut doc = doc_with(TWO_TYPED_FIELDS_NO_SUBMIT, Arc::clone(&navigation)); + doc.resolve(0.0); + + let user = doc.query_selector("#user").unwrap().expect("user field"); + press_enter(&mut doc, user); + + assert!( + navigation.navigations.lock().unwrap().is_empty(), + "with no submit button, more than one blocking field must block submission" + ); +} From ae090657f0800e855915062d3724b794a99d585e Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 9 Sep 2026 18:32:11 +0700 Subject: [PATCH 04/18] test: hover restyle reaches the painted frame through a pointer move Answers a QA finding that reported hover leaving every rendered pixel unchanged on four controls. Neither proposed cause holds in the engine: the hover media feature cannot evaluate false here (make_device passes a fixed PointerCapabilities::default(), with no host input), and a UiEvent::PointerMove followed by a resolve does reach the paint, including when the hover colour arrives through a custom property gated on (hover: hover), which is the shape the component library ships. --- .../tests/hover_pointer_move_repaint.rs | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 tests/blitz-tests/tests/hover_pointer_move_repaint.rs diff --git a/tests/blitz-tests/tests/hover_pointer_move_repaint.rs b/tests/blitz-tests/tests/hover_pointer_move_repaint.rs new file mode 100644 index 00000000..e9f3b597 --- /dev/null +++ b/tests/blitz-tests/tests/hover_pointer_move_repaint.rs @@ -0,0 +1,105 @@ +//! Hover driven the way a host drives it: a `UiEvent::PointerMove`, then a +//! resolve, then a paint. +//! +//! `hover_media.rs` already proves `@media (hover: hover)` evaluates true and +//! that `set_hover_to` restyles. That is not the path a host takes. This is, +//! including the case a component library actually ships: the hover colour on a +//! CSS custom property, read by a rule on the element itself. + +use anyrender::render_to_buffer; +use anyrender_vello_cpu::VelloCpuImageRenderer; +use blitz_dom::{Document, DocumentConfig}; +use blitz_html::{HtmlDocument, HtmlProvider}; +use blitz_paint::paint_scene; +use blitz_traits::events::{ + BlitzPointerEvent, BlitzPointerId, MouseEventButton, MouseEventButtons, Point, PointerCoords, + PointerDetails, UiEvent, +}; +use blitz_traits::shell::{ColorScheme, Viewport}; +use std::sync::Arc; + +fn pixel(doc: &mut HtmlDocument) -> [u8; 3] { + let buffer = render_to_buffer::( + |scene| paint_scene(scene, doc, 1.0, 200, 100, 0, 0), + 200, + 100, + ); + let offset = 40 * 200 * 4 + 40 * 4; + [buffer[offset], buffer[offset + 1], buffer[offset + 2]] +} + +fn pointer_move(x: f32, y: f32) -> UiEvent { + UiEvent::PointerMove(BlitzPointerEvent { + id: BlitzPointerId::Mouse, + is_primary: true, + coords: PointerCoords { + page_x: x, + page_y: y, + screen_x: x, + screen_y: y, + client_x: x, + client_y: y, + }, + button: MouseEventButton::Main, + buttons: MouseEventButtons::empty(), + mods: Default::default(), + details: PointerDetails::default(), + element: Point::default(), + active_pointers: Default::default(), + }) +} + +fn doc_from(html: &str) -> HtmlDocument { + HtmlDocument::from_html( + html, + DocumentConfig { + viewport: Some(Viewport::new(200, 100, 1.0, ColorScheme::Light)), + html_parser_provider: Some(Arc::new(HtmlProvider) as _), + ..Default::default() + }, + ) +} + +#[test] +fn a_pointer_move_repaints_a_hover_rule() { + let mut doc = doc_from( + r#""#, + ); + doc.resolve(0.0); + assert_eq!(pixel(&mut doc), [0, 0, 255]); + + doc.handle_ui_event(pointer_move(40.0, 40.0)); + doc.resolve(0.0); + assert_eq!( + pixel(&mut doc), + [255, 0, 0], + "a pointer move must leave the hover rule in the next painted frame" + ); +} + +#[test] +fn a_pointer_move_repaints_a_hover_rule_written_through_a_custom_property() { + let mut doc = doc_from( + r#""#, + ); + doc.resolve(0.0); + assert_eq!(pixel(&mut doc), [0, 0, 255]); + + doc.handle_ui_event(pointer_move(40.0, 40.0)); + doc.resolve(0.0); + assert_eq!( + pixel(&mut doc), + [255, 0, 0], + "the hover colour arrives through a custom property gated on (hover: hover)" + ); +} From 3bcf5f82f79032c5197eb54fdad49f1c1cc27de9 Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 9 Sep 2026 18:33:09 +0700 Subject: [PATCH 05/18] refactor(dom): reuse ElementData::is_submit_button for the default button The implicit-submission helper had grown a second definition of the button case. Delegate to the existing one, which also handles the command attributes that take a button back out of the Submit Button state, and keep only the input types here. --- packages/blitz-dom/src/events/keyboard.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/blitz-dom/src/events/keyboard.rs b/packages/blitz-dom/src/events/keyboard.rs index 3f1fd693..fd6dae68 100644 --- a/packages/blitz-dom/src/events/keyboard.rs +++ b/packages/blitz-dom/src/events/keyboard.rs @@ -155,14 +155,14 @@ impl BaseDocument { /// /// fn is_submit_button(element_data: &crate::ElementData) -> bool { - let type_attr = element_data.attr(local_name!("type")); - match element_data.name.local { - // A `button` with no type, or an unrecognised one, is in the Submit - // Button state. - local_name!("button") => !matches!(type_attr, Some("reset" | "button" | "menu")), - local_name!("input") => matches!(type_attr, Some("submit" | "image")), - _ => false, - } + // `ElementData::is_submit_button` covers ` +
+ + + "#, + ); + + let (category, pointer) = { + let inner = doc.inner(); + let category = inner.query_selector("#category").unwrap().unwrap(); + let pointer = match inner + .get_node(category) + .unwrap() + .synthetic_click_event(keyboard_types::Modifiers::empty()) + { + DomEventData::Click(pointer) => pointer, + _ => unreachable!(), + }; + (category, pointer) + }; + doc.dispatch_dom_event(DomEvent::new(category, DomEventData::Click(pointer))); + + assert_eq!( + text_of_selector(&doc, "#out"), + "category:true|", + "cancelBubble must report the stop-propagation flag, and the ancestor must not run" + ); +} + +#[test] +fn setting_cancel_bubble_stops_propagation() { + let mut doc = doc_from_html( + r#" + +
+
+ + + "#, + ); + + let (category, pointer) = { + let inner = doc.inner(); + let category = inner.query_selector("#category").unwrap().unwrap(); + let pointer = match inner + .get_node(category) + .unwrap() + .synthetic_click_event(keyboard_types::Modifiers::empty()) + { + DomEventData::Click(pointer) => pointer, + _ => unreachable!(), + }; + (category, pointer) + }; + doc.dispatch_dom_event(DomEvent::new(category, DomEventData::Click(pointer))); + + assert_eq!( + text_of_selector(&doc, "#out"), + "category|", + "the backdrop is an ancestor of the panel, so a dismiss handler on it must not run" + ); +} From 29954674f301557375a6f48a00b55ed160f713a6 Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 9 Sep 2026 18:49:01 +0700 Subject: [PATCH 07/18] feat(script): expose Element.scrollIntoView blitz-dom has had scroll_to_node all along, for the harness's agent action, but page script could not reach it: getElementById(...).scrollIntoView() raised "TypeError: not a callable function". Any anchor-scrolling router, a "back to top" control and a validation-error focuser all call it. The argument is accepted and ignored, because scroll_to_node lands the node at the top-left of each scrollport, which is the default block: "start". --- packages/blitz-script/src/dom/element.rs | 20 +++++ .../blitz-script/tests/scroll_into_view.rs | 90 +++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 packages/blitz-script/tests/scroll_into_view.rs diff --git a/packages/blitz-script/src/dom/element.rs b/packages/blitz-script/src/dom/element.rs index 505621a9..1cee8965 100644 --- a/packages/blitz-script/src/dom/element.rs +++ b/packages/blitz-script/src/dom/element.rs @@ -122,6 +122,7 @@ pub(crate) fn init_element_proto(proto: &JsObject, context: &mut Context) { define_method(proto, "hasAttribute", 1, has_attribute, context); define_method(proto, "focus", 0, focus, context); define_method(proto, "blur", 0, blur, context); + define_method(proto, "scrollIntoView", 1, scroll_into_view, context); define_method( proto, "getBoundingClientRect", @@ -1057,6 +1058,25 @@ fn blur(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult JsResult { + let ctx = dom_ctx(context)?; + let node_id = this_node_id(this)?; + // Scrolling is a geometry operation, so the mutations script already made + // have to be laid out before the offsets are computed. See DomCtx::flush_layout. + ctx.flush_layout(); + ctx.mutate_doc().scroll_to_node(node_id); + Ok(JsValue::undefined()) +} + // === Geometry === fn get_scroll_left(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { diff --git a/packages/blitz-script/tests/scroll_into_view.rs b/packages/blitz-script/tests/scroll_into_view.rs new file mode 100644 index 00000000..3e65bda7 --- /dev/null +++ b/packages/blitz-script/tests/scroll_into_view.rs @@ -0,0 +1,90 @@ +//! `scrollIntoView` on an element, which blitz-dom has had all along as +//! `scroll_to_node` but never exposed to page script. A page calling it got +//! "TypeError: not a callable function", which is what an anchor-scrolling +//! router, a "back to top" control and a validation-error focuser all call. + +use blitz_dom::{Document, DocumentConfig}; +use blitz_script::ScriptDocument; +use blitz_traits::events::{DomEvent, DomEventData}; + +fn doc_from_html(html: &str) -> ScriptDocument { + // A real viewport: a zero-width one gives every box a zero content size, so + // nothing overflows and nothing scrolls. + let mut doc = ScriptDocument::from_html( + html, + DocumentConfig { + viewport: Some(blitz_traits::shell::Viewport::new( + 800, + 600, + 1.0, + blitz_traits::shell::ColorScheme::Light, + )), + ..Default::default() + }, + ); + doc.execute_scripts(); + doc +} + +fn text_of_selector(doc: &ScriptDocument, selector: &str) -> String { + let inner = doc.inner(); + let node_id = inner + .query_selector(selector) + .unwrap() + .unwrap_or_else(|| panic!("no node matching {selector}")); + inner.get_node(node_id).unwrap().text_content() +} + +#[test] +fn scroll_into_view_is_callable_from_page_script() { + let mut doc = doc_from_html( + r#" + +
+
+
target
+
+ +
+ + + "#, + ); + doc.inner_mut().resolve(0.0); + + let (jump, pointer) = { + let inner = doc.inner(); + let jump = inner.query_selector("#jump").unwrap().unwrap(); + let pointer = match inner + .get_node(jump) + .unwrap() + .synthetic_click_event(keyboard_types::Modifiers::empty()) + { + DomEventData::Click(pointer) => pointer, + _ => unreachable!(), + }; + (jump, pointer) + }; + doc.dispatch_dom_event(DomEvent::new(jump, DomEventData::Click(pointer))); + + assert_eq!(text_of_selector(&doc, "#out"), "called"); + let scrolled = { + let inner = doc.inner(); + let pane = inner.query_selector("#pane").unwrap().unwrap(); + inner.get_node(pane).unwrap().scroll_offset().y + }; + assert!( + scrolled > 0.0, + "scrollIntoView must move the nearest scroll container, not just return" + ); +} From bddcd96cab91b80cdb51c5931d4623e09f7d88fb Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 9 Sep 2026 18:49:36 +0700 Subject: [PATCH 08/18] feat(script): give window.location assign, replace and reload location was a plain data object, so a page calling any of the three threw a TypeError out of whatever ran it. All three now go through the document's navigation provider, which is where a link click already goes; BaseDocument grows navigate_to_url and current_url because both resolve_url and the provider were internal to blitz-dom. --- packages/blitz-dom/src/document.rs | 25 +++++++ packages/blitz-script/src/runtime.rs | 41 ++++++++++++ .../blitz-script/tests/location_navigation.rs | 66 +++++++++++++++++++ 3 files changed, 132 insertions(+) create mode 100644 packages/blitz-script/tests/location_navigation.rs diff --git a/packages/blitz-dom/src/document.rs b/packages/blitz-dom/src/document.rs index 35f31c0c..13d26253 100644 --- a/packages/blitz-dom/src/document.rs +++ b/packages/blitz-dom/src/document.rs @@ -1395,6 +1395,31 @@ impl BaseDocument { }) } + /// Navigate to `raw`, resolved against this document's base URL. + /// + /// The same route a link click takes, exposed so that script can reach it: + /// `location.assign`, `location.replace` and `location.reload` had nowhere + /// to go, because `resolve_url` and the navigation provider are both + /// internal to this crate. Returns `false` when `raw` will not resolve, + /// so the caller can report that rather than navigate somewhere wrong. + pub fn navigate_to_url(&self, raw: &str) -> bool { + let Some(url) = self.url.resolve_relative(raw) else { + return false; + }; + self.navigation_provider + .navigate_to(blitz_traits::navigation::NavigationOptions::new( + url, + None, + self.id(), + )); + true + } + + /// This document's URL, as a page's `location.href` reads it. + pub fn current_url(&self) -> String { + self.url.to_string() + } + pub fn print_tree(&self) { crate::util::walk_tree(0, self.root_node()); } diff --git a/packages/blitz-script/src/runtime.rs b/packages/blitz-script/src/runtime.rs index 28e79b25..08de2614 100644 --- a/packages/blitz-script/src/runtime.rs +++ b/packages/blitz-script/src/runtime.rs @@ -1498,10 +1498,51 @@ fn build_location(base_url: Option<&Url>, context: &mut Context) -> JsValue { Attribute::all(), ) .property(js_string!("hash"), JsString::from(hash), Attribute::all()) + // `assign`, `replace` and `reload`. Without them `location` was a plain + // data object, so a page calling any of the three threw a TypeError out + // of whatever ran it. All three go through the document's navigation + // provider, which is where a link click goes. + .function( + NativeFunction::from_fn_ptr(location_assign), + js_string!("assign"), + 1, + ) + .function( + NativeFunction::from_fn_ptr(location_assign), + js_string!("replace"), + 1, + ) + .function( + NativeFunction::from_fn_ptr(location_reload), + js_string!("reload"), + 0, + ) .build() .into() } +/// Serves both `assign` and `replace`. They differ only in whether the current +/// entry is kept in the session history, and this runtime's history is a +/// script-level shim that a real navigation replaces wholesale either way. +fn location_assign(_: &JsValue, args: &[JsValue], context: &mut Context) -> JsResult { + let ctx = dom_ctx(context)?; + let url = crate::dom::to_rust_string(args.first().unwrap_or(&JsValue::undefined()), context)?; + if !ctx.doc.borrow().navigate_to_url(&url) { + return Err(JsNativeError::typ() + .with_message(format!("{url} is not a valid URL")) + .into()); + } + Ok(JsValue::undefined()) +} + +fn location_reload(_: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { + let ctx = dom_ctx(context)?; + let doc = ctx.doc.borrow(); + let current = doc.current_url(); + doc.navigate_to_url(¤t); + Ok(JsValue::undefined()) +} + fn window_inner_width(_: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { let ctx = dom_ctx(context)?; let viewport = ctx.doc.borrow().get_viewport(); diff --git a/packages/blitz-script/tests/location_navigation.rs b/packages/blitz-script/tests/location_navigation.rs new file mode 100644 index 00000000..1dc94203 --- /dev/null +++ b/packages/blitz-script/tests/location_navigation.rs @@ -0,0 +1,66 @@ +//! `location.assign`, `location.replace` and `location.reload`. `location` +//! was a plain data object, so a page calling any of the three threw. + +use blitz_dom::{Document, DocumentConfig}; +use blitz_script::ScriptDocument; +use blitz_traits::navigation::{NavigationOptions, NavigationProvider}; +use std::sync::{Arc, Mutex}; + +fn text_of_selector(doc: &ScriptDocument, selector: &str) -> String { + let inner = doc.inner(); + let node_id = inner + .query_selector(selector) + .unwrap() + .unwrap_or_else(|| panic!("no node matching {selector}")); + inner.get_node(node_id).unwrap().text_content() +} + +#[derive(Default)] +struct RecordingNavigation { + navigations: Mutex>, +} + +impl NavigationProvider for RecordingNavigation { + fn navigate_to(&self, options: NavigationOptions) { + self.navigations + .lock() + .unwrap() + .push(options.url.to_string()); + } +} + +#[test] +fn location_exposes_assign_replace_and_reload() { + let navigation = Arc::new(RecordingNavigation::default()); + let mut doc = ScriptDocument::from_html( + r#" + +
+ + + "#, + DocumentConfig { + navigation_provider: Some(Arc::clone(&navigation) as Arc), + base_url: Some("https://example.test/page".to_string()), + ..Default::default() + }, + ); + doc.execute_scripts(); + + assert_eq!( + text_of_selector(&doc, "#out"), + "assign:function|replace:function|reload:function" + ); + assert_eq!( + navigation.navigations.lock().unwrap().as_slice(), + ["https://example.test/next"], + "location.assign must reach the navigation provider" + ); +} From 44de0c7faf88b74ce2b6aee68c7c97633c867b79 Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 9 Sep 2026 18:50:51 +0700 Subject: [PATCH 09/18] feat(script): dispatch popstate on a history traversal, and give the window dispatchEvent history.back and history.forward changed the URL and announced nothing, so a router's navigate(-1) never redrew and every route was one-way. Announcing it needed a way to raise a window-targeted event at all: Node.prototype.dispatchEvent walks a node chain and never reaches the window listeners, so window.dispatchEvent is added here and the history shim uses it. --- packages/blitz-script/src/runtime.rs | 70 +++++++++++++++++++++++++ packages/blitz-script/tests/popstate.rs | 61 +++++++++++++++++++++ 2 files changed, 131 insertions(+) create mode 100644 packages/blitz-script/tests/popstate.rs diff --git a/packages/blitz-script/src/runtime.rs b/packages/blitz-script/src/runtime.rs index 08de2614..695f5eda 100644 --- a/packages/blitz-script/src/runtime.rs +++ b/packages/blitz-script/src/runtime.rs @@ -404,6 +404,7 @@ impl ScriptRuntime { 2, window_remove_event_listener, ); + register_global_fn(&mut context, "dispatchEvent", 1, window_dispatch_event); register_global_fn(&mut context, "__blitzRandomU32", 0, random_u32); let mut runtime = Self { @@ -579,6 +580,12 @@ impl ScriptRuntime { if (next === index) return; index = next; applyUrl(entries[index].url); + // A traversal has to announce itself. Without this a + // router's `navigate(-1)` changed the URL and nothing + // redrew, so every route was one-way. + const event = new Event("popstate"); + event.state = entries[index].state; + globalThis.dispatchEvent(event); }, back() { this.go(-1); }, forward() { this.go(1); }, @@ -1660,6 +1667,69 @@ fn clear_timer(_: &JsValue, args: &[JsValue], context: &mut Context) -> JsResult Ok(JsValue::undefined()) } +/// `window.dispatchEvent`, which the window did not have. +/// +/// `Node.prototype.dispatchEvent` walks a node chain and never reaches the +/// window listeners, so there was no way at all to raise a window-targeted +/// event, from a page or from the runtime's own `history` shim. +fn window_dispatch_event( + _: &JsValue, + args: &[JsValue], + context: &mut Context, +) -> JsResult { + let ctx = dom_ctx(context)?; + let event = args + .first() + .and_then(JsValue::as_object) + .filter(|event| event.downcast_ref::().is_some()) + .ok_or_else(|| JsNativeError::typ().with_message("dispatchEvent requires an Event"))?; + let event_type = crate::dom::to_rust_string(&event.get(js_string!("type"), context)?, context)?; + + let global: JsValue = context.global_object().into(); + crate::dom::define_value(&event, "target", global.clone(), context); + crate::dom::define_value(&event, "srcElement", global.clone(), context); + crate::dom::define_value(&event, "currentTarget", global.clone(), context); + crate::dom::define_value(&event, "eventPhase", JsValue::from(2), context); + + let listeners: Vec = { + let mut state = ctx.state.borrow_mut(); + match state.window_listeners.get_mut(&event_type) { + Some(listeners) => { + let cloned = listeners.clone(); + listeners.retain(|listener| !listener.once); + cloned + } + None => Vec::new(), + } + }; + for listener in listeners { + listener + .callback + .call(&global, &[event.clone().into()], context)?; + if event + .downcast_ref::() + .is_some_and(|event| event.stopped_immediate.get()) + { + break; + } + } + + // The `window.on` form, which is how a page most often listens for + // `popstate`. + let on_name = JsString::from(format!("on{event_type}")); + if let Some(handler) = context.global_object().get(on_name, context)?.as_object() + && handler.is_callable() + { + handler.call(&global, &[event.clone().into()], context)?; + } + + crate::dom::define_value(&event, "currentTarget", JsValue::null(), context); + let prevented = event + .downcast_ref::() + .is_some_and(|event| event.prevented.get()); + Ok(JsValue::from(!prevented)) +} + fn window_add_event_listener( _: &JsValue, args: &[JsValue], diff --git a/packages/blitz-script/tests/popstate.rs b/packages/blitz-script/tests/popstate.rs new file mode 100644 index 00000000..483557d7 --- /dev/null +++ b/packages/blitz-script/tests/popstate.rs @@ -0,0 +1,61 @@ +//! `history.back` and `history.forward` must dispatch `popstate`. +//! +//! Without it a router's `navigate(-1)` changed the URL and nothing redrew, so +//! every route was one-way. + +use blitz_dom::{Document, DocumentConfig}; +use blitz_script::ScriptDocument; + +fn doc_from_html(html: &str) -> ScriptDocument { + // A real viewport: a zero-width one gives every box a zero content size, so + // nothing overflows and nothing scrolls. + let mut doc = ScriptDocument::from_html( + html, + DocumentConfig { + viewport: Some(blitz_traits::shell::Viewport::new( + 800, + 600, + 1.0, + blitz_traits::shell::ColorScheme::Light, + )), + ..Default::default() + }, + ); + doc.execute_scripts(); + doc +} + +fn text_of_selector(doc: &ScriptDocument, selector: &str) -> String { + let inner = doc.inner(); + let node_id = inner + .query_selector(selector) + .unwrap() + .unwrap_or_else(|| panic!("no node matching {selector}")); + inner.get_node(node_id).unwrap().text_content() +} + +#[test] +fn history_back_dispatches_popstate() { + let doc = doc_from_html( + r#" + +
+ + + "#, + ); + + assert_eq!( + text_of_selector(&doc, "#out"), + "pop:null|pop:{\"route\":\"b\"}|", + "a traversal must announce itself, or a router never redraws" + ); +} From e405cc3e0715e51156609c6ec76dae8c7e489324 Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 9 Sep 2026 18:52:14 +0700 Subject: [PATCH 10/18] feat(script): fire change on a text control that loses focus having been edited change was synthesised only for checkbox and radio, so any onChange on a text field was dead. That shipped a product bug: a phone-number field bound to change gated a Confirm button that nothing could ever open. A text control commits rather than notifying, so the value at focus is recorded and compared on blur; an untouched field still fires nothing. The markup5ever dev-dependency is for the test, which sets a field's value the way the DOM does. --- packages/blitz-script/Cargo.toml | 1 + packages/blitz-script/src/runtime.rs | 40 +++++- .../blitz-script/tests/text_input_change.rs | 121 ++++++++++++++++++ 3 files changed, 159 insertions(+), 3 deletions(-) create mode 100644 packages/blitz-script/tests/text_input_change.rs diff --git a/packages/blitz-script/Cargo.toml b/packages/blitz-script/Cargo.toml index 14e75903..495d13a3 100644 --- a/packages/blitz-script/Cargo.toml +++ b/packages/blitz-script/Cargo.toml @@ -86,6 +86,7 @@ blitz-paint = { workspace = true } blitz-traits = { workspace = true } image = { workspace = true, features = ["png"] } keyboard-types = { workspace = true } +markup5ever = { workspace = true } url = { workspace = true } # Only on macOS, and only for `tests/text_metrics_macos.rs`, which measures diff --git a/packages/blitz-script/src/runtime.rs b/packages/blitz-script/src/runtime.rs index 695f5eda..22f34862 100644 --- a/packages/blitz-script/src/runtime.rs +++ b/packages/blitz-script/src/runtime.rs @@ -158,6 +158,10 @@ pub(crate) struct ScriptRuntime { /// a failure at evaluation time would report an error against every module /// on the modern web that waits for anything. pending_modules: Vec<(JsPromise, String)>, + /// The value a text control held when it was focussed, so that losing + /// focus can tell an edited field from an untouched one. `change` is a + /// commit event, not a blur notification. + focussed_text_value: Option<(NodeId, String)>, } impl ScriptRuntime { @@ -413,6 +417,7 @@ impl ScriptRuntime { diagnostics, module_loader, pending_modules: Vec::new(), + focussed_text_value: None, }; // Small JS bootstrap for APIs that are easiest to define in JS @@ -945,9 +950,28 @@ impl ScriptRuntime { // Browsers fire a `change` event after `input` events on checkbox/radio // inputs. Blitz only generates `input` events, so synthesise the `change` // event here. - if matches!(event.data, DomEventData::Input(_)) - && self.target_is_checkbox_or_radio(event.target) - { + // + // A text control commits instead: `change` fires when it loses focus + // having been edited, never on each keystroke. Only checkbox and radio + // were covered, so `change` on a text field was dead, and a phone-number + // field bound to it gated a Confirm button that nothing could open. + let synthesise_change = match &event.data { + DomEventData::Input(_) => self.target_is_checkbox_or_radio(event.target), + DomEventData::Focus(_) => { + self.focussed_text_value = self + .text_control_value(event.target) + .map(|value| (event.target, value)); + false + } + DomEventData::Blur(_) => match self.focussed_text_value.take() { + Some((node_id, focussed_value)) if node_id == event.target => self + .text_control_value(event.target) + .is_some_and(|current| current != focussed_value), + _ => false, + }, + _ => false, + }; + if synthesise_change { let mut change_state = EventState::default(); any_called |= self.dispatch_event_inner( chain, @@ -980,6 +1004,16 @@ impl ScriptRuntime { any_called } + /// The current text of a text input or textarea, or `None` for anything + /// that is not one. + fn text_control_value(&self, node_id: NodeId) -> Option { + let doc = self.ctx.doc.borrow(); + doc.get_node(node_id)? + .element_data()? + .text_input_data() + .map(|input| input.editor.text().to_string()) + } + fn target_is_checkbox_or_radio(&self, node_id: NodeId) -> bool { let doc = self.ctx.doc.borrow(); doc.get_node(node_id) diff --git a/packages/blitz-script/tests/text_input_change.rs b/packages/blitz-script/tests/text_input_change.rs new file mode 100644 index 00000000..97b14234 --- /dev/null +++ b/packages/blitz-script/tests/text_input_change.rs @@ -0,0 +1,121 @@ +//! `change` on a text control. +//! +//! It was synthesised only for checkbox and radio, so any `onChange` on a text +//! field was dead. That shipped a product bug: a phone-number field bound to +//! `change` gated a Confirm button that nothing could ever open. + +use blitz_dom::{Document, DocumentConfig}; +use blitz_script::ScriptDocument; +use blitz_traits::events::{DomEvent, DomEventData}; + +fn doc_from_html(html: &str) -> ScriptDocument { + // A real viewport: a zero-width one gives every box a zero content size, so + // nothing overflows and nothing scrolls. + let mut doc = ScriptDocument::from_html( + html, + DocumentConfig { + viewport: Some(blitz_traits::shell::Viewport::new( + 800, + 600, + 1.0, + blitz_traits::shell::ColorScheme::Light, + )), + ..Default::default() + }, + ); + doc.execute_scripts(); + doc +} + +fn text_of_selector(doc: &ScriptDocument, selector: &str) -> String { + let inner = doc.inner(); + let node_id = inner + .query_selector(selector) + .unwrap() + .unwrap_or_else(|| panic!("no node matching {selector}")); + inner.get_node(node_id).unwrap().text_content() +} + +#[test] +fn a_text_input_fires_change_when_it_loses_focus_with_a_new_value() { + let mut doc = doc_from_html( + r#" + + +
+ + + "#, + ); + + doc.inner_mut().resolve(0.0); + let phone = doc.inner().query_selector("#phone").unwrap().unwrap(); + doc.dispatch_dom_event(DomEvent::new( + phone, + DomEventData::Focus(blitz_traits::events::BlitzFocusEvent), + )); + doc.inner_mut().mutate().set_attribute( + phone, + markup5ever::QualName::new(None, markup5ever::ns!(), markup5ever::local_name!("value")), + "555", + ); + doc.dispatch_dom_event(DomEvent::new( + phone, + DomEventData::Input(blitz_traits::events::BlitzInputEvent { + value: "555".to_string(), + }), + )); + doc.dispatch_dom_event(DomEvent::new( + phone, + DomEventData::Blur(blitz_traits::events::BlitzFocusEvent), + )); + + assert_eq!( + text_of_selector(&doc, "#out"), + "input|change:555|", + "a text field must commit a change event, or an onChange gate can never open" + ); +} + +#[test] +fn an_unedited_text_input_does_not_fire_change_on_blur() { + let mut doc = doc_from_html( + r#" + + +
+ + + "#, + ); + + doc.inner_mut().resolve(0.0); + let phone = doc.inner().query_selector("#phone").unwrap().unwrap(); + doc.dispatch_dom_event(DomEvent::new( + phone, + DomEventData::Focus(blitz_traits::events::BlitzFocusEvent), + )); + doc.dispatch_dom_event(DomEvent::new( + phone, + DomEventData::Blur(blitz_traits::events::BlitzFocusEvent), + )); + + assert_eq!( + text_of_selector(&doc, "#out"), + "", + "change is a commit, not a blur notification" + ); +} From fc4eee80aa107bf896a8ec0108662e5e41a35dda Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 9 Sep 2026 19:00:21 +0700 Subject: [PATCH 11/18] fix(layout): give ` + * reported 0x0 and nothing on a page could find it or press it. Its content + * size comes from `layout::select_metrics_of`, because the options it is sized + * from are not in flow. + */ +select { + display: inline-block; + border: 1px solid #999; + padding: 1px 4px; + background-color: white; + white-space: nowrap; + overflow: clip; +} + option { display: none; } diff --git a/packages/blitz-dom/src/layout/mod.rs b/packages/blitz-dom/src/layout/mod.rs index 67f3fa0c..56bf5097 100644 --- a/packages/blitz-dom/src/layout/mod.rs +++ b/packages/blitz-dom/src/layout/mod.rs @@ -331,7 +331,54 @@ impl BaseDocument { } } +/// The widest option label, in characters, and the number of visible rows, for +/// a `` is sized from + // its options, which are other nodes. + let select_metrics = self.select_metrics(dom_node_id(node_id)); + let node = &mut self.nodes[dom_node_id(node_id)]; let font_styles = node.primary_styles().map(|style| { @@ -396,6 +448,26 @@ impl BaseDocument { // }) } NodeData::Element(element_data) | NodeData::AnonymousBlock(element_data) => { + // A `` has to get a layout box. +//! +//! There was no `select` rule in the user-agent stylesheet, so a select +//! computed `display: inline`, and `option { display: none }` left it with no +//! in-flow content. Height on a non-replaced inline is ignored, so even +//! ` + + + + "#); + + assert_eq!( + size_of(&doc, "#country"), + (120.0, 34.0), + "a select must take an authored width and height, which needs a block-level box" + ); +} + +#[test] +fn a_bare_select_still_gets_a_box() { + let doc = doc(r#" + + "#); + + let (width, height) = size_of(&doc, "#country"); + assert!( + width > 0.0 && height > 0.0, + "a select with no authored size must still be hittable, got {width}x{height}" + ); +} From 1e40949d676aa0181335c6d5cee213857e5c6d9e Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 9 Sep 2026 19:09:36 +0700 Subject: [PATCH 12/18] fix(layout): give table rows and row groups the box their cells occupy Table layout flattens rows into a CSS grid of cells, so tr, thead and tbody nodes have their box construction damage cleared and never reach Taffy. Nothing ever wrote a layout for them: every one reported 0x0 and, to anything asking whether an element is displayed, "no". A QA check walking a table by row had nothing to walk. A row is not laid out, it is described, so the context now records which cells belong to which row and which rows to which group, and a pass after rounding derives each box from them. After rounding because final_layout is what every geometry query reads and the rounding pass is what fills it; reading the cells any earlier gets zeroes. --- packages/blitz-dom/src/layout/table.rs | 135 +++++++++++++++++++++ packages/blitz-dom/src/resolve.rs | 7 ++ tests/blitz-tests/tests/table_row_boxes.rs | 115 ++++++++++++++++++ 3 files changed, 257 insertions(+) create mode 100644 tests/blitz-tests/tests/table_row_boxes.rs diff --git a/packages/blitz-dom/src/layout/table.rs b/packages/blitz-dom/src/layout/table.rs index c25ff139..b4ef366b 100644 --- a/packages/blitz-dom/src/layout/table.rs +++ b/packages/blitz-dom/src/layout/table.rs @@ -29,6 +29,7 @@ pub struct TableContext { pub style: taffy::Style, pub cells: Vec, pub rows: Vec, + pub row_groups: Vec, pub computed_grid_info: AtomicRefCell>, pub border_style: Option>, pub border_collapse: BorderCollapse, @@ -52,6 +53,21 @@ pub struct TableRow { // kind: TableItemKind, pub node_id: NodeId, pub height: f32, + /// Where this row's cells sit in [`TableContext::cells`]. + /// + /// Rows are flattened away into the grid, so nothing else records which + /// cells belong to which row, and without that a row cannot be given the + /// box its cells occupy. It reported 0x0 and "not displayed", and anything + /// walking a table by row had nothing to walk. + pub cells: Range, +} + +/// A ``, `` or ``, and the rows it holds. +#[derive(Debug, Clone)] +pub struct TableRowGroup { + pub node_id: NodeId, + /// Where this group's rows sit in [`TableContext::rows`]. + pub rows: Range, } pub(crate) fn build_table_context( @@ -60,6 +76,7 @@ pub(crate) fn build_table_context( ) -> (TableContext, Vec) { let mut cells: Vec = Vec::new(); let mut rows: Vec = Vec::new(); + let mut row_groups: Vec = Vec::new(); let mut row = 0u16; let mut col = 0u16; @@ -104,6 +121,7 @@ pub(crate) fn build_table_context( &mut col, &mut cells, &mut rows, + &mut row_groups, &mut column_sizes, &mut first_cell_border, ); @@ -170,6 +188,7 @@ pub(crate) fn build_table_context( style, cells, rows, + row_groups, computed_grid_info: AtomicRefCell::new(None), border_collapse, border_style: first_cell_border, @@ -188,6 +207,7 @@ pub(crate) fn collect_table_cells( col: &mut u16, cells: &mut Vec, rows: &mut Vec, + row_groups: &mut Vec, columns: &mut Vec, first_cell_border: &mut Option>, ) { @@ -213,6 +233,8 @@ pub(crate) fn collect_table_cells( | DisplayInside::TableHeaderGroup | DisplayInside::TableFooterGroup | DisplayInside::Contents => { + let is_row_group = !matches!(display.inside(), DisplayInside::Contents); + let first_row = rows.len(); let children = std::mem::take(&mut doc.nodes[node_id].children); for child_id in children.iter().copied() { doc.nodes[child_id] @@ -226,20 +248,30 @@ pub(crate) fn collect_table_cells( col, cells, rows, + row_groups, columns, first_cell_border, ); } doc.nodes[node_id].children = children; + if is_row_group { + row_groups.push(TableRowGroup { + node_id, + rows: first_row..rows.len(), + }); + } } DisplayInside::TableRow => { node.remove_damage(CONSTRUCT_DESCENDENT | CONSTRUCT_FC | CONSTRUCT_BOX); *row += 1; *col = 0; + let row_index = rows.len(); + let first_cell = cells.len(); rows.push(TableRow { node_id, height: 0.0, + cells: first_cell..first_cell, }); let children = std::mem::take(&mut doc.nodes[node_id].children); @@ -253,11 +285,13 @@ pub(crate) fn collect_table_cells( col, cells, rows, + row_groups, columns, first_cell_border, ); } doc.nodes[node_id].children = children; + rows[row_index].cells = first_cell..cells.len(); } DisplayInside::TableCell => { // node.remove_damage(CONSTRUCT_DESCENDENT | CONSTRUCT_FC | CONSTRUCT_BOX); @@ -355,6 +389,107 @@ pub(crate) fn collect_table_cells( } } +/// Give every `` and every row group the box its cells occupy. +/// +/// Table layout flattens the rows into a grid of cells, so the row and +/// row-group nodes never reach Taffy and nothing ever wrote a layout for them: +/// each one reported 0x0 and, to anything asking whether an element is +/// displayed, "no". A row is not laid out, it is described, so this runs after +/// the grid is computed and derives each box from the cells that are. +/// +/// Rounded and unrounded are both written. The rounding pass walks the box +/// tree, and these nodes are not in it, so a value left only in the unrounded +/// slot would never reach `final_layout`, which is what every geometry query +/// reads. +impl BaseDocument { + pub(crate) fn assign_table_row_layouts(&mut self) { + let contexts: Vec> = self + .nodes + .iter() + .filter(|(_, node)| node.flags.is_table_root()) + .filter_map( + |(_, node)| match &node.data.downcast_element()?.special_data { + crate::node::SpecialElementData::TableRoot(context) => { + Some(Arc::clone(context)) + } + _ => None, + }, + ) + .collect(); + for context in contexts { + assign_row_layouts(self, &context); + } + } +} + +pub(crate) fn assign_row_layouts(doc: &mut BaseDocument, ctx: &TableContext) { + if ctx.rows.is_empty() { + return; + } + + // The grid's own extent, so that a row spans the table rather than only the + // cells that happen to be in it. A row with a colspan short of the full + // width is still as wide as the table. + let mut left = f32::MAX; + let mut right = f32::MIN; + for cell in &ctx.cells { + let Some(node) = doc.nodes.get(cell.node_id) else { + continue; + }; + let layout = node.final_layout(); + left = left.min(layout.location.x); + right = right.max(layout.location.x + layout.size.width); + } + if left > right { + return; + } + + let mut row_extents: Vec> = Vec::with_capacity(ctx.rows.len()); + for row in &ctx.rows { + let mut top = f32::MAX; + let mut bottom = f32::MIN; + for cell in &ctx.cells[row.cells.clone()] { + let Some(node) = doc.nodes.get(cell.node_id) else { + continue; + }; + let layout = node.final_layout(); + top = top.min(layout.location.y); + bottom = bottom.max(layout.location.y + layout.size.height); + } + if top > bottom { + row_extents.push(None); + continue; + } + row_extents.push(Some((top, bottom))); + write_box(doc, row.node_id, left, top, right - left, bottom - top); + } + + for group in &ctx.row_groups { + let mut top = f32::MAX; + let mut bottom = f32::MIN; + for extent in row_extents[group.rows.clone()].iter().flatten() { + top = top.min(extent.0); + bottom = bottom.max(extent.1); + } + if top > bottom { + continue; + } + write_box(doc, group.node_id, left, top, right - left, bottom - top); + } +} + +fn write_box(doc: &mut BaseDocument, node_id: NodeId, x: f32, y: f32, width: f32, height: f32) { + let Some(node) = doc.nodes.get_mut(node_id) else { + return; + }; + let mut layout = taffy::Layout::with_order(node.final_layout().order); + layout.location = taffy::Point { x, y }; + layout.size = taffy::Size { width, height }; + layout.content_size = layout.size; + *node.unrounded_layout_mut() = layout; + *node.final_layout_mut() = layout; +} + pub struct RangeIter(Range); impl Iterator for RangeIter { diff --git a/packages/blitz-dom/src/resolve.rs b/packages/blitz-dom/src/resolve.rs index b5a6141f..afdf500f 100644 --- a/packages/blitz-dom/src/resolve.rs +++ b/packages/blitz-dom/src/resolve.rs @@ -1058,6 +1058,13 @@ impl BaseDocument { taffy::compute_root_layout(self, root_element_id, available_space); taffy::round_layout(self, root_element_id); + // Table rows and row groups are flattened into a grid of cells and + // never reach Taffy, so nothing wrote a layout for them at all. Describe + // each from the cells it holds, after rounding: `final_layout` is what + // every geometry query reads and the rounding pass is what fills it, so + // doing this any earlier reads cells that are still zero. + self.assign_table_row_layouts(); + // Taffy currently maps CSS `position: fixed` to absolute positioning, // which leaves the box relative to its DOM layout parent. A portal // mounted after a full-height application root therefore starts one diff --git a/tests/blitz-tests/tests/table_row_boxes.rs b/tests/blitz-tests/tests/table_row_boxes.rs new file mode 100644 index 00000000..87e0a950 --- /dev/null +++ b/tests/blitz-tests/tests/table_row_boxes.rs @@ -0,0 +1,115 @@ +//! ``, `` and `` must report a box. +//! +//! Table layout flattens the rows into a CSS grid of cells: the row and +//! row-group nodes have their box construction damage cleared and never reach +//! Taffy, so every one of them reported 0x0 and "not displayed". Anything that +//! walks a table by row, a QA check included, had nothing to walk. + +use blitz_dom::DocumentConfig; +use blitz_html::{HtmlDocument, HtmlProvider}; +use blitz_traits::shell::{ColorScheme, Viewport}; +use std::sync::Arc; + +const HTML: &str = r#" + + + + + + + + +
alpha1
beta2
+ +"#; + +fn rect(doc: &HtmlDocument, selector: &str) -> (f32, f32, f32, f32) { + let node_id = doc + .query_selector(selector) + .unwrap() + .unwrap_or_else(|| panic!("no node matching {selector}")); + let node = doc.get_node(node_id).unwrap(); + let position = node.absolute_position(0.0, 0.0); + let layout = node.final_layout(); + ( + position.x, + position.y, + layout.size.width, + layout.size.height, + ) +} + +#[test] +fn every_row_reports_the_box_its_cells_occupy() { + let mut doc = HtmlDocument::from_html( + HTML, + DocumentConfig { + viewport: Some(Viewport::new(800, 600, 1.0, ColorScheme::Light)), + html_parser_provider: Some(Arc::new(HtmlProvider) as _), + ..Default::default() + }, + ); + doc.resolve(0.0); + + let head = rect(&doc, "#head"); + let first = rect(&doc, "#first"); + let second = rect(&doc, "#second"); + + for (name, row) in [("head", head), ("first", first), ("second", second)] { + assert!( + row.2 > 0.0 && row.3 > 0.0, + "row #{name} has no box: {row:?}" + ); + } + + assert_eq!( + (head.0, head.2), + (first.0, first.2), + "every row spans the same horizontal extent, the table's" + ); + assert_eq!((first.0, first.2), (second.0, second.2)); + assert_eq!(head.3, 20.0, "a row is as tall as its cells"); + + assert!( + head.1 < first.1 && first.1 < second.1, + "rows stack in tree order: {head:?} {first:?} {second:?}" + ); + assert!( + head.1 + head.3 <= first.1 && first.1 + first.3 <= second.1, + "rows do not overlap: {head:?} {first:?} {second:?}" + ); +} + +#[test] +fn a_row_group_spans_the_rows_it_holds() { + let mut doc = HtmlDocument::from_html( + HTML, + DocumentConfig { + viewport: Some(Viewport::new(800, 600, 1.0, ColorScheme::Light)), + html_parser_provider: Some(Arc::new(HtmlProvider) as _), + ..Default::default() + }, + ); + doc.resolve(0.0); + + let body = rect(&doc, "#body"); + let first = rect(&doc, "#first"); + let second = rect(&doc, "#second"); + + assert_eq!( + (body.0, body.2), + (first.0, first.2), + "a row group spans the table horizontally" + ); + assert_eq!(body.1, first.1, "tbody starts at its first row"); + assert_eq!( + body.1 + body.3, + second.1 + second.3, + "tbody ends at its last row" + ); + assert!(body.3 > first.3, "tbody covers both of its rows"); +} From c6c185c14a82b3956b32a3a88defb211921bfc39 Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 9 Sep 2026 19:19:32 +0700 Subject: [PATCH 13/18] fix(layout): generate a table box for a block whose children are table internals table { display: block } is the standard wide-table horizontal-scroll pattern, and it made every row render side by side: the header on one line, then every data row at the same y and increasing x. With the table no longer a table, its thead and tbody were plain block children, and their own displays have no mapping in the style conversion, so they fell through to Taffy's default, which is flex, and a flex container lays its items out in a row. CSS 2.1 17.2.1 requires an anonymous table box around misparented table-internal boxes. When the container holds nothing else, which is this whole pattern and every case seen on a real page, that anonymous box would be its only child and take its content box, so it is fused into the container instead of inserted under it. A container with mixed content still needs a separate anonymous box and does not get one here. --- packages/blitz-dom/src/layout/construct.rs | 108 ++++++++++++++---- .../blitz-tests/tests/display_block_table.rs | 94 +++++++++++++++ 2 files changed, 182 insertions(+), 20 deletions(-) create mode 100644 tests/blitz-tests/tests/display_block_table.rs diff --git a/packages/blitz-dom/src/layout/construct.rs b/packages/blitz-dom/src/layout/construct.rs index caebb689..101f0d08 100644 --- a/packages/blitz-dom/src/layout/construct.rs +++ b/packages/blitz-dom/src/layout/construct.rs @@ -581,6 +581,25 @@ pub(crate) fn collect_layout_children( push_hoisted_children_and_pseudos(doc, container_node_id, out); } DisplayInside::Flow | DisplayInside::FlowRoot | DisplayInside::TableCell => { + // An anonymous table box, fused into this container rather than + // inserted under it. + // + // `table { display: block }` is the standard wide-table + // horizontal-scroll pattern, and it left `thead` and `tbody` as + // plain block children whose own displays have no mapping in the + // style conversion. They fell through to Taffy's default, which is + // flex, so every row rendered SIDE BY SIDE. CSS 2.1 17.2.1 requires + // an anonymous table box around misparented table-internal boxes. + // + // Fusing is exact when the container holds nothing but those boxes, + // which is this whole pattern and every case seen on a real page: + // the anonymous table would be the container's only child and would + // take its content box. A container with mixed content still needs a + // separate anonymous box, and does not get one here. + if block_contains_only_table_internals(doc, container_node_id) { + return make_table_root(doc, container_node_id, out); + } + // display:contents children are transparent for box generation: // their children participate in this container's formatting // context, so classification must recurse into them. @@ -678,26 +697,7 @@ pub(crate) fn collect_layout_children( ); } - DisplayInside::Table => { - let (table_context, tlayout_children) = build_table_context(doc, container_node_id); - #[allow(clippy::arc_with_non_send_sync)] - let data = SpecialElementData::TableRoot(Arc::new(table_context)); - doc.nodes[container_node_id] - .flags - .insert(NodeFlags::IS_TABLE_ROOT); - doc.nodes[container_node_id] - .data - .downcast_element_mut() - .unwrap() - .special_data = data; - if let Some(before) = doc.nodes[container_node_id].before() { - out.push(before, doc); - } - out.extend(&tlayout_children, doc); - if let Some(after) = doc.nodes[container_node_id].after() { - out.push(after, doc); - } - } + DisplayInside::Table => make_table_root(doc, container_node_id, out), _ => { // Internal table boxes can receive direct text from malformed or @@ -737,6 +737,74 @@ pub(crate) fn collect_layout_children( } } +/// Turn `container_node_id` into a table root and give it the flattened grid of +/// cells as its layout children. +fn make_table_root(doc: &mut BaseDocument, container_node_id: NodeId, out: &mut LayoutChildren) { + let (table_context, tlayout_children) = build_table_context(doc, container_node_id); + #[allow(clippy::arc_with_non_send_sync)] + let data = SpecialElementData::TableRoot(Arc::new(table_context)); + doc.nodes[container_node_id] + .flags + .insert(NodeFlags::IS_TABLE_ROOT); + doc.nodes[container_node_id] + .data + .downcast_element_mut() + .unwrap() + .special_data = data; + if let Some(before) = doc.nodes[container_node_id].before() { + out.push(before, doc); + } + out.extend(&tlayout_children, doc); + if let Some(after) = doc.nodes[container_node_id].after() { + out.push(after, doc); + } +} + +/// Whether every box this container generates is a table-internal one, so the +/// container is the table its author took the `display: table` off. +fn block_contains_only_table_internals(doc: &BaseDocument, container_node_id: NodeId) -> bool { + // Not for an anonymous block: those are generated *by* this pass, and one + // of them wrapping table internals means the wrapping has already been + // decided elsewhere. + if doc.nodes[container_node_id] + .data + .downcast_element() + .is_none() + { + return false; + } + + let mut saw_table_internal = false; + for child_id in doc.nodes[container_node_id] + .layout_dom_children() + .iter() + .copied() + { + let child = &doc.nodes[child_id]; + if child.data.kind() == NodeKind::Comment || child.is_whitespace_node() { + continue; + } + let Some(display) = child.display_style() else { + // A text child: real content, so this is a block with a stray table + // box in it and not a table. + return false; + }; + match display.inside() { + DisplayInside::None => {} + DisplayInside::TableRowGroup + | DisplayInside::TableHeaderGroup + | DisplayInside::TableFooterGroup + | DisplayInside::TableRow + | DisplayInside::TableCell + | DisplayInside::TableColumn + | DisplayInside::TableColumnGroup => saw_table_internal = true, + _ => return false, + } + } + + saw_table_internal +} + /// Extract the text generated by a pseudo-element's `content` property /// (only string content items are currently supported). fn pe_content_text(style: &style::properties::ComputedValues) -> Option<&str> { diff --git a/tests/blitz-tests/tests/display_block_table.rs b/tests/blitz-tests/tests/display_block_table.rs new file mode 100644 index 00000000..5d398b43 --- /dev/null +++ b/tests/blitz-tests/tests/display_block_table.rs @@ -0,0 +1,94 @@ +//! `table { display: block }` still lays its rows out as a table. +//! +//! This is the standard wide-table horizontal-scroll pattern, and it made every +//! row render SIDE BY SIDE: the header row on one line, then six data rows all +//! at the same y and at increasing x. With the table no longer a table, its +//! `thead` and `tbody` were plain children of a block, and their own displays +//! (`table-header-group`, `table-row-group`) have no mapping in the style +//! conversion, so they fell through to Taffy's default, which is flex. A flex +//! container lays its items out in a row. +//! +//! CSS 2.1 17.2.1 requires an anonymous table box to be generated around +//! misparented table-internal boxes instead. + +use blitz_dom::DocumentConfig; +use blitz_html::{HtmlDocument, HtmlProvider}; +use blitz_traits::shell::{ColorScheme, Viewport}; +use std::sync::Arc; + +const HTML: &str = r#" + + +
+ + + + + + +
NameSize
alpha1
beta2
+
+ +"#; + +fn origin(doc: &HtmlDocument, selector: &str) -> (f32, f32) { + let node_id = doc + .query_selector(selector) + .unwrap() + .unwrap_or_else(|| panic!("no node matching {selector}")); + let position = doc.get_node(node_id).unwrap().absolute_position(0.0, 0.0); + (position.x, position.y) +} + +#[test] +fn rows_stack_when_the_table_is_display_block() { + let mut doc = HtmlDocument::from_html( + HTML, + DocumentConfig { + viewport: Some(Viewport::new(800, 600, 1.0, ColorScheme::Light)), + html_parser_provider: Some(Arc::new(HtmlProvider) as _), + ..Default::default() + }, + ); + doc.resolve(0.0); + + let head = origin(&doc, "#h1"); + let first = origin(&doc, "#a1"); + let second = origin(&doc, "#b1"); + + assert!( + head.1 < first.1 && first.1 < second.1, + "rows must stack, not sit side by side: header {head:?} first {first:?} second {second:?}" + ); + assert_eq!( + (head.0, first.0), + (first.0, second.0), + "the first cell of every row starts at the same x" + ); +} + +#[test] +fn cells_sit_side_by_side_when_the_table_is_display_block() { + let mut doc = HtmlDocument::from_html( + HTML, + DocumentConfig { + viewport: Some(Viewport::new(800, 600, 1.0, ColorScheme::Light)), + html_parser_provider: Some(Arc::new(HtmlProvider) as _), + ..Default::default() + }, + ); + doc.resolve(0.0); + + let left = origin(&doc, "#a1"); + let right = origin(&doc, "#a2"); + + assert_eq!(left.1, right.1, "cells in one row share a baseline"); + assert!( + right.0 > left.0, + "the second cell is to the right of the first: {left:?} {right:?}" + ); +} From e6287bd0fcc5448a9cc210ceeffd8067740f12a7 Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 9 Sep 2026 20:00:22 +0700 Subject: [PATCH 14/18] release: 0.4.7 The QA fixes in this branch are what downstream needs, and nothing can ask for them until they are released. The branch originally carried a bump to 0.4.2; master reached that version independently and has since released 0.4.6, so the same intent is now a bump to 0.4.7. Thirteen commits sit above the 0.4.6 release commit and none of them are on crates.io: 0fbbf52b size input[type=number] like the other text inputs 01b484ee prune removed nodes from a stacking context before resolving it 594d2986 make implicit form submission safe and spec-correct 3bcf5f82 reuse ElementData::is_submit_button for the default button afcb61e0 expose Event.cancelBubble 29954674 expose Element.scrollIntoView bddcd96c give window.location assign, replace and reload 44de0c7f dispatch popstate on a history traversal, and give the window dispatchEvent e405cc3e fire change on a text control that loses focus having been edited fc4eee80 give live selectedness and put it in the semantic tree There was no notion of selectedness anywhere in the engine. An