diff --git a/packages/blitz-dom/src/accessibility.rs b/packages/blitz-dom/src/accessibility.rs index cbd5aefc..9b0421c5 100644 --- a/packages/blitz-dom/src/accessibility.rs +++ b/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); diff --git a/packages/blitz-dom/src/document.rs b/packages/blitz-dom/src/document.rs index 13d26253..37208a89 100644 --- a/packages/blitz-dom/src/document.rs +++ b/packages/blitz-dom/src/document.rs @@ -255,6 +255,41 @@ pub struct BaseDocument { /// paints beneath every background between them and disappears. pub(crate) hoisted_fixed_parents: HashMap, + /// Every `position: fixed` node whose containing block is the viewport, + /// which is every one of them except those under a transformed ancestor. + /// + /// Collected by the walk that hoists them, and used by + /// `resolve_fixed_positions` to hold them still while the page scrolls. + pub(crate) fixed_nodes: Vec, + + /// The viewport scroll currently baked into those nodes' locations. + /// + /// One value for the whole document rather than one per node: the pin is + /// the same displacement for every fixed box, because they all share the + /// viewport as their containing block. Reset by `resolve_layout`, which + /// rewrites the locations it was added to. + pub(crate) fixed_scroll_offset: crate::Point, + + /// Every `position: sticky` node in the document, in tree order. + /// + /// Collected by the same walk that hoists fixed nodes, because both need + /// one pre-order pass over the box tree and a second one would cost the + /// same on every frame of every document, sticky or not. Tree order matters: + /// a sticky box inside another sticky box is adjusted on top of its + /// ancestor's adjustment, so the ancestor has to be settled first. + pub(crate) sticky_nodes: Vec, + + /// For each sticky node, the offset currently baked into its + /// `final_layout().location`. + /// + /// The adjustment is written into the box itself so that paint, hit testing + /// and `absolute_position` cannot disagree about where the box is. That + /// makes the pass non-idempotent unless it can recover the flow position it + /// started from, which is what this records. Cleared by `resolve_layout`, + /// which rewrites every location from taffy and so discards the offsets + /// along with them. + pub(crate) sticky_offsets: HashMap>, + /// Stacking contexts holding a hoisted child that an ancestor clips. /// /// Collected while flushing styles so that `resolve_hoisted_clips` visits @@ -529,6 +564,10 @@ impl BaseDocument { let mut doc = Self { hoisted_fixed_parents: HashMap::new(), + fixed_nodes: Vec::new(), + fixed_scroll_offset: crate::Point::ZERO, + sticky_nodes: Vec::new(), + sticky_offsets: HashMap::new(), hoisted_clip_hosts: Vec::new(), id, tx, @@ -2589,7 +2628,27 @@ impl BaseDocument { /// Scroll a node by given x and y /// Will bubble scrolling up to parent node once it can no longer scroll further /// If we're already at the root node, bubbles scrolling up to the viewport + /// + /// A `position: sticky` box is held against the edge of the scrollport it + /// lives in, so the boxes have to be re-adjusted here rather than only in + /// `resolve`: a wheel event does not necessarily produce a style and layout + /// pass, and a header that only unstuck on the next restyle is a header + /// that visibly lags the scroll. pub fn scroll_node_by_has_changed( + &mut self, + node_id: NodeId, + x: f64, + y: f64, + dispatch_event: F, + ) -> bool { + let has_changed = self.scroll_node_by_inner(node_id, x, y, dispatch_event); + if has_changed { + self.resolve_sticky_positions(); + } + has_changed + } + + fn scroll_node_by_inner( &mut self, node_id: NodeId, x: f64, @@ -2653,7 +2712,7 @@ impl BaseDocument { if bubble_x != 0.0 || bubble_y != 0.0 { let bubbled = if let Some(parent) = parent { - self.scroll_node_by_has_changed(parent, bubble_x, bubble_y, dispatch_event) + self.scroll_node_by_inner(parent, bubble_x, bubble_y, dispatch_event) } else { self.scroll_viewport_by_has_changed(bubble_x, bubble_y) }; @@ -2743,7 +2802,7 @@ impl BaseDocument { if bubble_x != 0.0 || bubble_y != 0.0 { if let Some(parent) = parent { - return self.scroll_node_by_has_changed(parent, bubble_x, bubble_y, dispatch_event) + return self.scroll_node_by_inner(parent, bubble_x, bubble_y, dispatch_event) | has_changed; } else { return self.scroll_viewport_by_has_changed(bubble_x, bubble_y) | has_changed; @@ -2783,7 +2842,16 @@ impl BaseDocument { self.viewport_scroll.y = f64::max(0.0, f64::min(new_scroll.1, content_height - window_height)); - self.viewport_scroll != initial + let has_changed = self.viewport_scroll != initial; + if has_changed { + // The viewport is the scrollport a page-level sticky box is held + // against, and the containing block a fixed box is pinned to, so + // both move with this and not with the next relayout. See + // `resolve_sticky_positions` and `resolve_fixed_positions`. + self.resolve_sticky_positions(); + self.resolve_fixed_positions(); + } + has_changed } pub fn scroll_by( diff --git a/packages/blitz-dom/src/events/keyboard.rs b/packages/blitz-dom/src/events/keyboard.rs index fd6dae68..f1bdcda0 100644 --- a/packages/blitz-dom/src/events/keyboard.rs +++ b/packages/blitz-dom/src/events/keyboard.rs @@ -86,6 +86,25 @@ pub(crate) fn handle_key_or_input_event( 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( } } +/// 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( + 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( &mut self, diff --git a/packages/blitz-dom/src/events/mod.rs b/packages/blitz-dom/src/events/mod.rs index b1f0409d..d4c279ff 100644 --- a/packages/blitz-dom/src/events/mod.rs +++ b/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. diff --git a/packages/blitz-dom/src/events/pointer.rs b/packages/blitz-dom/src/events/pointer.rs index 09d63e03..4bd3fd50 100644 --- a/packages/blitz-dom/src/events/pointer.rs +++ b/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 of a
element toggles // the details' `open` attribute (expand/collapse). local_name!("summary") => { diff --git a/packages/blitz-dom/src/form.rs b/packages/blitz-dom/src/form.rs index 86f5a66f..e0e09202 100644 --- a/packages/blitz-dom/src/form.rs +++ b/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") diff --git a/packages/blitz-dom/src/layout/construct.rs b/packages/blitz-dom/src/layout/construct.rs index 101f0d08..1e37f020 100644 --- a/packages/blitz-dom/src/layout/construct.rs +++ b/packages/blitz-dom/src/layout/construct.rs @@ -501,6 +501,14 @@ pub(crate) fn collect_layout_children( } } + // A select has no in-flow content of its own: `option { display: none }` + // in the user-agent sheet sees to that, so returning here costs nothing + // and keeps the options out of the box the control occupies. + if tag_name == "select" { + create_select(doc, container_node_id); + return; + } + #[cfg(feature = "svg")] if matches!(tag_name, "svg") { // Serialised rather than `outer_html`, so that symbols referenced @@ -1118,6 +1126,24 @@ fn create_checkbox_input(doc: &mut BaseDocument, input_element_id: NodeId) { } } +fn create_select(doc: &mut BaseDocument, select_element_id: NodeId) { + // Read before the node is borrowed mutably: the seed comes from the + // options, which are other nodes. + let initial = doc.initial_select_data(select_element_id); + let option_count = initial.len(); + + let node = &mut doc.nodes[select_element_id]; + let element = &mut node.data.downcast_element_mut().unwrap(); + match element.special_data { + // Construction runs again on every resolve. Re-seeding would put the + // control back to its parsed state on the next frame, so a selection + // made by the user or by script would survive exactly until anything + // else on the page changed. Only the option count is refreshed. + SpecialElementData::Select(ref mut data) => data.resize(option_count), + _ => element.special_data = SpecialElementData::Select(initial), + } +} + /// Find and return the "layout_children" (inline boxes) for an inline layout /// without actually constructing the layout. This allows us to defer the expensive /// construction of the Parley layout (which invokes text shaping) to a paralell phase. diff --git a/packages/blitz-dom/src/layout/mod.rs b/packages/blitz-dom/src/layout/mod.rs index 56bf5097..49dc1236 100644 --- a/packages/blitz-dom/src/layout/mod.rs +++ b/packages/blitz-dom/src/layout/mod.rs @@ -338,6 +338,15 @@ impl BaseDocument { /// sheet sees to that, and nothing replaces it. Without a measure of its own it /// laid out at zero and no site's country picker, currency picker or language /// picker had a box to press. +/// +/// Still a character count rather than shaped text, because the options never +/// reach layout and so are never shaped. What it counts is now the option's +/// *label*, which is the string the control actually shows: a `label` +/// attribute overrides the element's text, and the text is collapsed the way it +/// would be rendered. Counting `text_content().trim()` measured the hidden text +/// of a labelled option, and `trim()` only strips the ends, so a label whose own +/// words were split across source lines counted the newline and the indentation +/// before the next word and came out that much too wide. fn select_metrics_of( doc: &BaseDocument, node_id: blitz_traits::node_id::NodeId, @@ -348,14 +357,10 @@ fn select_metrics_of( return None; } - let widest = crate::traversal::TreeTraverser::new_with_root(doc, node_id) - .filter_map(|descendant_id| doc.nodes.get(descendant_id)) - .filter(|descendant| { - descendant - .data - .is_element_with_tag_name(&local_name!("option")) - }) - .map(|option| option.text_content().trim().chars().count()) + let widest = doc + .select_options(node_id) + .into_iter() + .map(|option_id| doc.option_label(option_id).chars().count()) .max() .unwrap_or(0); diff --git a/packages/blitz-dom/src/lib.rs b/packages/blitz-dom/src/lib.rs index 6b83f9c8..70570688 100644 --- a/packages/blitz-dom/src/lib.rs +++ b/packages/blitz-dom/src/lib.rs @@ -53,6 +53,7 @@ mod mutator; pub mod paint_damage; mod query_selector; mod resolve; +mod select; mod selection; #[cfg(feature = "shadow-dom")] mod shadow; @@ -98,7 +99,7 @@ pub use markup5ever::{ namespace_prefix, namespace_url, ns, }; pub use mutator::DocumentMutator; -pub use node::{Attribute, DocumentData, ElementData, Node, NodeData, TextNodeData}; +pub use node::{Attribute, DocumentData, ElementData, Node, NodeData, SelectData, TextNodeData}; pub use paint_damage::PaintDamage; // Re-exported because `PaintDamage` takes and returns `kurbo::Rect` across the // crate boundary. A consumer that pulls kurbo in itself and lands on a diff --git a/packages/blitz-dom/src/mutator.rs b/packages/blitz-dom/src/mutator.rs index 4b6ef2c0..84dc62fd 100644 --- a/packages/blitz-dom/src/mutator.rs +++ b/packages/blitz-dom/src/mutator.rs @@ -497,6 +497,49 @@ impl DocumentMutator<'_> { || (tag, attr) == tag_and_attr!("iframe", "srcdoc") { self.load_iframe(node_id); + } else if (tag, attr) == tag_and_attr!("option", "selected") { + // `selected` is an HTML boolean attribute: present means selected, + // whatever the value reads. The same trap `checked` fell into, where + // `selected="false"` selected the option. + // + // Selectedness lives on the owning select once that has been + // constructed, and construction is idempotent, so writing the + // attribute alone would land nowhere anything reads. Before + // construction the attribute is the only carrier and seeds the + // state on the next resolve, which is why this is allowed to do + // nothing at all. + self.set_option_selected_state(node_id, true); + } + } + + /// Push an option's selectedness into the owning select's live state, if + /// that state exists yet. + fn set_option_selected_state(&mut self, option_id: NodeId, selected: bool) { + let Some(select_id) = self.doc.option_owner_select(option_id) else { + return; + }; + let Some(index) = self + .doc + .select_options(select_id) + .iter() + .position(|id| *id == option_id) + else { + return; + }; + let changed = if selected { + self.doc.set_select_selected_index(select_id, index) + } else { + self.doc + .get_node_mut(select_id) + .and_then(|node| node.data.downcast_element_mut()) + .and_then(|el| el.select_data_mut()) + .is_some_and(|data| data.set_selected(index, false)) + }; + if changed { + // `option:checked` is matched from this state, so the restyle has + // to be asked for here or the change is invisible to CSS. + self.doc.snapshot_node(option_id); + self.doc.snapshot_node(select_id); } } @@ -1392,6 +1435,7 @@ impl<'doc> DocumentMutator<'doc> { SpecialElementData::TableRoot(_) => {} SpecialElementData::TextInput(_) => {} SpecialElementData::CheckboxInput(_) => {} + SpecialElementData::Select(_) => {} #[cfg(feature = "file-input")] SpecialElementData::FileInput(_) => {} SpecialElementData::None => {} diff --git a/packages/blitz-dom/src/node/element.rs b/packages/blitz-dom/src/node/element.rs index 15075fb8..49061f72 100644 --- a/packages/blitz-dom/src/node/element.rs +++ b/packages/blitz-dom/src/node/element.rs @@ -32,7 +32,7 @@ use super::stylo_data::StyloData; use super::{Attribute, Attributes}; use crate::Document; use crate::layout::table::TableContext; -use crate::node::{TextBrush, TextInputData, TextLayout}; +use crate::node::{SelectData, TextBrush, TextInputData, TextLayout}; #[cfg(feature = "shadow-dom")] use super::custom_element::CustomElementData; @@ -383,6 +383,7 @@ pub enum SpecialElementType { TableRoot, TextInput, CheckboxInput, + Select, #[cfg(feature = "file-input")] FileInput, #[default] @@ -412,6 +413,8 @@ pub enum SpecialElementData { TextInput(TextInputData), /// Checkbox checked state CheckboxInput(bool), + /// A \ element's selectedness and open state + Select(SelectData), /// Selected files #[cfg(feature = "file-input")] FileInput(FileData), @@ -434,6 +437,7 @@ impl Clone for SpecialElementData { Self::TableRoot(data) => Self::TableRoot(data.clone()), Self::TextInput(data) => Self::TextInput(data.clone()), Self::CheckboxInput(data) => Self::CheckboxInput(*data), + Self::Select(data) => Self::Select(data.clone()), #[cfg(feature = "file-input")] Self::FileInput(data) => Self::FileInput(data.clone()), Self::None => Self::None, @@ -682,6 +686,27 @@ impl ElementData { } } + /// The live state of a `` element. + +/// The selectedness of a `` can +/// have any number of options selected at once, and a single index would make +/// the multiple case unrepresentable rather than merely unsupported. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct SelectData { + selected: Vec, + open: bool, +} + +impl SelectData { + /// Seed the state with one entry per option, in the select's list order. + pub fn new(selected: Vec) -> Self { + Self { + selected, + open: false, + } + } + + /// Grow or shrink to `len` options, keeping the selectedness of the options + /// that are still there. + /// + /// Layout construction runs again on every resolve, so this is the only + /// place a script-added `