diff --git a/moli-layout/src/form.rs b/moli-layout/src/form.rs index 86dd3c59e0..513ffe21d9 100644 --- a/moli-layout/src/form.rs +++ b/moli-layout/src/form.rs @@ -17,53 +17,64 @@ where let controls = (0..world.boxes.len()) .map(LayoutBoxId::from_index) .filter_map(|id| { - form_control_text(&world.boxes[id.index()].element_semantics).map(|text| (id, text)) + form_control_texts(&world.boxes[id.index()].element_semantics).map(|texts| (id, texts)) }) - .filter(|(_, text)| !text.is_empty()) + .filter(|(_, texts)| !texts.is_empty()) .collect::>(); - for (control, text) in controls { + for (control, texts) in controls { let Some(owner) = world.boxes[control.index()].source else { continue; }; + if world.boxes[control.index()] + .element_semantics + .as_ref() + .is_some_and(LayoutElementSemantics::is_select_listbox) + { + world.boxes[control.index()] + .style + .clip_form_control_contents(); + } let owner_label = world.boxes[control.index()].source_label.clone(); let wrapper_style = ResolvedLayoutStyle::anonymous_from( &world.boxes[control.index()].style, LayoutDisplay::Block, ); let text_style = ResolvedLayoutStyle::text_leaf_from(&wrapper_style); - let mut wrapper = LayoutWorld::new_box( - None, - Some(owner), - None, - format!("form-content({owner_label})"), - Some(owner_label.clone()), - None, - Some(LayoutAnonymousReason::FormControlContent), - LayoutBoxKind::AnonymousBlock, - wrapper_style, - None, - ); - wrapper.inline_formatting_context = true; - let wrapper_id = world.allocate(wrapper); - let text_id = world.allocate(LayoutWorld::new_box( - None, - Some(owner), - None, - format!("form-content({owner_label})::text"), - Some(owner_label), - None, - Some(LayoutAnonymousReason::FormControlContent), - LayoutBoxKind::Text, - text_style, - Some(Arc::from(text)), - )); - world.append_synthesized_child(wrapper_id, text_id)?; - world.append_synthesized_child(control, wrapper_id)?; + for (row, text) in texts.into_iter().enumerate() { + let mut wrapper = LayoutWorld::new_box( + None, + Some(owner), + None, + format!("form-content({owner_label})[{row}]"), + Some(owner_label.clone()), + None, + Some(LayoutAnonymousReason::FormControlContent), + LayoutBoxKind::AnonymousBlock, + wrapper_style.clone(), + None, + ); + wrapper.inline_formatting_context = true; + let wrapper_id = world.allocate(wrapper); + let text_id = world.allocate(LayoutWorld::new_box( + None, + Some(owner), + None, + format!("form-content({owner_label})[{row}]::text"), + Some(owner_label.clone()), + None, + Some(LayoutAnonymousReason::FormControlContent), + LayoutBoxKind::Text, + text_style.clone(), + Some(text), + )); + world.append_synthesized_child(wrapper_id, text_id)?; + world.append_synthesized_child(control, wrapper_id)?; + } } Ok(()) } -fn form_control_text(semantics: &Option) -> Option { +fn form_control_texts(semantics: &Option) -> Option>> { let semantics = semantics.as_ref()?; let LayoutElementCategory::FormControl(kind) = semantics.category else { return None; @@ -77,6 +88,9 @@ fn form_control_text(semantics: &Option) -> Option match input { LayoutInputControlKind::Button => data.value.to_string(), @@ -142,7 +156,15 @@ fn form_control_text(semantics: &Option) -> Option String::new(), }; - Some(text) + Some(if text.is_empty() { + Vec::new() + } else { + vec![text.into()] + }) +} + +pub(crate) fn select_row_height(font_size: f32, line_height: f32) -> f32 { + line_height.max(font_size) + 5.0 } pub(crate) fn form_control_context( @@ -229,11 +251,8 @@ pub(crate) fn form_control_context( height: f32::from(data.rows) * line_height.max(font_size), }, LayoutFormControlKind::Select => { - let rows = data - .size - .unwrap_or(if data.multiple { 4 } else { 1 }) - .max(1); - let listbox = data.multiple || rows > 1; + let rows = data.select_rows(); + let listbox = rows > 1; let characters = data .maximum_option_characters .max(u16::try_from(data.value.chars().count()).unwrap_or(u16::MAX)) @@ -241,7 +260,11 @@ pub(crate) fn form_control_context( Size { width: text_width(usize::from(characters)) + if listbox { 4.0 } else { 18.0 }, height: f32::from(rows) - * (line_height.max(font_size) + if listbox { 5.0 } else { 0.0 }), + * if listbox { + select_row_height(font_size, line_height) + } else { + single_line_height + }, } } LayoutFormControlKind::Progress | LayoutFormControlKind::Meter => Size { diff --git a/moli-layout/src/source.rs b/moli-layout/src/source.rs index 634a6bdef6..d688139012 100644 --- a/moli-layout/src/source.rs +++ b/moli-layout/src/source.rs @@ -252,6 +252,8 @@ pub struct LayoutListData { pub struct LayoutFormControlData { pub value: Arc, pub placeholder: Arc, + /// Display labels in option order, independently of selectedness. + pub option_labels: Vec>, pub size: Option, pub columns: u16, pub rows: u16, @@ -266,6 +268,7 @@ impl Default for LayoutFormControlData { Self { value: Arc::from(""), placeholder: Arc::from(""), + option_labels: Vec::new(), size: None, columns: 20, rows: 2, @@ -277,6 +280,14 @@ impl Default for LayoutFormControlData { } } +impl LayoutFormControlData { + pub(crate) fn select_rows(&self) -> u16 { + self.size + .unwrap_or(if self.multiple { 4 } else { 1 }) + .max(1) + } +} + /// Optional typed HTML metadata retained beside element classification. #[derive(Clone, Debug, Default, PartialEq, Eq, Hash)] pub struct LayoutElementMetadata { @@ -341,6 +352,17 @@ impl LayoutElementSemantics { self.replaced.is_some() } + pub(crate) fn is_select_listbox(&self) -> bool { + matches!( + self.category, + LayoutElementCategory::FormControl(LayoutFormControlKind::Select) + ) && self + .metadata + .form_control + .as_ref() + .is_some_and(|data| data.select_rows() > 1) + } + pub(crate) fn is_html_element(&self, local_name: &str) -> bool { self.namespace == LayoutNamespace::Html && self.local_name.as_ref() == local_name } diff --git a/moli-layout/src/style.rs b/moli-layout/src/style.rs index f27d3d09e9..616779a9f4 100644 --- a/moli-layout/src/style.rs +++ b/moli-layout/src/style.rs @@ -1687,6 +1687,11 @@ impl ResolvedLayoutStyle { self.overflow_clips } + /// Native controls have an internal viewport even when their CSS overflow is visible. + pub(crate) fn clip_form_control_contents(&mut self) { + self.overflow_clips = true; + } + pub(crate) fn establishes_scroll_container(&self) -> bool { // `overflow: clip` clips paint but deliberately does not create the // scrolling mechanism that selects a sticky scrollport. diff --git a/moli-layout/src/taffy_tree.rs b/moli-layout/src/taffy_tree.rs index a606b8993e..1bb3850eaf 100644 --- a/moli-layout/src/taffy_tree.rs +++ b/moli-layout/src/taffy_tree.rs @@ -673,26 +673,33 @@ fn finish_form_control_contents(world: &mut LayoutWorld) where N: Copy + Debug + Eq + Hash, { - let contents = (0..world.boxes.len()) - .map(LayoutBoxId::from_index) - .filter(|id| { + let contents = world + .boxes + .iter() + .flat_map(|layout_box| layout_box.children.iter().copied().enumerate()) + .filter(|(_, id)| { world.boxes[id.index()].anonymous_reason == Some(crate::LayoutAnonymousReason::FormControlContent) && world.boxes[id.index()].kind == LayoutBoxKind::AnonymousBlock }) .collect::>(); - for content in contents { + for (row, content) in contents { let Some(control) = world.boxes[content.index()].layout_parent else { continue; }; let control_layout = world.boxes[control.index()].unrounded_layout; let parent_writing_mode = world.boxes[control.index()].style.writing_mode(); + let listbox = world.boxes[control.index()] + .element_semantics + .as_ref() + .is_some_and(crate::LayoutElementSemantics::is_select_listbox); + let inline_padding = if listbox { 2.0 } else { 4.0 }; let content_width = (control_layout.size.width - control_layout.border.left - control_layout.border.right - control_layout.padding.left - control_layout.padding.right - - 8.0) + - 2.0 * inline_padding) .max(0.0); let inputs = LayoutInput { known_dimensions: Size::NONE, @@ -714,9 +721,17 @@ where vertical_margins_are_collapsible: Line::FALSE, }; let output = world.compute_child_layout(content.to_taffy(), inputs); - let x = control_layout.border.left + control_layout.padding.left + 4.0; - let y = ((control_layout.size.height - output.size.height) * 0.5) - .max(control_layout.border.top + control_layout.padding.top); + let x = control_layout.border.left + control_layout.padding.left + inline_padding; + let y = if listbox { + let style = &world.boxes[control.index()].style; + control_layout.border.top + + control_layout.padding.top + + row as f32 + * crate::form::select_row_height(style.font_size(), style.line_height()) + } else { + ((control_layout.size.height - output.size.height) * 0.5) + .max(control_layout.border.top + control_layout.padding.top) + }; world.set_inline_child_layout( content, Point { x, y }, diff --git a/moli-renderer-v8/src/layout_renderer/source_view.rs b/moli-renderer-v8/src/layout_renderer/source_view.rs index 2d26db0fa4..ab9e7148a4 100644 --- a/moli-renderer-v8/src/layout_renderer/source_view.rs +++ b/moli-renderer-v8/src/layout_renderer/source_view.rs @@ -465,19 +465,22 @@ fn layout_element_semantics_for_source( element: &crate::dom::native::Element, ) -> LayoutElementSemantics { let mut semantics = layout_element_semantics(element); + let mut option_labels = Vec::new(); let (selected_text, maximum_option_characters) = if element.is_html_select() { let options = host.select_option_elements(node); let option_text = |option| { host.node(option) .and_then(Node::as_element) - .and_then(|option| option.attribute("label")) - .map(str::to_owned) - .or_else(|| host.text_content(option)) + .map(|element| element.option_label(host.dom(), option)) .unwrap_or_default() }; - let maximum = options + option_labels = options .iter() - .map(|option| option_text(*option).chars().count()) + .map(|option| option_text(*option).into()) + .collect::>>(); + let maximum = option_labels + .iter() + .map(|label| label.chars().count()) .max() .unwrap_or_default(); let selected = host @@ -496,6 +499,9 @@ fn layout_element_semantics_for_source( selected_text, maximum_option_characters, ); + if let Some(data) = semantics.metadata.form_control.as_mut() { + data.option_labels = option_labels; + } semantics } @@ -550,6 +556,7 @@ fn layout_element_metadata( metadata.form_control = Some(LayoutFormControlData { value: value.into(), placeholder: element.attribute("placeholder").unwrap_or("").into(), + option_labels: Vec::new(), size: optional_positive_u16_attribute(element, "size", 1, 65_535), columns: positive_u16_attribute(element, "cols", 20, 65_535), rows: positive_u16_attribute(element, "rows", 2, 65_535), diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update.rs b/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update.rs index eda088b39c..9c7df83094 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update.rs @@ -51,6 +51,7 @@ async fn dispatch_main_document_domcontentloaded_for_rendering_test( Ok(owner) } +mod form_controls; mod layout_geometry; mod painting_tables; mod scrollbars; diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update/form_controls.rs b/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update/form_controls.rs new file mode 100644 index 0000000000..51c1e959a9 --- /dev/null +++ b/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update/form_controls.rs @@ -0,0 +1,136 @@ +use super::*; + +fn dark_pixels(image: &moli_paint::RasterImage, x: u32, y: u32, width: u32, height: u32) -> usize { + (y..y + height) + .flat_map(|y| (x..x + width).map(move |x| ((y * image.width + x) * 4) as usize)) + .filter(|&index| { + image.rgba[index..index + 3] + .iter() + .all(|&channel| channel < 128) + }) + .count() +} + +#[tokio::test(flavor = "current_thread")] +async fn screenshot_select_listboxes_paint_every_option_and_clip_their_contents() { + run_page_vm_async_test(async move { + let loader = + crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); + let mut page_vm = test_page_vm_with_loader_and_document_url( + &loader, + Vec::new(), + Url::parse("https://example.com/select-listbox.html")?, + ); + page_vm.vm_mut().eval( + r#" +document.head.innerHTML = ``; +const options = ``; +document.body.innerHTML = ` + + +`; +document.getElementById('selected').options[1].selected = true; +document.getElementById('single').value = 'b'; +'installed' +"#, + )?; + page_vm.vm_mut().sync_live_document_style_sources(); + assert_eq!( + page_vm.vm_mut().eval( + "[...document.getElementById('multi').options].map(o=>o.label).join('|') + ';' + document.getElementById('multi').selectedOptions.length", + )?, + "Alpha|Beta|Gamma;0", + "the unselected listbox must still have all three option labels", + ); + let viewport = moli_layout::PaintViewport::new(800, 120, 1.0); + let snapshot = page_vm + .vm_mut() + .screenshot_layout_snapshot(viewport)? + .expect("select fixture must retain a layout root"); + let image = moli_paint::raster_snapshot(&snapshot)?; + for (name, x) in [("multiple", 0), ("selected multiple", 160), ("size=3", 320)] { + for (row, label) in ["Alpha", "Beta", "Gamma"].into_iter().enumerate() { + assert!( + dark_pixels(&image, x + 2, 1 + row as u32 * 25, 130, 24) > 10, + "{name} must paint {label} on row {row}, independently of selectedness", + ); + } + assert_eq!( + dark_pixels(&image, x + 52, 26, 80, 24), + 0, + "{name} must use the option's label attribute rather than its longer text content", + ); + } + assert!(dark_pixels(&image, 482, 2, 130, 18) > 10); + assert_eq!(dark_pixels(&image, 482, 24, 130, 60), 0, "a dropdown paints only its selected label"); + assert!(dark_pixels(&image, 642, 2, 130, 24) > 10); + assert_eq!(dark_pixels(&image, 642, 37, 136, 70), 0, "listbox rows must be clipped vertically"); + assert_eq!(dark_pixels(&image, 781, 2, 19, 32), 0, "long option labels must be clipped horizontally"); + + assert_eq!( + page_vm.vm_mut().eval( + r#"(() => { +const select = document.getElementById('multi'); +select.options[0].label = 'Delta'; +select.options[1].remove(); +select.options[0].selected = true; +select.options[1].selected = true; +return select.value + ';' + [...select.selectedOptions].map(o=>o.label).join('|'); +})()"#, + )?, + "a;Delta|Gamma", + ); + let updated = page_vm + .vm_mut() + .screenshot_layout_snapshot(viewport)? + .expect("mutated select fixture must retain a layout root"); + let updated = moli_paint::raster_snapshot(&updated)?; + assert!(dark_pixels(&updated, 2, 1, 130, 24) > 10); + assert!(dark_pixels(&updated, 2, 26, 130, 24) > 10); + assert_eq!(dark_pixels(&updated, 2, 51, 130, 24), 0, "removing an option must remove its painted row"); + let first_row_end = (25 * image.width * 4) as usize; + assert_ne!( + image.rgba[..first_row_end], updated.rgba[..first_row_end], + "changing an option label must change the painted text", + ); + + page_vm.vm_mut().eval( + "document.getElementById('multi').add(new Option('', 'blank'), 1); 'inserted'", + )?; + let with_blank = page_vm + .vm_mut() + .screenshot_layout_snapshot(viewport)? + .expect("select with an empty option must retain a layout root"); + let with_blank = moli_paint::raster_snapshot(&with_blank)?; + assert!(dark_pixels(&with_blank, 2, 1, 130, 24) > 10); + assert_eq!(dark_pixels(&with_blank, 2, 26, 130, 24), 0, "an empty option must keep its row"); + assert!(dark_pixels(&with_blank, 2, 51, 130, 24) > 10); + + assert_eq!(page_vm.vm_mut().eval( + r#"(() => { +const select = document.getElementById('multi'); +select.options[0].label = ''; +select.size = 1; +select.value = 'c'; +select.style.height = '22px'; +return select.value + ';' + select.selectedOptions[0].label; +})()"#, + )?, "c;Gamma"); + let dropdown = page_vm + .vm_mut() + .screenshot_layout_snapshot(viewport)? + .expect("single-row multiple select must retain a layout root"); + let dropdown = moli_paint::raster_snapshot(&dropdown)?; + assert!(dark_pixels(&dropdown, 2, 2, 130, 18) > 10, "size=1 must paint the selected label even when multiple is set"); + assert_eq!(dark_pixels(&dropdown, 2, 24, 130, 60), 0); + Ok::<_, anyhow::Error>(()) + }) + .await + .expect("select listbox rendering fixture should run"); +}