Rescue: <select> support found unpushed in a temp directory

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/accessibility.rs

@@ -136,6 +136,33 @@ impl BaseDocument {
             };
 
             builder.set_role(role);
+
+            /*
+             * A select and its options carried their roles and nothing else, so
+             * the tree said "there is a combo box here" and stopped. What it
+             * offers, what is chosen, and which option that is were all absent,
+             * which is exactly the set of questions a QA harness asks of a
+             * picker before it can drive one.
+             *
+             * The options are already in the tree: the traversal walks raw
+             * children, so `option { display: none }` does not hide them. Only
+             * the state was missing.
+             */
+            match &*name {
+                "select" => {
+                    builder.set_value(self.select_label(node.id));
+                }
+                "option" => {
+                    // An explicit label, rather than relying on the text child
+                    // labelling its parent: an option's text is `display: none`
+                    // and a consumer that resolves names through the child text
+                    // runs would be reading a node that never gets laid out.
+                    builder.set_label(self.option_label(node.id));
+                    builder.set_selected(self.option_is_selected(node.id));
+                }
+                _ => {}
+            }
+
             builder.set_html_tag(name);
         } else if node.is_text_node() {
             builder.set_role(Role::TextRun);

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

@@ -86,6 +86,25 @@ pub(crate) fn handle_key_or_input_event<F: FnMut(DomEvent)>(
             return;
         }
 
+        // A focussed select owns the arrows, Home and End: they move the
+        // selection. Checked before the mutable borrow below, because deciding
+        // where a select's selection goes means reading its options, which are
+        // other nodes.
+        //
+        // There was no keyboard activation for anything but a text input, so a
+        // picker could be focussed and then not driven at all. A harness has no
+        // other way in: worktables.dev's schema designer selects its column
+        // type by keyboard or not at all.
+        if doc
+            .get_node(node_id)
+            .is_some_and(|node| node.data.is_element_with_tag_name(&local_name!("select")))
+        {
+            if let KeyboardOrTextInputEvent::KeyPress(key_event) = &event {
+                handle_select_keypress(doc, node_id, key_event, dispatch_event);
+            }
+            return;
+        }
+
         let node = &mut doc.nodes[node_id];
         let Some(element_data) = node.element_data_mut() else {
             return;
@@ -116,6 +135,49 @@ pub(crate) fn handle_key_or_input_event<F: FnMut(DomEvent)>(
     }
 }
 
+/// Move a focussed select's selection with the arrows, Home and End.
+///
+/// A select commits immediately, the way a checkbox does and unlike a text
+/// control: the keystroke *is* the commit, so `input` fires here and
+/// blitz-script synthesises the `change` that follows it. Waiting for blur, as
+/// a text field does, would leave a picker that never reports what was chosen.
+fn handle_select_keypress<F: FnMut(DomEvent)>(
+    doc: &mut BaseDocument,
+    node_id: NodeId,
+    event: &BlitzKeyEvent,
+    mut dispatch_event: F,
+) {
+    if !event.state.is_pressed() {
+        return;
+    }
+
+    let target_index = match &event.key {
+        Key::ArrowDown => doc.select_index_step(node_id, true),
+        Key::ArrowUp => doc.select_index_step(node_id, false),
+        Key::Home => doc.select_index_edge(node_id, false),
+        Key::End => doc.select_index_edge(node_id, true),
+        _ => return,
+    };
+    let Some(index) = target_index else {
+        // Already at the end of the list, or nothing selectable. Silently
+        // doing nothing is right: a browser does not wrap around.
+        return;
+    };
+
+    if !doc.set_select_selected_index(node_id, index) {
+        return;
+    }
+    // The selectedness is what `:checked` and the accessibility tree read, so
+    // the restyle has to be asked for or the change is invisible.
+    doc.snapshot_node(node_id);
+
+    let value = doc.select_value(node_id);
+    dispatch_event(DomEvent::new(
+        node_id,
+        DomEventData::Input(BlitzInputEvent { value }),
+    ));
+}
+
 impl BaseDocument {
     pub(crate) fn apply_generated_text_input_event<F: FnMut(DomEvent)>(
         &mut self,
@@ -150,20 +212,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 +301,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/events/mod.rs

@@ -371,7 +371,14 @@ fn scroll_key_is_claimed_by(doc: &BaseDocument, node_id: NodeId, key: &Key) -> b
     }
     doc.get_node(node_id)
         .and_then(|node| node.element_data())
-        .is_some_and(|element| element.text_input_data().is_some())
+        .is_some_and(|element| {
+            element.text_input_data().is_some()
+                // A focussed select moves its selection with the arrows, Home
+                // and End. Without claiming them here the branch above scrolls
+                // the page and *returns*, so the keyboard handler never runs
+                // and a select could not be driven by keyboard at all.
+                || element.select_data().is_some()
+        })
 }
 
 /// The scroll a key asks for, in CSS pixels, or `None` if it asks for none.

packages/blitz-dom/src/events/pointer.rs

@@ -638,6 +638,12 @@ pub(crate) fn checkable_activation_target(doc: &BaseDocument, target: NodeId) ->
                 return Some(node_id);
             }
             local_name!("label") => return None,
+            // A select is not checkable and has an activation behaviour of its
+            // own, so the walk stops here for the same reason it stops at a
+            // text input: `handle_click` claims the press, and the two have to
+            // agree about where the walk ends or a checkbox wrapping a select
+            // would toggle without ever seeing a click.
+            local_name!("select") => return None,
             _ => {}
         }
 
@@ -716,6 +722,27 @@ pub(crate) fn handle_click(
 
                     break 'matched true;
                 }
+                // A press on a select focuses it. Nothing else yet: there is no
+                // popup to open.
+                //
+                // Without an arm of its own the walk fell through to the
+                // no-match tail below, which calls `clear_focus()`. Clicking a
+                // select therefore actively *unfocused* the page, and since the
+                // keyboard handler is gated on focus, the arrows could not
+                // drive a select the user had just pressed. `handle_pointerdown`
+                // is no help either: it classifies a select as
+                // `ClickTarget::SelectableText` and only the `TextInput` arm
+                // there generates focus events.
+                local_name!("select") => {
+                    generate_focus_events(
+                        doc,
+                        &mut |doc| {
+                            doc.set_focus_to(node_id);
+                        },
+                        dispatch_event,
+                    );
+                    break 'matched true;
+                }
                 // Activating the first <summary> of a <details> element toggles
                 // the details' `open` attribute (expand/collapse).
                 local_name!("summary") => {

packages/blitz-dom/src/form.rs

@@ -267,11 +267,29 @@ fn construct_entry_list(doc: &BaseDocument, form_id: NodeId, submitter_id: NodeI
             continue;
         };
 
-        // TODO: If the field element is a select element,
-        //  then for each option element in the select element's
-        //  list of options whose selectedness is true and that is not disabled,
-        //  create an entry with name and the value of the option element,
-        //  and append it to entry list.
+        // If the field element is a select element, then for each option
+        // element in the select element's list of options whose selectedness is
+        // true and that is not disabled, create an entry with name and the
+        // value of the option element, and append it to entry list.
+        //
+        // Without this a select fell through to the generic tail below and
+        // submitted its own literal `value` attribute, which a select does not
+        // have. Every form containing a picker posted the wrong body.
+        if element.name.local == local_name!("select") {
+            let options = doc.select_options(control_id);
+            for (index, option_id) in options.iter().enumerate() {
+                let is_selected = element
+                    .select_data()
+                    .map(|data| data.is_selected(index))
+                    // Before layout construction has run there is no live
+                    // state, and the content attribute is all there is.
+                    .unwrap_or_else(|| doc.option_is_selected(*option_id));
+                if is_selected && !doc.option_is_disabled(*option_id) {
+                    create_entry(name, doc.option_value(*option_id).as_str().into());
+                }
+            }
+            continue;
+        }
 
         // Otherwise, if the field element is an input element whose type attribute is in the Checkbox state or the Radio Button state, then:
         if element.name.local == local_name!("input")