Simplify control flow of whitespace handling.

3 files changed, 102 insertions, 74 deletions

diff --git a/components/layout_2020/flow/construct.rs b/components/layout_2020/flow/construct.rs
index 73aa0c151b5..5cb1fe51e73 100644
--- a/components/layout_2020/flow/construct.rs
+++ b/components/layout_2020/flow/construct.rs
@@ -294,78 +294,96 @@ where
     }
 
     fn handle_text(&mut self, info: &NodeAndStyleInfo<Node>, input: Cow<'dom, str>) {
+        // Skip any leading whitespace as dictated by the node's style.
         let white_space = info.style.get_inherited_text().white_space;
-        let (leading_whitespace, mut input) = self.handle_leading_whitespace(&input, white_space);
-        if leading_whitespace || !input.is_empty() {
-            // This text node should be pushed either to the next ongoing
-            // inline level box with the parent style of that inline level box
-            // that will be ended, or directly to the ongoing inline formatting
-            // context with the parent style of that builder.
-            let inlines = self.current_inline_level_boxes();
-
-            let mut new_text_run_contents;
-            let output;
-
-            {
-                let mut last_box = inlines.last_mut().map(|last| last.borrow_mut());
-                let last_text = last_box.as_mut().and_then(|last| match &mut **last {
-                    InlineLevelBox::TextRun(last) => Some(&mut last.text),
-                    _ => None,
-                });
+        let (preserved_leading_whitespace, mut input) =
+            self.handle_leading_whitespace(&input, white_space);
 
-                if let Some(text) = last_text {
-                    // Append to the existing text run
-                    new_text_run_contents = None;
-                    output = text;
-                } else {
-                    new_text_run_contents = Some(String::new());
-                    output = new_text_run_contents.as_mut().unwrap();
-                }
+        if !preserved_leading_whitespace && input.is_empty() {
+            return;
+        }
 
-                if leading_whitespace {
-                    output.push(' ')
-                }
+        // This text node should be pushed either to the next ongoing
+        // inline level box with the parent style of that inline level box
+        // that will be ended, or directly to the ongoing inline formatting
+        // context with the parent style of that builder.
+        let inlines = self.current_inline_level_boxes();
 
-                match (
-                    white_space.preserve_spaces(),
-                    white_space.preserve_newlines(),
-                ) {
-                    (true, true) => {
-                        output.push_str(input);
-                    },
+        let mut new_text_run_contents;
+        let output;
 
-                    (true, false) => unreachable!(),
+        {
+            let mut last_box = inlines.last_mut().map(|last| last.borrow_mut());
+            let last_text = last_box.as_mut().and_then(|last| match &mut **last {
+                InlineLevelBox::TextRun(last) => Some(&mut last.text),
+                _ => None,
+            });
+
+            if let Some(text) = last_text {
+                // Append to the existing text run
+                new_text_run_contents = None;
+                output = text;
+            } else {
+                new_text_run_contents = Some(String::new());
+                output = new_text_run_contents.as_mut().unwrap();
+            }
+
+            if preserved_leading_whitespace {
+                output.push(' ')
+            }
 
-                    (false, preserve_newlines) => loop {
-                        if let Some(i) = input.bytes().position(|b| {
-                            b.is_ascii_whitespace() && (!preserve_newlines || b != b'\n')
+            match (
+                white_space.preserve_spaces(),
+                white_space.preserve_newlines(),
+            ) {
+                // All whitespace is significant, so we don't need to transform
+                // the input at all.
+                (true, true) => {
+                    output.push_str(input);
+                },
+
+                // There are no cases in CSS where where need to preserve spaces
+                // but not newlines.
+                (true, false) => unreachable!(),
+
+                // Spaces are not significant, but newlines might be. We need
+                // to collapse non-significant whitespace as appropriate.
+                (false, preserve_newlines) => loop {
+                    // If there are any spaces that need preserving, split the string
+                    // that precedes them, collapse them into a single whitespace,
+                    // then process the remainder of the string independently.
+                    if let Some(i) = input
+                        .bytes()
+                        .position(|b| b.is_ascii_whitespace() && (!preserve_newlines || b != b'\n'))
+                    {
+                        let (non_whitespace, rest) = input.split_at(i);
+                        output.push_str(non_whitespace);
+                        output.push(' ');
+
+                        // Find the first byte that is either significant whitespace or
+                        // non-whitespace to continue processing it.
+                        if let Some(i) = rest.bytes().position(|b| {
+                            !b.is_ascii_whitespace() || (preserve_newlines && b == b'\n')
                         }) {
-                            let (non_whitespace, rest) = input.split_at(i);
-                            output.push_str(non_whitespace);
-                            output.push(' ');
-
-                            if let Some(i) = rest.bytes().position(|b| {
-                                !b.is_ascii_whitespace() || (preserve_newlines && b == b'\n')
-                            }) {
-                                input = &rest[i..];
-                            } else {
-                                break;
-                            }
+                            input = &rest[i..];
                         } else {
-                            output.push_str(input);
                             break;
                         }
-                    },
-                }
+                    } else {
+                        // No whitespace found, so no transformation is required.
+                        output.push_str(input);
+                        break;
+                    }
+                },
             }
+        }
 
-            if let Some(text) = new_text_run_contents {
-                inlines.push(ArcRefCell::new(InlineLevelBox::TextRun(TextRun {
-                    tag: Tag::from_node_and_style_info(info),
-                    parent_style: Arc::clone(&info.style),
-                    text,
-                })))
-            }
+        if let Some(text) = new_text_run_contents {
+            inlines.push(ArcRefCell::new(InlineLevelBox::TextRun(TextRun {
+                tag: Tag::from_node_and_style_info(info),
+                parent_style: Arc::clone(&info.style),
+                text,
+            })))
         }
     }
 }
diff --git a/components/layout_2020/flow/inline.rs b/components/layout_2020/flow/inline.rs
index d0b91aef506..fa79264db5a 100644
--- a/components/layout_2020/flow/inline.rs
+++ b/components/layout_2020/flow/inline.rs
@@ -757,14 +757,20 @@ impl TextRun {
             let mut advance_width = Length::zero();
             let mut last_break_opportunity = None;
             let mut force_line_break = false;
+            // Fit as many glyphs within a single line as possible.
             loop {
                 let next = runs.next();
-                if next.as_ref().map_or(true, |run| {
-                    run.glyph_store.is_whitespace() || force_line_break
-                }) {
-                    if advance_width > ifc.containing_block.inline_size - ifc.inline_position ||
-                        force_line_break
-                    {
+                // If there are no more text runs we still need to check if the last
+                // run was a forced line break
+                if next
+                    .as_ref()
+                    .map_or(true, |run| run.glyph_store.is_whitespace())
+                {
+                    // If this run exceeds the bounds of the containing block, then
+                    // we need to attempt to break the line.
+                    if advance_width > ifc.containing_block.inline_size - ifc.inline_position {
+                        // Reset the text run iterator to the last whitespace if possible,
+                        // to attempt to re-layout the most recent glyphs on a new line.
                         if let Some((len, width, iter)) = last_break_opportunity.take() {
                             glyphs.truncate(len);
                             advance_width = width;
@@ -776,18 +782,24 @@ impl TextRun {
                 if let Some(run) = next {
                     if run.glyph_store.is_whitespace() {
                         last_break_opportunity = Some((glyphs.len(), advance_width, runs.clone()));
-                        if self.text.as_bytes().get(run.range.end().to_usize() - 1) == Some(&b'\n')
-                        {
-                            force_line_break = self
-                                .parent_style
+                        // If this whitespace ends with a newline, we need to check if
+                        // it's meaningful within the current style. If so, we force
+                        // a line break immediately.
+                        let last_byte = self.text.as_bytes().get(run.range.end().to_usize() - 1);
+                        if last_byte == Some(&b'\n') &&
+                            self.parent_style
                                 .get_inherited_text()
                                 .white_space
-                                .preserve_newlines();
+                                .preserve_newlines()
+                        {
+                            force_line_break = true;
+                            break;
                         }
                     }
                     glyphs.push(run.glyph_store.clone());
                     advance_width += Length::from(run.glyph_store.total_advance());
                 } else {
+                    // No more runs, so we can end the line.
                     break;
                 }
             }
@@ -822,6 +834,7 @@ impl TextRun {
                     glyphs,
                     text_decoration_line: ifc.current_nesting_level.text_decoration_line,
                 }));
+            // If this line is being broken because of a trailing newline, we can't ignore it.
             if runs.as_slice().is_empty() && !force_line_break {
                 break;
             } else {
diff --git a/tests/wpt/metadata-layout-2020/css/cssom/serialize-values.html.ini b/tests/wpt/metadata-layout-2020/css/cssom/serialize-values.html.ini
index 21832fe8ef4..89f8f5b5a5e 100644
--- a/tests/wpt/metadata-layout-2020/css/cssom/serialize-values.html.ini
+++ b/tests/wpt/metadata-layout-2020/css/cssom/serialize-values.html.ini
@@ -254,9 +254,6 @@
   [clear: both]
     expected: FAIL
 
-  [list-style-image: url(http://localhost/)]
-    expected: FAIL
-
   [outline-width: 0px]
     expected: FAIL