servo: Merge #7951 - Add support for `pre-wrap` and `pre-line` values for `white-space` (from eefriedman:white-space); r=pcwalton

This is mostly straightforward.  I had to modify a couple of places
which were accidentally discarding whitespace.

Fixes #1513.

This fixes some relevant tests from the CSS testsuite... but a lot of
them are either manual, or don't pass because of unrelated issues.  (For
example, white-space-mixed-002 renders correctly, but
white-space-mixed-002-ref doesn't because of a float bug.)

I'd appreciate any suggestions for how to go about adding tests for this.

Source-Repo: https://github.com/servo/servo
Source-Revision: c3ab71109ee2ffcc31b40890f4c6739d8f5b1333

servo/components/gfx/text/util.rs

@@ -38,7 +38,7 @@ pub fn transform_text(text: &str,
                     output_text.push(ch);
                 }
             }
-            text.len() > 0 && is_in_whitespace(text.char_at_reverse(0), mode)
+            false
         },
 
         CompressionMode::CompressWhitespace | CompressionMode::CompressWhitespaceNewline => {

servo/components/layout/fragment.rs

@@ -48,7 +48,7 @@ use util;
 use util::geometry::ZERO_POINT;
 use util::logical_geometry::{LogicalMargin, LogicalRect, LogicalSize, WritingMode};
 use util::range::*;
-use util::str::{is_whitespace, slice_chars};
+use util::str::slice_chars;
 use wrapper::{PseudoElementType, ThreadSafeLayoutNode};
 
 /// Fragments (`struct Fragment`) are the leaves of the layout tree. They cannot position
@@ -1248,6 +1248,36 @@ impl Fragment {
         self.style().get_inheritedtext().white_space
     }
 
+    pub fn white_space_allow_wrap(&self) -> bool {
+        match self.white_space() {
+            white_space::T::nowrap |
+            white_space::T::pre => false,
+            white_space::T::normal |
+            white_space::T::pre_wrap |
+            white_space::T::pre_line => true,
+        }
+    }
+
+    pub fn white_space_preserve_newlines(&self) -> bool {
+        match self.white_space() {
+            white_space::T::normal |
+            white_space::T::nowrap => false,
+            white_space::T::pre |
+            white_space::T::pre_wrap |
+            white_space::T::pre_line => true,
+        }
+    }
+
+    pub fn white_space_preserve_spaces(&self) -> bool {
+        match self.white_space() {
+            white_space::T::normal |
+            white_space::T::nowrap |
+            white_space::T::pre_line => false,
+            white_space::T::pre |
+            white_space::T::pre_wrap => true,
+        }
+    }
+
     /// Returns the text decoration of this fragment, according to the style of the nearest ancestor
     /// element.
     ///
@@ -1275,10 +1305,9 @@ impl Fragment {
     }
 
     /// Returns true if this element can be split. This is true for text fragments, unless
-    /// `white-space: pre` is set.
+    /// `white-space: pre` or `white-space: nowrap` is set.
     pub fn can_split(&self) -> bool {
-        self.is_scanned_text_fragment() &&
-            self.style.get_inheritedtext().white_space != white_space::T::pre
+        self.is_scanned_text_fragment() && self.white_space_allow_wrap()
     }
 
     /// Returns true if and only if this fragment is a generated content fragment.
@@ -1352,9 +1381,10 @@ impl Fragment {
                                                              .metrics_for_range(range)
                                                              .advance_width;
 
-                let min_line_inline_size = match self.style.get_inheritedtext().white_space {
-                    white_space::T::pre | white_space::T::nowrap => max_line_inline_size,
-                    white_space::T::normal => text_fragment_info.run.min_width_for_range(range),
+                let min_line_inline_size = if self.white_space_allow_wrap() {
+                    text_fragment_info.run.min_width_for_range(range)
+                } else {
+                    max_line_inline_size
                 };
 
                 result.union_block(&IntrinsicISizes {
@@ -1533,7 +1563,6 @@ impl Fragment {
                 return None
             };
 
-        let mut pieces_processed_count: u32 = 0;
         let mut remaining_inline_size = max_inline_size;
         let mut inline_start_range = Range::new(text_fragment_info.range.begin(), CharIndex(0));
         let mut inline_end_range = None;
@@ -1560,18 +1589,9 @@ impl Fragment {
             // Have we found the split point?
             if advance <= remaining_inline_size || slice.glyphs.is_whitespace() {
                 // Keep going; we haven't found the split point yet.
-                if flags.contains(STARTS_LINE) &&
-                        pieces_processed_count == 0 &&
-                        slice.glyphs.is_whitespace() {
-                    debug!("calculate_split_position_using_breaking_strategy: skipping \
-                            leading trimmable whitespace");
-                    inline_start_range.shift_by(slice.range.length());
-                } else {
-                    debug!("calculate_split_position_using_breaking_strategy: enlarging span");
-                    remaining_inline_size = remaining_inline_size - advance;
-                    inline_start_range.extend_by(slice.range.length());
-                }
-                pieces_processed_count += 1;
+                debug!("calculate_split_position_using_breaking_strategy: enlarging span");
+                remaining_inline_size = remaining_inline_size - advance;
+                inline_start_range.extend_by(slice.range.length());
                 continue
             }
 
@@ -1668,21 +1688,6 @@ impl Fragment {
         self.meld_with_next_inline_fragment(&next_fragment);
     }
 
-    /// Returns true if this fragment is an unscanned text fragment that consists entirely of
-    /// whitespace that should be stripped.
-    pub fn is_ignorable_whitespace(&self) -> bool {
-        match self.white_space() {
-            white_space::T::pre => return false,
-            white_space::T::normal | white_space::T::nowrap => {}
-        }
-        match self.specific {
-            SpecificFragmentInfo::UnscannedText(ref text_fragment_info) => {
-                is_whitespace(&text_fragment_info.text)
-            }
-            _ => false,
-        }
-    }
-
     /// Assigns replaced inline-size, padding, and margins for this fragment only if it is replaced
     /// content per CSS 2.1 § 10.3.2.
     pub fn assign_replaced_inline_size_if_necessary(&mut self, container_inline_size: Au) {
@@ -2243,7 +2248,7 @@ impl Fragment {
     }
 
     pub fn strip_leading_whitespace_if_necessary(&mut self) -> WhitespaceStrippingResult {
-        if self.style.get_inheritedtext().white_space == white_space::T::pre {
+        if self.white_space_preserve_spaces() {
             return WhitespaceStrippingResult::RetainFragment
         }
 
@@ -2306,7 +2311,7 @@ impl Fragment {
 
     /// Returns true if the entire fragment was stripped.
     pub fn strip_trailing_whitespace_if_necessary(&mut self) -> WhitespaceStrippingResult {
-        if self.style.get_inheritedtext().white_space == white_space::T::pre {
+        if self.white_space_preserve_spaces() {
             return WhitespaceStrippingResult::RetainFragment
         }
 

servo/components/layout/inline.rs

@@ -4,7 +4,7 @@
 
 #![deny(unsafe_code)]
 
-use app_units::{Au, MAX_AU};
+use app_units::Au;
 use block::{AbsoluteAssignBSizesTraversal, AbsoluteStoreOverflowTraversal};
 use context::LayoutContext;
 use display_list_builder::{FragmentDisplayListBuilding, InlineFlowDisplayListBuilding};
@@ -179,15 +179,6 @@ int_range_index! {
     struct FragmentIndex(isize)
 }
 
-bitflags! {
-    flags InlineReflowFlags: u8 {
-        #[doc = "The `white-space: nowrap` property from CSS 2.1 § 16.6 is in effect."]
-        const NO_WRAP_INLINE_REFLOW_FLAG = 0x01,
-        #[doc = "The `white-space: pre` property from CSS 2.1 § 16.6 is in effect."]
-        const WRAP_ON_NEWLINE_INLINE_REFLOW_FLAG = 0x02
-    }
-}
-
 /// Arranges fragments into lines, splitting them up as necessary.
 struct LineBreaker {
     /// The floats we need to flow around.
@@ -321,17 +312,8 @@ impl LineBreaker {
                 Some(fragment) => fragment,
             };
 
-            // Set up our reflow flags.
-            let flags = match fragment.style().get_inheritedtext().white_space {
-                white_space::T::normal => InlineReflowFlags::empty(),
-                white_space::T::nowrap => NO_WRAP_INLINE_REFLOW_FLAG,
-                white_space::T::pre => {
-                    WRAP_ON_NEWLINE_INLINE_REFLOW_FLAG | NO_WRAP_INLINE_REFLOW_FLAG
-                }
-            };
-
             // Try to append the fragment.
-            self.reflow_fragment(fragment, flow, layout_context, flags);
+            self.reflow_fragment(fragment, flow, layout_context);
         }
 
         if !self.pending_line_is_empty() {
@@ -540,8 +522,7 @@ impl LineBreaker {
     fn reflow_fragment(&mut self,
                        mut fragment: Fragment,
                        flow: &InlineFlow,
-                       layout_context: &LayoutContext,
-                       flags: InlineReflowFlags) {
+                       layout_context: &LayoutContext) {
         // Determine initial placement for the fragment if we need to.
         //
         // Also, determine whether we can legally break the line before, or inside, this fragment.
@@ -552,7 +533,7 @@ impl LineBreaker {
             self.pending_line.green_zone = line_bounds.size;
             false
         } else {
-            !flags.contains(NO_WRAP_INLINE_REFLOW_FLAG)
+            fragment.white_space_allow_wrap()
         };
 
         debug!("LineBreaker: trying to append to line {} (fragment size: {:?}, green zone: {:?}): \
@@ -582,13 +563,15 @@ impl LineBreaker {
 
         // If we must flush the line after finishing this fragment due to `white-space: pre`,
         // detect that.
-        let line_flush_mode =
-            if flags.contains(WRAP_ON_NEWLINE_INLINE_REFLOW_FLAG) &&
-                    fragment.requires_line_break_afterward_if_wrapping_on_newlines() {
+        let line_flush_mode = if fragment.white_space_preserve_newlines() {
+            if fragment.requires_line_break_afterward_if_wrapping_on_newlines() {
                 LineFlushMode::Flush
             } else {
                 LineFlushMode::No
-            };
+            }
+        } else {
+            LineFlushMode::No
+        };
 
         // If we're not going to overflow the green zone vertically, we might still do so
         // horizontally. We'll try to place the whole fragment on this line and break somewhere if
@@ -602,35 +585,23 @@ impl LineBreaker {
             return
         }
 
-        // If we can't split the fragment and we're at the start of the line, then just overflow.
-        if !fragment.can_split() && self.pending_line_is_empty() {
-            debug!("LineBreaker: fragment can't split and line {} is empty, so overflowing",
-                    self.lines.len());
-            self.push_fragment_to_line(layout_context, fragment, LineFlushMode::No);
-            return
-        }
-
         // If the wrapping mode prevents us from splitting, then back up and split at the last
         // known good split point.
-        if flags.contains(NO_WRAP_INLINE_REFLOW_FLAG) &&
-                !flags.contains(WRAP_ON_NEWLINE_INLINE_REFLOW_FLAG) {
-            debug!("LineBreaker: white-space: nowrap in effect; falling back to last known good \
-                    split point");
+        if !fragment.white_space_allow_wrap() {
+            debug!("LineBreaker: fragment can't split; falling back to last known good split point");
             if !self.split_line_at_last_known_good_position() {
                 // No line breaking opportunity exists at all for this line. Overflow.
-                self.push_fragment_to_line(layout_context, fragment, LineFlushMode::No)
+                self.push_fragment_to_line(layout_context, fragment, line_flush_mode);
             } else {
-                self.work_list.push_front(fragment)
+                self.work_list.push_front(fragment);
             }
-            return
+            return;
         }
 
         // Split it up!
-        let available_inline_size = if !flags.contains(NO_WRAP_INLINE_REFLOW_FLAG) {
-            green_zone.inline - self.pending_line.bounds.size.inline - indentation
-        } else {
-            MAX_AU
-        };
+        let available_inline_size = green_zone.inline -
+                                    self.pending_line.bounds.size.inline -
+                                    indentation;
         let inline_start_fragment;
         let inline_end_fragment;
         let split_result = match fragment.calculate_split_position(available_inline_size,
@@ -1371,6 +1342,27 @@ impl Flow for InlineFlow {
                         intrinsic_sizes_for_inline_run = IntrinsicISizesContribution::new();
                     }
                 }
+                white_space::T::pre_wrap |
+                white_space::T::pre_line => {
+                    // Flush the intrinsic sizes we were gathering up for the nonbroken run, if
+                    // necessary.
+                    intrinsic_sizes_for_inline_run.union_inline(
+                        &intrinsic_sizes_for_nonbroken_run.finish());
+                    intrinsic_sizes_for_nonbroken_run = IntrinsicISizesContribution::new();
+
+                    intrinsic_sizes_for_nonbroken_run.union_inline(&intrinsic_sizes_for_fragment);
+
+                    // Flush the intrinsic sizes we've been gathering up in order to handle the
+                    // line break, if necessary.
+                    if fragment.requires_line_break_afterward_if_wrapping_on_newlines() {
+                        intrinsic_sizes_for_inline_run.union_inline(
+                            &intrinsic_sizes_for_nonbroken_run.finish());
+                        intrinsic_sizes_for_nonbroken_run = IntrinsicISizesContribution::new();
+                        intrinsic_sizes_for_flow.union_block(
+                            &intrinsic_sizes_for_inline_run.finish());
+                        intrinsic_sizes_for_inline_run = IntrinsicISizesContribution::new();
+                    }
+                }
                 white_space::T::normal => {
                     // Flush the intrinsic sizes we were gathering up for the nonbroken run, if
                     // necessary.
@@ -1378,7 +1370,7 @@ impl Flow for InlineFlow {
                         &intrinsic_sizes_for_nonbroken_run.finish());
                     intrinsic_sizes_for_nonbroken_run = IntrinsicISizesContribution::new();
 
-                    intrinsic_sizes_for_nonbroken_run.union_inline(&intrinsic_sizes_for_fragment)
+                    intrinsic_sizes_for_nonbroken_run.union_inline(&intrinsic_sizes_for_fragment);
                 }
             }
         }

servo/components/layout/text.rs

@@ -40,13 +40,10 @@ fn text(fragments: &LinkedList<Fragment>) -> String {
     for fragment in fragments {
         match fragment.specific {
             SpecificFragmentInfo::UnscannedText(ref info) => {
-                match fragment.white_space() {
-                    white_space::T::normal | white_space::T::nowrap => {
-                        text.push_str(&info.text.replace("\n", " "));
-                    }
-                    white_space::T::pre => {
-                        text.push_str(&info.text);
-                    }
+                if fragment.white_space_preserve_newlines() {
+                    text.push_str(&info.text);
+                } else {
+                    text.push_str(&info.text.replace("\n", " "));
                 }
             }
             _ => {}
@@ -161,10 +158,11 @@ impl TextRunScanner {
                 let inherited_text_style = in_fragment.style().get_inheritedtext();
                 fontgroup = font_context.layout_font_group_for_style(font_style);
                 compression = match in_fragment.white_space() {
-                    white_space::T::normal | white_space::T::nowrap => {
-                        CompressionMode::CompressWhitespaceNewline
-                    }
-                    white_space::T::pre => CompressionMode::CompressNone,
+                    white_space::T::normal |
+                    white_space::T::nowrap => CompressionMode::CompressWhitespaceNewline,
+                    white_space::T::pre |
+                    white_space::T::pre_wrap => CompressionMode::CompressNone,
+                    white_space::T::pre_line => CompressionMode::CompressWhitespace,
                 };
                 text_transform = inherited_text_style.text_transform;
                 letter_spacing = inherited_text_style.letter_spacing.0;
@@ -413,6 +411,10 @@ fn split_first_fragment_at_newline_if_necessary(fragments: &mut LinkedList<Fragm
         let string_before;
         let insertion_point_before;
         {
+            if !first_fragment.white_space_preserve_newlines() {
+                return;
+            }
+
             let unscanned_text_fragment_info = match first_fragment.specific {
                 SpecificFragmentInfo::UnscannedText(ref mut unscanned_text_fragment_info) => {
                     unscanned_text_fragment_info
@@ -420,10 +422,6 @@ fn split_first_fragment_at_newline_if_necessary(fragments: &mut LinkedList<Fragm
                 _ => return,
             };
 
-            if first_fragment.style.get_inheritedtext().white_space != white_space::T::pre {
-                return
-            }
-
             let position = match unscanned_text_fragment_info.text.find('\n') {
                 Some(position) if position < unscanned_text_fragment_info.text.len() - 1 => {
                     position

servo/components/layout/wrapper.rs

@@ -804,8 +804,11 @@ impl<'ln> ThreadSafeLayoutNode<'ln> {
             // If you implement other values for this property, you will almost certainly
             // want to update this check.
             match self.style().get_inheritedtext().white_space {
-                white_space::T::normal => true,
-                _ => false,
+                white_space::T::normal |
+                white_space::T::nowrap => true,
+                white_space::T::pre |
+                white_space::T::pre_wrap |
+                white_space::T::pre_line => false,
             }
         }
     }

servo/components/style/properties.mako.rs

@@ -2347,7 +2347,7 @@ pub mod longhands {
         }
     </%self:longhand>
 
-    ${single_keyword("white-space", "normal pre nowrap")}
+    ${single_keyword("white-space", "normal pre nowrap pre-wrap pre-line")}
 
     // TODO(pcwalton): `full-width`
     ${single_keyword("text-transform", "none capitalize uppercase lowercase")}