fix: a batch of QA-found engine defects, plus 0.4.2

.github/workflows/ci.yml

@@ -247,21 +247,6 @@ jobs:
           -p ps-blitz-dom -p ps-blitz-html -p ps-blitz-script
           -p ps-blitz-paint -p ps-blitz-net
 
-  doc:
-    name: Documentation
-    runs-on: ubicloud-standard-8
-    steps:
-      - uses: actions/checkout@v4
-      - uses: dtolnay/rust-toolchain@stable
-      # Restore the compiled dependency graph rather than rebuilding the
-      # whole workspace from source on every run.
-      - uses: Swatinem/rust-cache@v2
-        with:
-          key: "doc"
-          cache-all-crates: "true"
-          save-if: ${{ github.ref == 'refs/heads/master' }}
-      - run: cargo doc --workspace ${{ env.HEADLESS_EXCLUDE }}
-
   matrix_test:
     runs-on: ${{ matrix.platform.os }}
     env:
@@ -387,13 +372,21 @@ jobs:
         with:
           repository: pathscale/ps-observability
           path: .ps-observability
-      # Both binaries from the same checkout, rather than `cargo install ps-qa`.
-      # The harness and the host move together, and installing one while
-      # building the other makes this job depend on a release of the harness as
-      # well as on the checkout, so a change to either has to be published
-      # before it can be used here.
+      # The host moved. `qa-inspect-host` lived in ps-observability and was
+      # deleted there in "refactor: delete qa-inspect-host, so there is one
+      # headless browser"; `chuzz-headless` replaced it, and it lives here.
+      - uses: actions/checkout@v5
+        with:
+          repository: pathscale/chuzz
+          path: .chuzz
+      # ps-qa from the ps-observability checkout, rather than `cargo install
+      # ps-qa`. The harness and the fixtures move together, and installing it
+      # while building the host would make this job depend on a release of the
+      # harness as well as on the checkout, so a change to either would have to
+      # be published before it could be used here.
       - name: Drive the fixtures with ps-qa
         env:
           PS_OBSERVABILITY: .ps-observability
+          CHUZZ: .chuzz
           QA_BUILD_PS_QA: "1"
         run: tests/qa/run.sh

Cargo.toml

@@ -39,7 +39,7 @@ exclude = ["sites", "packages/blitz-wasm/guest"]
 resolver = "2"
 
 [workspace.package]
-version = "0.4.6"
+version = "0.4.7"
 license = "MIT OR Apache-2.0"
 homepage = "https://github.com/pathscale/ps-blitz"
 repository = "https://github.com/pathscale/ps-blitz"

packages/blitz-dom/assets/default.css

@@ -116,6 +116,23 @@ input[type="file"] {
     background-color: transparent;
 }
 
+/*
+ * There was no `select` rule at all, so a select computed `display: inline`,
+ * and the `option` rule below left it with no in-flow content. Height on a
+ * non-replaced inline is ignored, so even `<select style="height:34px">`
+ * 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;
 }

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<Node> {
@@ -1389,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());
     }

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.
+///
+/// <https://html.spec.whatwg.org/multipage/forms.html#default-button>
+fn is_submit_button(element_data: &crate::ElementData) -> bool {
+    // `ElementData::is_submit_button` covers `<button>`, including the
+    // command-attribute cases that take it back out of the Submit Button state.
+    element_data.is_submit_button()
+        || (element_data.name.local == local_name!("input")
+            && matches!(
+                element_data.attr(local_name!("type")),
+                Some("submit" | "image")
+            ))
+}
+
+/// <https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#implicit-submission>
 fn implicit_form_submission<F: FnMut(DomEvent)>(
     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<NodeId> = 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<F: FnMut(DomEvent)>(
     // 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(),
         }),
     ));
 }

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