diff --git a/Cargo.toml b/Cargo.toml index 6fd813c4..498ff402 100644 --- a/Cargo.toml +++ b/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" diff --git a/packages/blitz-dom/assets/default.css b/packages/blitz-dom/assets/default.css index 1681772a..ff8f5d43 100644 --- a/packages/blitz-dom/assets/default.css +++ b/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 ``. `None` for anything else. +/// +/// A select has no in-flow content: `option { display: none }` in the user-agent +/// 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, +) -> Option<(usize, f32)> { + let node = doc.nodes.get(node_id)?; + let element = node.data.downcast_element()?; + if element.name.local != local_name!("select") { + return None; + } + + let widest = doc + .select_options(node_id) + .into_iter() + .map(|option_id| doc.option_label(option_id).chars().count()) + .max() + .unwrap_or(0); + + // A dropdown shows one row. `size` names the row count for a list box, and + // `multiple` without `size` shows four, which is what browsers settled on. + let rows = element + .attr(local_name!("size")) + .and_then(|size| size.parse::().ok()) + .filter(|rows| *rows >= 1.0) + .unwrap_or(if element.attr(local_name!("multiple")).is_some() { + 4.0 + } else { + 1.0 + }); + + Some((widest, rows)) +} + impl BaseDocument { + fn select_metrics(&self, node_id: blitz_traits::node_id::NodeId) -> Option<(usize, f32)> { + select_metrics_of(self, node_id) + } + fn compute_child_layout_internal( &mut self, node_id: NodeId, @@ -344,6 +396,11 @@ impl BaseDocument { // radius separates them, and a cache hit never reaches this function. #[cfg(feature = "log-phase-times")] layout_counters::note_computed(dom_node_id(node_id)); + + // Read before the node is borrowed mutably: a `` is measured from its options, which are not in + // flow. The character-count estimate is the same one the + // `cols` attribute of a textarea uses below: a select's label + // is not laid out as text anywhere yet, so there is no real + // measurement to take. An authored width or height still wins, + // this only supplies the content size. + if let Some((widest_label, rows)) = select_metrics { + let advance = font_size.unwrap_or(16.0) * 0.6; + let line_height = resolved_line_height.unwrap_or(16.0); + return compute_leaf_layout( + inputs, + node.style(), + resolve_calc_value, + |_known_size, _available_space| taffy::Size { + width: widest_label as f32 * advance, + height: line_height * rows, + }, + ); + } + // TODO: deduplicate with single-line text input if *element_data.name.local == *"textarea" { let rows = element_data @@ -519,7 +596,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/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/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 `