From 8a90cda09f3bcb7ffc40eb456aefddb2495f1ff6 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Wed, 11 Jan 2017 23:29:56 +0000 Subject: [PATCH 1/3] Do not replace an ellipsis when reflowing a line The ellipsis should be replaced, if needed, when the text itself is reflowed. --- components/layout/fragment.rs | 15 ++++++++++----- components/layout/inline.rs | 11 +++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index 9ac181c432a..09cebfae17f 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -764,14 +764,17 @@ impl Fragment { text_overflow_string: String) -> Fragment { let mut unscanned_ellipsis_fragments = LinkedList::new(); - unscanned_ellipsis_fragments.push_back(self.transform( - self.border_box.size, - SpecificFragmentInfo::UnscannedText( - box UnscannedTextFragmentInfo::new(text_overflow_string, None)))); + let mut ellipsis_fragment = self.transform( + self.border_box.size, + SpecificFragmentInfo::UnscannedText( + box UnscannedTextFragmentInfo::new(text_overflow_string, None))); + unscanned_ellipsis_fragments.push_back(ellipsis_fragment); let ellipsis_fragments = TextRunScanner::new().scan_for_runs(&mut layout_context.font_context(), unscanned_ellipsis_fragments); debug_assert!(ellipsis_fragments.len() == 1); - ellipsis_fragments.fragments.into_iter().next().unwrap() + ellipsis_fragment = ellipsis_fragments.fragments.into_iter().next().unwrap(); + ellipsis_fragment.flags |= IS_ELLIPSIS; + ellipsis_fragment } pub fn restyle_damage(&self) -> RestyleDamage { @@ -2997,6 +3000,8 @@ bitflags! { const IS_INLINE_FLEX_ITEM = 0b0000_0001, /// Whether this fragment represents a child in a column flex container. const IS_BLOCK_FLEX_ITEM = 0b0000_0010, + /// Whether this fragment represents the generated text from a text-overflow clip. + const IS_ELLIPSIS = 0b0000_0100, } } diff --git a/components/layout/inline.rs b/components/layout/inline.rs index 2d591b1ee02..47e1da7b2bb 100644 --- a/components/layout/inline.rs +++ b/components/layout/inline.rs @@ -15,6 +15,7 @@ use flow::{CONTAINS_TEXT_OR_REPLACED_FRAGMENTS, EarlyAbsolutePositionInfo, Mutab use flow::OpaqueFlow; use flow_ref::FlowRef; use fragment::{CoordinateSystem, Fragment, FragmentBorderBoxIterator, Overflow}; +use fragment::IS_ELLIPSIS; use fragment::SpecificFragmentInfo; use gfx::display_list::OpaqueNode; use gfx::font::FontMetrics; @@ -331,6 +332,16 @@ impl LineBreaker { None => break, Some(fragment) => fragment, }; + + // If ellipsis are still needed, then they were already needed for the previous fragment. + if fragment.flags.contains(IS_ELLIPSIS) { + continue + } + + // If ellipsis are still needed, then they were already needed for the previous fragment. + if fragment.flags.contains(IS_ELLIPSIS) { + continue + } // Try to append the fragment. self.reflow_fragment(fragment, flow, layout_context); From cef4ebed2088ea5e4c457eba34fbd48f00fefaff Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 12 Jan 2017 18:34:36 +0000 Subject: [PATCH 2/3] Preserve the un-truncated version of fragments Fixes #14952 --- components/layout/display_list_builder.rs | 15 ++- components/layout/fragment.rs | 146 ++++++++++++++++++++-- components/layout/inline.rs | 25 ++-- 3 files changed, 154 insertions(+), 32 deletions(-) diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index 4c7e29866c7..8d9e6547c4d 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -19,7 +19,7 @@ use flex::FlexFlow; use flow::{BaseFlow, Flow, IS_ABSOLUTELY_POSITIONED}; use flow_ref::FlowRef; use fragment::{CoordinateSystem, Fragment, ImageFragmentInfo, ScannedTextFragmentInfo}; -use fragment::SpecificFragmentInfo; +use fragment::{SpecificFragmentInfo, TruncatedFragmentInfo}; use gfx::display_list::{BLUR_INFLATION_FACTOR, BaseDisplayItem, BorderDisplayItem}; use gfx::display_list::{BorderRadii, BoxShadowClipMode, BoxShadowDisplayItem, ClippingRegion}; use gfx::display_list::{DisplayItem, DisplayItemMetadata, DisplayList, DisplayListSection}; @@ -1402,7 +1402,11 @@ impl FragmentDisplayListBuilding for Fragment { self.stacking_relative_content_box(stacking_relative_border_box); match self.specific { - SpecificFragmentInfo::ScannedText(ref text_fragment) => { + SpecificFragmentInfo::TruncatedFragment(box TruncatedFragmentInfo { + text_info: Some(ref text_fragment), + .. + }) | + SpecificFragmentInfo::ScannedText(box ref text_fragment) => { // Create items for shadows. // // NB: According to CSS-BACKGROUNDS, text shadows render in *reverse* order (front @@ -1410,7 +1414,7 @@ impl FragmentDisplayListBuilding for Fragment { for text_shadow in self.style.get_inheritedtext().text_shadow.0.iter().rev() { self.build_display_list_for_text_fragment(state, - &**text_fragment, + &*text_fragment, &stacking_relative_content_box, Some(text_shadow), clip); @@ -1418,7 +1422,7 @@ impl FragmentDisplayListBuilding for Fragment { // Create the main text display item. self.build_display_list_for_text_fragment(state, - &**text_fragment, + &*text_fragment, &stacking_relative_content_box, None, clip); @@ -1428,7 +1432,7 @@ impl FragmentDisplayListBuilding for Fragment { self.style(), stacking_relative_border_box, &stacking_relative_content_box, - &**text_fragment, + &*text_fragment, clip); } } @@ -1443,6 +1447,7 @@ impl FragmentDisplayListBuilding for Fragment { SpecificFragmentInfo::InlineBlock(_) | SpecificFragmentInfo::InlineAbsoluteHypothetical(_) | SpecificFragmentInfo::InlineAbsolute(_) | + SpecificFragmentInfo::TruncatedFragment(_) | SpecificFragmentInfo::Svg(_) => { if opts::get().show_debug_fragment_borders { self.build_debug_borders_around_fragment(state, diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index 09cebfae17f..558abe47b37 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -186,6 +186,12 @@ pub enum SpecificFragmentInfo { Multicol, MulticolColumn, UnscannedText(Box), + + /// A container for a fragment that got truncated by text-overflow. + /// "Totally truncated fragments" are not rendered at all. + /// Text fragments may be partially truncated (in which case this renders like a text fragment). + /// Other fragments can only be totally truncated or not truncated at all. + TruncatedFragment(Box), } impl SpecificFragmentInfo { @@ -206,6 +212,7 @@ impl SpecificFragmentInfo { SpecificFragmentInfo::Multicol | SpecificFragmentInfo::MulticolColumn | SpecificFragmentInfo::UnscannedText(_) | + SpecificFragmentInfo::TruncatedFragment(_) | SpecificFragmentInfo::Generic => return RestyleDamage::empty(), SpecificFragmentInfo::InlineAbsoluteHypothetical(ref info) => &info.flow_ref, SpecificFragmentInfo::InlineAbsolute(ref info) => &info.flow_ref, @@ -237,6 +244,7 @@ impl SpecificFragmentInfo { SpecificFragmentInfo::Multicol => "SpecificFragmentInfo::Multicol", SpecificFragmentInfo::MulticolColumn => "SpecificFragmentInfo::MulticolColumn", SpecificFragmentInfo::UnscannedText(_) => "SpecificFragmentInfo::UnscannedText", + SpecificFragmentInfo::TruncatedFragment(_) => "SpecificFragmentInfo::TruncatedFragment" } } } @@ -573,11 +581,11 @@ pub struct SplitResult { } /// Describes how a fragment should be truncated. -pub struct TruncationResult { +struct TruncationResult { /// The part of the fragment remaining after truncation. - pub split: SplitInfo, + split: SplitInfo, /// The text run which is being truncated. - pub text_run: Arc, + text_run: Arc, } /// Data for an unscanned text fragment. Unscanned text fragments are the results of flow @@ -622,6 +630,15 @@ impl TableColumnFragmentInfo { } } +/// A wrapper for fragments that have been truncated by the `text-overflow` property. +/// This may have an associated text node, or, if the fragment was completely truncated, +/// it may act as an invisible marker for incremental reflow. +#[derive(Clone)] +pub struct TruncatedFragmentInfo { + pub text_info: Option, + pub full: Fragment, +} + impl Fragment { /// Constructs a new `Fragment` instance. pub fn new(node: &N, specific: SpecificFragmentInfo, ctx: &LayoutContext) -> Fragment { @@ -846,6 +863,7 @@ impl Fragment { base_quantities } } + SpecificFragmentInfo::TruncatedFragment(_) | SpecificFragmentInfo::ScannedText(_) | SpecificFragmentInfo::TableColumn(_) | SpecificFragmentInfo::UnscannedText(_) | @@ -1501,7 +1519,11 @@ impl Fragment { }); } - SpecificFragmentInfo::ScannedText(ref text_fragment_info) => { + SpecificFragmentInfo::TruncatedFragment(box TruncatedFragmentInfo { + text_info: Some(ref text_fragment_info), + .. + }) | + SpecificFragmentInfo::ScannedText(box ref text_fragment_info) => { let range = &text_fragment_info.range; // See http://dev.w3.org/csswg/css-sizing/#max-content-inline-size. @@ -1521,6 +1543,12 @@ impl Fragment { preferred_inline_size: max_line_inline_size, }) } + + SpecificFragmentInfo::TruncatedFragment(box TruncatedFragmentInfo { + text_info: None, + .. + }) => return IntrinsicISizesContribution::new(), + SpecificFragmentInfo::UnscannedText(..) => { panic!("Unscanned text fragments should have been scanned by now!") } @@ -1562,7 +1590,11 @@ impl Fragment { /// this fragment.) pub fn minimum_splittable_inline_size(&self) -> Au { match self.specific { - SpecificFragmentInfo::ScannedText(ref text) => { + SpecificFragmentInfo::TruncatedFragment(box TruncatedFragmentInfo { + text_info: Some(ref text), + .. + }) | + SpecificFragmentInfo::ScannedText(box ref text) => { text.run.minimum_splittable_inline_size(&text.range) } _ => Au(0), @@ -1623,9 +1655,55 @@ impl Fragment { } } + /// Truncates this fragment to the given `max_inline_size`, using a character-based breaking + /// strategy. The resulting fragment will have `SpecificFragmentInfo::TruncatedFragment`, + /// preserving the original fragment for use in incremental reflow. + /// + /// This function will panic if self is already truncated. + pub fn truncate_to_inline_size(self, max_inline_size: Au) -> Fragment { + if let SpecificFragmentInfo::TruncatedFragment(_) = self.specific { + panic!("Cannot truncate an already truncated fragment"); + } + let info = self.calculate_truncate_to_inline_size(max_inline_size); + let (size, text_info) = match info { + Some(TruncationResult { split: SplitInfo { inline_size, range }, text_run } ) => { + let size = LogicalSize::new(self.style.writing_mode, + inline_size, + self.border_box.size.block); + // Preserve the insertion point if it is in this fragment's range or it is at line end. + let (flags, insertion_point) = match self.specific { + SpecificFragmentInfo::ScannedText(ref info) => { + match info.insertion_point { + Some(index) if range.contains(index) => (info.flags, info.insertion_point), + Some(index) if index == ByteIndex(text_run.text.chars().count() as isize - 1) && + index == range.end() => (info.flags, info.insertion_point), + _ => (info.flags, None) + } + }, + _ => (ScannedTextFlags::empty(), None) + }; + let text_info = ScannedTextFragmentInfo::new( + text_run, + range, + size, + insertion_point, + flags); + (size, Some(text_info)) + } + None => + (LogicalSize::zero(self.style.writing_mode), None) + }; + let mut result = self.transform(size, SpecificFragmentInfo::Generic); + result.specific = SpecificFragmentInfo::TruncatedFragment(box TruncatedFragmentInfo { + text_info: text_info, + full: self, + }); + result + } + /// Truncates this fragment to the given `max_inline_size`, using a character-based breaking /// strategy. If no characters could fit, returns `None`. - pub fn truncate_to_inline_size(&self, max_inline_size: Au) -> Option { + fn calculate_truncate_to_inline_size(&self, max_inline_size: Au) -> Option { let text_fragment_info = if let SpecificFragmentInfo::ScannedText(ref text_fragment_info) = self.specific { text_fragment_info @@ -1821,6 +1899,10 @@ impl Fragment { container_inline_size: Au, container_block_size: Option) { match self.specific { + SpecificFragmentInfo::TruncatedFragment(box TruncatedFragmentInfo { + text_info: None, + .. + }) | SpecificFragmentInfo::Generic | SpecificFragmentInfo::GeneratedContent(_) | SpecificFragmentInfo::Table | @@ -1842,6 +1924,7 @@ impl Fragment { SpecificFragmentInfo::InlineAbsoluteHypothetical(_) | SpecificFragmentInfo::InlineAbsolute(_) | SpecificFragmentInfo::ScannedText(_) | + SpecificFragmentInfo::TruncatedFragment(_) | SpecificFragmentInfo::Svg(_) => {} }; @@ -1873,7 +1956,11 @@ impl Fragment { } // Text - SpecificFragmentInfo::ScannedText(ref info) => { + SpecificFragmentInfo::TruncatedFragment(box TruncatedFragmentInfo { + text_info: Some(ref info), + .. + }) | + SpecificFragmentInfo::ScannedText(box ref info) => { // Scanned text fragments will have already had their content inline-sizes assigned // by this point. self.border_box.size.inline = info.content_size.inline + @@ -1898,6 +1985,10 @@ impl Fragment { /// Ideally, this should follow CSS 2.1 ยง 10.6.2. pub fn assign_replaced_block_size_if_necessary(&mut self) { match self.specific { + SpecificFragmentInfo::TruncatedFragment(box TruncatedFragmentInfo { + text_info: None, + .. + }) | SpecificFragmentInfo::Generic | SpecificFragmentInfo::GeneratedContent(_) | SpecificFragmentInfo::Table | @@ -1919,12 +2010,20 @@ impl Fragment { SpecificFragmentInfo::InlineAbsoluteHypothetical(_) | SpecificFragmentInfo::InlineAbsolute(_) | SpecificFragmentInfo::ScannedText(_) | + SpecificFragmentInfo::TruncatedFragment(box TruncatedFragmentInfo { + text_info: Some(_), + .. + }) | SpecificFragmentInfo::Svg(_) => {} } match self.specific { // Text - SpecificFragmentInfo::ScannedText(ref info) => { + SpecificFragmentInfo::TruncatedFragment(box TruncatedFragmentInfo { + text_info: Some(ref info), + .. + }) | + SpecificFragmentInfo::ScannedText(box ref info) => { // Scanned text fragments' content block-sizes are calculated by the text run // scanner during flow construction. self.border_box.size.block = info.content_size.block + @@ -2001,7 +2100,11 @@ impl Fragment { ascent: ascent, } } - SpecificFragmentInfo::ScannedText(ref info) => { + SpecificFragmentInfo::TruncatedFragment(box TruncatedFragmentInfo { + text_info: Some(ref info), + .. + }) | + SpecificFragmentInfo::ScannedText(box ref info) => { // Fragments with no glyphs don't contribute any inline metrics. // TODO: Filter out these fragments during flow construction? if info.insertion_point.is_none() && info.content_size.inline == Au(0) { @@ -2019,6 +2122,10 @@ impl Fragment { SpecificFragmentInfo::InlineAbsoluteHypothetical(ref info) => { inline_metrics_of_block(&info.flow_ref, &*self.style) } + SpecificFragmentInfo::TruncatedFragment(box TruncatedFragmentInfo { + text_info: None, + .. + }) | SpecificFragmentInfo::InlineAbsolute(_) => { InlineMetrics::new(Au(0), Au(0), Au(0)) } @@ -2267,6 +2374,7 @@ impl Fragment { SpecificFragmentInfo::TableCell | SpecificFragmentInfo::TableColumn(_) | SpecificFragmentInfo::TableRow | + SpecificFragmentInfo::TruncatedFragment(_) | SpecificFragmentInfo::Multicol | SpecificFragmentInfo::UnscannedText(_) => true, } @@ -2354,6 +2462,7 @@ impl Fragment { pub fn establishes_stacking_context(&self) -> bool { // Text fragments shouldn't create stacking contexts. match self.specific { + SpecificFragmentInfo::TruncatedFragment(_) | SpecificFragmentInfo::ScannedText(_) | SpecificFragmentInfo::UnscannedText(_) => return false, _ => {} @@ -2484,7 +2593,11 @@ impl Fragment { pub fn requires_line_break_afterward_if_wrapping_on_newlines(&self) -> bool { match self.specific { - SpecificFragmentInfo::ScannedText(ref scanned_text) => { + SpecificFragmentInfo::TruncatedFragment(box TruncatedFragmentInfo { + text_info: Some(ref scanned_text), + .. + }) | + SpecificFragmentInfo::ScannedText(box ref scanned_text) => { scanned_text.requires_line_break_afterward_if_wrapping_on_newlines() } _ => false, @@ -2497,7 +2610,11 @@ impl Fragment { } match self.specific { - SpecificFragmentInfo::ScannedText(ref mut scanned_text_fragment_info) => { + SpecificFragmentInfo::TruncatedFragment(box TruncatedFragmentInfo { + text_info: Some(ref mut scanned_text_fragment_info), + .. + }) | + SpecificFragmentInfo::ScannedText(box ref mut scanned_text_fragment_info) => { let leading_whitespace_byte_count = scanned_text_fragment_info.text() .find(|c| !char_is_whitespace(c)) .unwrap_or(scanned_text_fragment_info.text().len()); @@ -2551,7 +2668,11 @@ impl Fragment { } match self.specific { - SpecificFragmentInfo::ScannedText(ref mut scanned_text_fragment_info) => { + SpecificFragmentInfo::TruncatedFragment(box TruncatedFragmentInfo { + text_info: Some(ref mut scanned_text_fragment_info), + .. + }) | + SpecificFragmentInfo::ScannedText(box ref mut scanned_text_fragment_info) => { let mut trailing_whitespace_start_byte = 0; for (i, c) in scanned_text_fragment_info.text().char_indices().rev() { if !char_is_whitespace(c) { @@ -2737,6 +2858,7 @@ impl Fragment { SpecificFragmentInfo::Iframe(_) | SpecificFragmentInfo::Image(_) | SpecificFragmentInfo::ScannedText(_) | + SpecificFragmentInfo::TruncatedFragment(_) | SpecificFragmentInfo::Svg(_) | SpecificFragmentInfo::UnscannedText(_) => true } diff --git a/components/layout/inline.rs b/components/layout/inline.rs index 47e1da7b2bb..8a086d34320 100644 --- a/components/layout/inline.rs +++ b/components/layout/inline.rs @@ -332,16 +332,15 @@ impl LineBreaker { None => break, Some(fragment) => fragment, }; - - // If ellipsis are still needed, then they were already needed for the previous fragment. - if fragment.flags.contains(IS_ELLIPSIS) { - continue - } - // If ellipsis are still needed, then they were already needed for the previous fragment. - if fragment.flags.contains(IS_ELLIPSIS) { + // Do not reflow truncated fragments. Reflow the original fragment only. + let fragment = if fragment.flags.contains(IS_ELLIPSIS) { continue - } + } else if let SpecificFragmentInfo::TruncatedFragment(info) = fragment.specific { + info.full + } else { + fragment + }; // Try to append the fragment. self.reflow_fragment(fragment, flow, layout_context); @@ -718,13 +717,9 @@ impl LineBreaker { if let Some(string) = ellipsis { let ellipsis = fragment.transform_into_ellipsis(layout_context, string); - if let Some(truncation_info) = - fragment.truncate_to_inline_size(available_inline_size - - ellipsis.margin_box_inline_size()) { - let fragment = fragment.transform_with_split_info(&truncation_info.split, - truncation_info.text_run); - self.push_fragment_to_line_ignoring_text_overflow(fragment, layout_context); - } + let truncated = fragment.truncate_to_inline_size(available_inline_size - + ellipsis.margin_box_inline_size()); + self.push_fragment_to_line_ignoring_text_overflow(truncated, layout_context); self.push_fragment_to_line_ignoring_text_overflow(ellipsis, layout_context); } else { self.push_fragment_to_line_ignoring_text_overflow(fragment, layout_context); From d4ea380440153ebc06b79646262801ea553e2b83 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 12 Jan 2017 20:03:00 +0000 Subject: [PATCH 3/3] Add tests for #14952 --- tests/wpt/mozilla/meta/MANIFEST.json | 24 +++++++++++++++++++ .../tests/css/text_overflow_reflow.html | 15 ++++++++++++ .../tests/css/text_overflow_reflow_ref.html | 5 ++++ 3 files changed, 44 insertions(+) create mode 100644 tests/wpt/mozilla/tests/css/text_overflow_reflow.html create mode 100644 tests/wpt/mozilla/tests/css/text_overflow_reflow_ref.html diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 5b3faa75be7..3f3b41a79ea 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -5544,6 +5544,18 @@ "url": "/_mozilla/css/text_overflow_ellipsis.html" } ], + "css/text_overflow_reflow.html": [ + { + "path": "css/text_overflow_reflow.html", + "references": [ + [ + "/_mozilla/css/text_overflow_reflow_ref.html", + "==" + ] + ], + "url": "/_mozilla/css/text_overflow_reflow.html" + } + ], "css/text_overflow_string.html": [ { "path": "css/text_overflow_string.html", @@ -20874,6 +20886,18 @@ "url": "/_mozilla/css/text_overflow_ellipsis.html" } ], + "css/text_overflow_reflow.html": [ + { + "path": "css/text_overflow_reflow.html", + "references": [ + [ + "/_mozilla/css/text_overflow_reflow_ref.html", + "==" + ] + ], + "url": "/_mozilla/css/text_overflow_reflow.html" + } + ], "css/text_overflow_string.html": [ { "path": "css/text_overflow_string.html", diff --git a/tests/wpt/mozilla/tests/css/text_overflow_reflow.html b/tests/wpt/mozilla/tests/css/text_overflow_reflow.html new file mode 100644 index 00000000000..958e2826fa4 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/text_overflow_reflow.html @@ -0,0 +1,15 @@ + + + +Text overflow should disappear when the container becomes large enough + + + +

XXXXXXXXXX

+ diff --git a/tests/wpt/mozilla/tests/css/text_overflow_reflow_ref.html b/tests/wpt/mozilla/tests/css/text_overflow_reflow_ref.html new file mode 100644 index 00000000000..3997908406b --- /dev/null +++ b/tests/wpt/mozilla/tests/css/text_overflow_reflow_ref.html @@ -0,0 +1,5 @@ + + +Text overflow should disappear when the container becomes large enough + +

XXXXXXXXXX