From 121df3a6d30d3c437a09c39d2e79ea4792d506a2 Mon Sep 17 00:00:00 2001 From: kevin Date: Fri, 18 Sep 2026 07:18:03 +0800 Subject: [PATCH] fix(layout): exclude collapsed line-end spaces from overflow --- moli-layout/src/inline.rs | 70 +++++++++- moli-layout/tests/phase5_output_contract.rs | 134 +++++++++++++++++++- 2 files changed, 199 insertions(+), 5 deletions(-) diff --git a/moli-layout/src/inline.rs b/moli-layout/src/inline.rs index bb9a03586..669dd6280 100644 --- a/moli-layout/src/inline.rs +++ b/moli-layout/src/inline.rs @@ -99,6 +99,9 @@ pub(crate) struct InlineTextUnit { pub(crate) ancestors: Vec, pub(crate) sources: Vec, pub(crate) control: bool, + /// A collapsed CSS space, removable again at a wrapped line boundary. + /// Preserved spaces and non-breaking spaces must retain their geometry. + pub(crate) collapsed_space: bool, pub(crate) break_spaces_opportunity: bool, } @@ -449,10 +452,28 @@ pub(crate) fn build_inline_fragments( for (line_index, line) in layout.lines().enumerate() { let metrics = line.metrics(); + let line_range = line.text_range(); + let trailing_start = overlapping_output_ranges(&context.text_units, &line_range) + .iter() + .rev() + .take_while(|unit| unit.control || unit.collapsed_space) + .filter(|unit| unit.collapsed_space) + .map(|unit| unit.output_range.start) + .last(); + let mut collapsed_trailing_advance = 0.0; + if let Some(start) = trailing_start { + for run in line.runs() { + for cluster in run.visual_clusters() { + if cluster.text_range().start >= start { + collapsed_trailing_advance += cluster.advance().max(0.0); + } + } + } + } let placement = line_placements .get(line_index) .filter(|placement| placement.line_index == line_index); - let line_rect = placement.map_or_else( + let mut line_rect = placement.map_or_else( || { PaintRect::new( metrics.inline_min_coord + metrics.offset, @@ -463,6 +484,14 @@ pub(crate) fn build_inline_fragments( }, |placement| placement.rect, ); + // Parley retains hanging spaces for shaping/line breaking. CSS + // collapsed line-end spaces contribute neither range/inline-box + // geometry nor scrollable overflow. Do not trim preserved or NBSP + // advances merely because Parley classifies them as whitespace. + line_rect.width = (line_rect.width - collapsed_trailing_advance).max(0.0); + if layout.is_rtl() { + line_rect.x += collapsed_trailing_advance; + } fragments.lines.push(InlineLineFragment { line_index, rect: line_rect, @@ -487,6 +516,9 @@ pub(crate) fn build_inline_fragments( let run_metrics = run.metrics(); for cluster in run.visual_clusters() { let range = cluster.text_range(); + if trailing_start.is_some_and(|start| range.start >= start) { + continue; + } let style_index = cluster .glyphs() .next() @@ -2258,6 +2290,7 @@ impl InlineNormalizer { ancestors: pending.ancestors, sources: pending.sources, control: false, + collapsed_space: true, break_spaces_opportunity: false, }, ); @@ -2359,6 +2392,7 @@ impl InlineNormalizer { ancestors: ancestors.to_vec(), sources, control, + collapsed_space: false, break_spaces_opportunity: false, }); } @@ -2747,6 +2781,40 @@ mod tests { normalizer.finish() } + #[test] + fn only_collapsed_spaces_are_marked_for_line_end_removal() { + let text = LayoutBoxId::from_index(1); + for mode in [ + InlineWhiteSpaceCollapse::Collapse, + InlineWhiteSpaceCollapse::PreserveBreaks, + InlineWhiteSpaceCollapse::Preserve, + InlineWhiteSpaceCollapse::BreakSpaces, + ] { + let input = normalize(&[(text, "A \u{a0} B")], mode, InlineTextTransform::None); + let collapsed = input + .units + .iter() + .filter(|unit| unit.collapsed_space) + .collect::>(); + assert_eq!( + collapsed.len(), + if matches!( + mode, + InlineWhiteSpaceCollapse::Collapse | InlineWhiteSpaceCollapse::PreserveBreaks + ) { + 2 + } else { + 0 + } + ); + assert!( + collapsed + .iter() + .all(|unit| &input.text[unit.output_range.clone()] == " ") + ); + } + } + #[test] fn preserve_merges_crlf_across_adjacent_text_nodes_with_both_origins() { let first = LayoutBoxId::from_index(1); diff --git a/moli-layout/tests/phase5_output_contract.rs b/moli-layout/tests/phase5_output_contract.rs index c72577e99..d877144e0 100644 --- a/moli-layout/tests/phase5_output_contract.rs +++ b/moli-layout/tests/phase5_output_contract.rs @@ -940,12 +940,45 @@ fn local_scrollbar_feedback_invalidates_only_the_changed_subtree_and_ancestor_pa } } +fn fixed_inline_font() -> (ResolvedLayoutStyle, DocumentLayoutServices) { + use style::values::computed::font::{ + FamilyName, FontFamily, FontFamilyList, FontFamilyNameSyntax, SingleFontFamily, + }; + let mut font = style::properties::style_structs::Font::initial_values(); + font.set_font_family(FontFamily { + families: FontFamilyList { + list: style::ArcSlice::from_iter(std::iter::once(SingleFontFamily::FamilyName( + FamilyName { + name: Atom::from("Moli Ahem"), + syntax: FontFamilyNameSyntax::Quoted, + }, + ))), + }, + is_system_font: false, + is_initial: false, + }); + let style = ResolvedLayoutStyle::from_stylo( + style::properties::ComputedValues::initial_values_with_font_override(font), + ); + let mut services = + DocumentLayoutServices::with_system_font_policy(moli_layout::SystemFontPolicy::Disabled); + services + .register_web_font(moli_layout::WebFontRegistration::new( + "fixed", + moli_layout::WebFontFace::new("Moli Ahem"), + include_bytes!("fixtures/moli-ahem.ttf").to_vec(), + )) + .unwrap(); + (style, services) +} + #[test] fn scrollbar_feedback_rebreaks_the_reused_inline_layout_at_its_final_width() { const TEXT: &str = "alpha beta gamma delta epsilon zeta eta theta iota kappa"; let source = Source(vec![ Node::element("root", vec![1]), Node::element("scroller", vec![2]), + Node::element("fixed-font", vec![3]), Node::text("text", TEXT), ]); let mut styles = Styles::default(); @@ -972,7 +1005,17 @@ fn scrollbar_feedback_rebreaks_the_reused_inline_layout_at_its_final_width() { ), ); - let feedback = build(&source, &mut styles); + // The old system-font fixture happened to fit on macOS but exposed + // hanging-space overflow on Linux. Bind shaping to the same test face. + let (font_style, mut fixed_services) = fixed_inline_font(); + styles.0.insert(2, font_style); + let feedback = build_layout_pass( + &source, + &mut styles, + &mut fixed_services, + LayoutPassRequest::new(LayoutViewport::new(320, 240, 1.0), LayoutFlushReason::Test), + ) + .unwrap(); assert_eq!(feedback.metrics.numeric_layout_pass_count, 2); assert_eq!( feedback.element_metrics_for_source(1).unwrap().client_size, @@ -982,7 +1025,11 @@ fn scrollbar_feedback_rebreaks_the_reused_inline_layout_at_its_final_width() { let extent = feedback.scroll_extent(scroller_box).unwrap(); assert!(extent.vertical_scrollbar.is_some()); assert!(extent.horizontal_scrollbar.is_none()); - let feedback_text = feedback.client_rects_for_source(2); + let feedback_text = feedback.text_range_rects(3, 0..TEXT.encode_utf16().count()); + assert!( + !feedback_text.is_empty(), + "compare real text fragments, not element-only client rects" + ); // Lay out the same paragraph directly at the converged 85px content // width. Its line fragments must match the scrollbar-corrected result; @@ -1000,8 +1047,87 @@ fn scrollbar_feedback_rebreaks_the_reused_inline_layout_at_its_final_width() { }, ), ); - let direct = build(&source, &mut styles); - assert_eq!(feedback_text, direct.client_rects_for_source(2)); + let direct = build_layout_pass( + &source, + &mut styles, + &mut fixed_services, + LayoutPassRequest::new(LayoutViewport::new(320, 240, 1.0), LayoutFlushReason::Test), + ) + .unwrap(); + assert_eq!( + feedback_text, + direct.text_range_rects(3, 0..TEXT.encode_utf16().count()) + ); + assert!( + feedback_text + .iter() + .all(|quad| quad.points.iter().all(|point| point.x <= 85.0)) + ); +} + +#[test] +fn inline_scroll_overflow_keeps_unbreakable_text_and_nbsp() { + for text in ["abcdefghijklmno", "alpha\u{a0}beta\u{a0}gamma"] { + let source = Source(vec![ + Node::element("root", vec![1]), + Node::element("scroller", vec![2]), + Node::element("fixed-font", vec![3]), + Node::text("text", text), + ]); + let (font_style, mut services) = fixed_inline_font(); + let mut styles = Styles::default(); + styles + .0 + .insert(0, fixed_size(LayoutDisplay::Block, 320.0, 240.0)); + styles.0.insert( + 1, + resolved( + LayoutDisplay::Block, + Style { + size: Size { + width: length(85.0), + height: length(40.0), + }, + overflow: Point { + x: Overflow::Scroll, + y: Overflow::Scroll, + }, + ..Style::default() + }, + ), + ); + styles.0.insert(2, font_style); + let output = build_layout_pass( + &source, + &mut styles, + &mut services, + LayoutPassRequest::new(LayoutViewport::new(320, 240, 1.0), LayoutFlushReason::Test), + ) + .unwrap(); + let scroller = output.source_output(1).unwrap().principal_box.unwrap(); + assert!( + output + .scroll_extent(scroller) + .unwrap() + .horizontal_scrollbar + .is_some(), + "{text}" + ); + assert!( + output + .element_metrics_for_source(1) + .unwrap() + .scroll_size + .width + > 85.0, + "{text}" + ); + assert!( + !output + .text_range_rects(3, 0..text.encode_utf16().count()) + .is_empty() + ); + } } #[test]