From 383ba964aba4ef74913a719a4f9cd2c49abc9241 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 10 Jul 2016 00:53:37 -0700 Subject: [PATCH 1/2] layout: text: Don't consider the REQUIRES_LINE_BREAK_AFTERWARD_IF_WRAPPING_ON_NEWLINES if the fragment can wrap text. This was making us fall in a loop where the start split was empty, but we didn't ignore it because the current fragment had this flag, but then we treated it differently depending on the white_space property. Not totally sure this is the proper fix, but it makes sense to me. In case it is: Fixes #12369. --- components/layout/fragment.rs | 25 ++++++++++++------------- components/layout/inline.rs | 8 +++++--- components/layout/text.rs | 11 +++++------ 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index 4561a1bf5a2..8092985f401 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -1541,12 +1541,11 @@ impl Fragment { /// information are both optional due to the possibility of them being whitespace. pub fn calculate_split_position(&self, max_inline_size: Au, starts_line: bool) -> Option { - let text_fragment_info = - if let SpecificFragmentInfo::ScannedText(ref text_fragment_info) = self.specific { - text_fragment_info - } else { - return None - }; + let text_fragment_info = match self.specific { + SpecificFragmentInfo::ScannedText(ref text_fragment_info) + => text_fragment_info, + _ => return None, + }; let mut flags = SplitOptions::empty(); if starts_line { @@ -1618,12 +1617,11 @@ impl Fragment { flags: SplitOptions) -> Option where I: Iterator> { - let text_fragment_info = - if let SpecificFragmentInfo::ScannedText(ref text_fragment_info) = self.specific { - text_fragment_info - } else { - return None - }; + let text_fragment_info = match self.specific { + SpecificFragmentInfo::ScannedText(ref text_fragment_info) + => text_fragment_info, + _ => return None, + }; let mut remaining_inline_size = max_inline_size; let mut inline_start_range = Range::new(text_fragment_info.range.begin(), ByteIndex(0)); @@ -1661,7 +1659,8 @@ impl Fragment { // see if we're going to overflow the line. If so, perform a best-effort split. let mut remaining_range = slice.text_run_range(); let split_is_empty = inline_start_range.is_empty() && - !self.requires_line_break_afterward_if_wrapping_on_newlines(); + !(self.requires_line_break_afterward_if_wrapping_on_newlines() && + !self.white_space().allow_wrap()); if split_is_empty { // We're going to overflow the line. overflowing = true; diff --git a/components/layout/inline.rs b/components/layout/inline.rs index 6195024f2ca..5d7cbffb970 100644 --- a/components/layout/inline.rs +++ b/components/layout/inline.rs @@ -459,10 +459,12 @@ impl LineBreaker { kind: FloatKind::Left, }); + let fragment_margin_box_inline_size = first_fragment.margin_box_inline_size(); + // Simple case: if the fragment fits, then we can stop here. - if line_bounds.size.inline > first_fragment.margin_box_inline_size() { + if line_bounds.size.inline > fragment_margin_box_inline_size { debug!("LineBreaker: fragment fits on line {}", self.lines.len()); - return (line_bounds, first_fragment.margin_box_inline_size()); + return (line_bounds, fragment_margin_box_inline_size); } // If not, but we can't split the fragment, then we'll place the line here and it will @@ -471,7 +473,7 @@ impl LineBreaker { debug!("LineBreaker: line doesn't fit, but is unsplittable"); } - (line_bounds, first_fragment.margin_box_inline_size()) + (line_bounds, fragment_margin_box_inline_size) } /// Performs float collision avoidance. This is called when adding a fragment is going to diff --git a/components/layout/text.rs b/components/layout/text.rs index 250ad9d5e73..2b60fb1ee74 100644 --- a/components/layout/text.rs +++ b/components/layout/text.rs @@ -340,22 +340,21 @@ impl TextRunScanner { let mut byte_range = Range::new(ByteIndex(mapping.byte_range.begin() as isize), ByteIndex(mapping.byte_range.length() as isize)); + let mut flags = ScannedTextFlags::empty(); + let text_size = old_fragment.border_box.size; + let requires_line_break_afterward_if_wrapping_on_newlines = scanned_run.run.text[mapping.byte_range.begin()..mapping.byte_range.end()] .ends_with('\n'); + if requires_line_break_afterward_if_wrapping_on_newlines { byte_range.extend_by(ByteIndex(-1)); // Trim the '\n' + flags.insert(REQUIRES_LINE_BREAK_AFTERWARD_IF_WRAPPING_ON_NEWLINES); } - let text_size = old_fragment.border_box.size; - - let mut flags = ScannedTextFlags::empty(); if mapping.selected { flags.insert(SELECTED); } - if requires_line_break_afterward_if_wrapping_on_newlines { - flags.insert(REQUIRES_LINE_BREAK_AFTERWARD_IF_WRAPPING_ON_NEWLINES); - } let insertion_point = if mapping.contains_insertion_point(scanned_run.insertion_point) { scanned_run.insertion_point From b54b57620292f0695b0446ea48a6186b639a9658 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 10 Jul 2016 01:07:21 -0700 Subject: [PATCH 2/2] layout: Add test for pre-line and pre-wrap wrapping not crashing servo. --- tests/wpt/mozilla/meta/MANIFEST.json | 12 +++++++++++ .../css/white-space-pre-line-long-line.html | 20 +++++++++++++++++++ .../css/white-space-pre-wrap-long-line.html | 20 +++++++++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 tests/wpt/mozilla/tests/css/white-space-pre-line-long-line.html create mode 100644 tests/wpt/mozilla/tests/css/white-space-pre-wrap-long-line.html diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 94676a48fdc..94481deec7e 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -6138,6 +6138,18 @@ "url": "/_mozilla/css/test_variable_serialization_specified.html" } ], + "css/white-space-pre-line-long-line.html": [ + { + "path": "css/white-space-pre-line-long-line.html", + "url": "/_mozilla/css/white-space-pre-line-long-line.html" + } + ], + "css/white-space-pre-wrap-long-line.html": [ + { + "path": "css/white-space-pre-wrap-long-line.html", + "url": "/_mozilla/css/white-space-pre-wrap-long-line.html" + } + ], "mozilla/DOMParser.html": [ { "path": "mozilla/DOMParser.html", diff --git a/tests/wpt/mozilla/tests/css/white-space-pre-line-long-line.html b/tests/wpt/mozilla/tests/css/white-space-pre-line-long-line.html new file mode 100644 index 00000000000..bf0d0085fef --- /dev/null +++ b/tests/wpt/mozilla/tests/css/white-space-pre-line-long-line.html @@ -0,0 +1,20 @@ + + +White space pre-line crashtest (#12369) + + + +
This is an actual loooooooong line +Line 2
+ diff --git a/tests/wpt/mozilla/tests/css/white-space-pre-wrap-long-line.html b/tests/wpt/mozilla/tests/css/white-space-pre-wrap-long-line.html new file mode 100644 index 00000000000..f965a31a209 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/white-space-pre-wrap-long-line.html @@ -0,0 +1,20 @@ + + +White space pre-wrap crashtest (#12369) + + + +
This is an actual loooooooong line +Line 2
+