[css-text] Handling trailing ideographic spaces in Legacy layout

In r824597 we have landed the new behavior of the NGLineBreaker class to
deal with trailing ideographic spaces, according to the last changes in
the CSS Text 3 specification.

These spaces are defined in the spec as 'other space separator' [1] and
as such, they should not be considered 'collapsible spaces' [2]. This
fact has some implications in the implementation of the Phase I [3] of
the White Space Processing algorithm.

Additionally, this CL implements also the required logic in the
BreakingContext class to deal with this kind of trailing spaces,
according to the value of the 'white-space' [4] CSS property. When its
allowed, these trailing spaces will be hang or even broken (in case of
break-space value), instead of selecting previous breaking opportunities
to avoid overflow, like it's described in the bug.

It's worth mentioning that even though this CL is about hanging trailing
spaces, we need to apply some changes in the TextBreakIterator. These
changes are needed to avoid ideographic spaces to be handled by ICU. The
problem is that ICU, following the UAX14 spec where ideographic spaces
are classified as BA, will return breaking opportunities 'after' the
trailing spaces character sequence. However, due to some performance
optimizations, we have decided to implement the BreakingContext logic
assuming that we always break before space (kBeforeEverySpace).

We have landed a patch in r807457 with an important refactoring to sync
TextBreakIterator behavior on the white-space handling with ICU, so that
we always break 'before space'. However, this change affects only to the
LayoutNG inline-layout logic, which was adapted accordingly to the new
behavior.

We have decided to avoid this approach for Legacy layout, since a big
refactoring like this could imply important regressions, difficult to
track and fix nowadays. The changes in the TextBreakIterator performed
by this CL try to handle ideographic spaces as if they were regular
white space character, avoiding ICU. The specific hanging behavior will
be implemented then by the BreakingContext class, assuming the old
'break before space' behavior.


[1] https://drafts.csswg.org/css-text-3/#other-space-separators
[2] https://drafts.csswg.org/css-text-3/#collapsible-white-space
[3] https://drafts.csswg.org/css-text-3/#white-space-phase-1
[4] https://drafts.csswg.org/css-text-3/#white-space-property

Bug: 972952
Change-Id: Ibc5a9d4c0b420320ef03f2a088685775b0034ec5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2521614Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Cr-Commit-Position: refs/heads/master@{#824903}

third_party/blink/renderer/core/layout/line/breaking_context_inline_headers.h

@@ -79,6 +79,7 @@ class BreakingContext {
         at_start_(true),
         ignoring_spaces_(false),
         current_character_is_space_(false),
+        is_space_or_other_space_separator_(false),
         applied_start_width_(applied_start_width),
         include_end_width_(true),
         auto_wrap_(false),
@@ -205,6 +206,7 @@ class BreakingContext {
   bool at_start_;
   bool ignoring_spaces_;
   bool current_character_is_space_;
+  bool is_space_or_other_space_separator_;
   bool previous_character_is_space_;
   bool has_former_opportunity_;
   unsigned current_start_offset_;  // initial offset for the current text
@@ -290,9 +292,14 @@ inline bool RequiresLineBox(
     return true;
 
   UChar current = it.Current();
+  if (whitespace_position == kLeadingWhitespace &&
+      current == kIdeographicSpaceCharacter &&
+      line_info.PreviousLineBrokeCleanly())
+    return true;
   bool not_just_whitespace = current != kSpaceCharacter &&
                              current != kTabulationCharacter &&
                              current != kSoftHyphenCharacter &&
+                             current != kIdeographicSpaceCharacter &&
                              (current != kNewlineCharacter ||
                               it.GetLineLayoutItem().PreservesNewline());
   return not_just_whitespace || IsEmptyInline(it.GetLineLayoutItem());
@@ -376,8 +383,10 @@ inline void BreakingContext::InitializeForCurrentObject() {
 
   // Ensure the whitespace in constructions like '<span style="white-space:
   // pre-wrap">text <span><span> text</span>' does not collapse.
-  if (collapse_white_space_ && !ComputedStyle::CollapseWhiteSpace(last_ws_))
+  if (collapse_white_space_ && !ComputedStyle::CollapseWhiteSpace(last_ws_)) {
     current_character_is_space_ = false;
+    is_space_or_other_space_separator_ = false;
+  }
 
   // Since current_ iterates all along the text's length, we need to store the
   // initial offset of the current handle text so that we can then identify
@@ -629,6 +638,7 @@ inline void BreakingContext::HandleEmptyInline() {
       // If this object is at the start of the line, we need to behave like list
       // markers and start ignoring spaces.
       current_character_is_space_ = true;
+      is_space_or_other_space_separator_ = true;
       ignoring_spaces_ = true;
     } else {
       // If we are after a trailing space but aren't ignoring spaces yet then
@@ -666,6 +676,7 @@ inline void BreakingContext::HandleReplaced() {
   line_info_.SetEmpty(false);
   ignoring_spaces_ = false;
   current_character_is_space_ = false;
+  is_space_or_other_space_separator_ = false;
   trailing_objects_.Clear();
 
   // Optimize for a common case. If we can't find whitespace after the list
@@ -682,6 +693,7 @@ inline void BreakingContext::HandleReplaced() {
       // Like with inline flows, we start ignoring spaces to make sure that any
       // additional spaces we see will be discarded.
       current_character_is_space_ = true;
+      is_space_or_other_space_separator_ = true;
       ignoring_spaces_ = true;
     }
     if (LineLayoutListMarker(current_.GetLineLayoutItem()).IsInside())
@@ -709,6 +721,8 @@ ALWAYS_INLINE void BreakingContext::SetCurrentCharacterIsSpace(UChar c) {
   current_character_is_space_ =
       c == kSpaceCharacter || c == kTabulationCharacter ||
       (!preserves_newline_ && (c == kNewlineCharacter));
+  is_space_or_other_space_separator_ =
+      current_character_is_space_ || c == kIdeographicSpaceCharacter;
 }
 
 inline float FirstPositiveWidth(const WordMeasurements& word_measurements) {
@@ -1063,9 +1077,12 @@ inline bool BreakingContext::HandleText(WordMeasurements& word_measurements,
   UChar last_character = layout_text_info_.line_break_iterator_.LastCharacter();
   UChar second_to_last_character =
       layout_text_info_.line_break_iterator_.SecondToLastCharacter();
+  bool previous_is_space_or_other_space_separator = false;
   for (; current_.Offset() < layout_text.TextLength();
        current_.FastIncrementInTextNode()) {
     previous_character_is_space_ = current_character_is_space_;
+    previous_is_space_or_other_space_separator =
+        is_space_or_other_space_separator_;
     UChar c = current_.Current();
     SetCurrentCharacterIsSpace(c);
 
@@ -1128,7 +1145,7 @@ inline bool BreakingContext::HandleText(WordMeasurements& word_measurements,
       }
 
       PrepareForNextCharacter(layout_text, prohibit_break_inside,
-                              previous_character_is_space_);
+                              previous_is_space_or_other_space_separator);
       at_start_ = false;
       NextCharacter(c, last_character, second_to_last_character);
       continue;
@@ -1168,8 +1185,8 @@ inline bool BreakingContext::HandleText(WordMeasurements& word_measurements,
     // We keep track of the total width contributed by trailing space as we
     // often want to exclude it when determining
     // if a run fits on a line.
-    if (collapse_white_space_ && previous_character_is_space_ &&
-        current_character_is_space_ && last_width_measurement)
+    if (collapse_white_space_ && previous_is_space_or_other_space_separator &&
+        is_space_or_other_space_separator_ && last_width_measurement)
       width_.SetTrailingWhitespaceWidth(last_width_measurement);
 
     // If this is the end of the first word in run of text then make sure we
@@ -1287,7 +1304,7 @@ inline bool BreakingContext::HandleText(WordMeasurements& word_measurements,
     }
 
     PrepareForNextCharacter(layout_text, prohibit_break_inside,
-                            previous_character_is_space_);
+                            previous_is_space_or_other_space_separator);
     at_start_ = false;
     is_line_empty = line_info_.IsEmpty();
     NextCharacter(c, last_character, second_to_last_character);
@@ -1322,7 +1339,7 @@ inline bool BreakingContext::HandleText(WordMeasurements& word_measurements,
   width_.AddUncommittedWidth(last_width_measurement +
                              additional_width_from_ancestors);
 
-  if (collapse_white_space_ && current_character_is_space_ &&
+  if (collapse_white_space_ && is_space_or_other_space_separator_ &&
       last_width_measurement)
     width_.SetTrailingWhitespaceWidth(last_width_measurement +
                                       additional_width_from_ancestors);
@@ -1367,7 +1384,7 @@ inline bool BreakingContext::HandleText(WordMeasurements& word_measurements,
 inline void BreakingContext::PrepareForNextCharacter(
     const LineLayoutText& layout_text,
     bool& prohibit_break_inside,
-    bool previous_character_is_space) {
+    bool previous_is_space_or_other_space_separator) {
   if (layout_text.IsSVGInlineText() && current_.Offset()) {
     // Force creation of new InlineBoxes for each absolute positioned character
     // (those that start new text chunks).
@@ -1379,11 +1396,12 @@ inline void BreakingContext::PrepareForNextCharacter(
     current_.SetNextBreakablePosition(layout_text.TextLength());
     prohibit_break_inside = false;
   }
-  if (current_character_is_space_ && !previous_character_is_space) {
+  if (current_character_is_space_ && !previous_character_is_space_) {
     start_of_ignored_spaces_.SetLineLayoutItem(current_.GetLineLayoutItem());
     start_of_ignored_spaces_.SetOffset(current_.Offset());
   }
-  if (!current_character_is_space_ && previous_character_is_space) {
+  if (!is_space_or_other_space_separator_ &&
+      previous_is_space_or_other_space_separator) {
     if (auto_wrap_ && current_style_->BreakOnlyAfterWhiteSpace()) {
       line_break_.MoveTo(current_.GetLineLayoutItem(), current_.Offset(),
                          current_.NextBreakablePosition());
@@ -1462,10 +1480,13 @@ inline bool BreakingContext::TrailingSpaceExceedsAvailableWidth(
     bool apply_word_spacing,
     bool word_spacing,
     const Font& font) {
+  bool is_other_space_separator =
+      current_.Current() == kIdeographicSpaceCharacter;
   // If we break only after white-space, consider the current character
   // as candidate width for this line.
-  if (width_.FitsOnLine() && current_character_is_space_ &&
-      current_style_->BreakOnlyAfterWhiteSpace()) {
+  if (width_.FitsOnLine() && (is_other_space_separator ||
+                              (current_character_is_space_ &&
+                               current_style_->BreakOnlyAfterWhiteSpace()))) {
     float char_width = TextWidth(layout_text, current_.Offset(), 1, font,
                                  width_.CurrentWidth(), collapse_white_space_,
                                  &word_measurement.fallback_fonts,

third_party/blink/renderer/platform/text/text_break_iterator.cc

@@ -24,7 +24,6 @@
 #include "third_party/blink/renderer/platform/text/text_break_iterator.h"
 
 #include "base/stl_util.h"
-#include "third_party/blink/renderer/platform/text/character.h"
 #include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"
 #include "third_party/blink/renderer/platform/wtf/text/ascii_ctype.h"
 #include "third_party/blink/renderer/platform/wtf/text/character_names.h"
@@ -321,7 +320,7 @@ inline int LazyLineBreakIterator::NextBreakablePosition(
     is_space = IsBreakableSpace(ch);
     switch (break_space) {
       case BreakSpaceType::kBeforeEverySpace:
-        if (is_space)
+        if (is_space || IsOtherSpaceSeparator<CharacterType>(ch))
           return i;
         break;
       case BreakSpaceType::kBeforeSpaceRun:

third_party/blink/renderer/platform/text/text_break_iterator.h

@@ -29,6 +29,7 @@
 #include "base/containers/span.h"
 #include "base/macros.h"
 #include "third_party/blink/renderer/platform/platform_export.h"
+#include "third_party/blink/renderer/platform/text/character.h"
 #include "third_party/blink/renderer/platform/wtf/text/atomic_string.h"
 #include "third_party/blink/renderer/platform/wtf/text/character_names.h"
 #include "third_party/blink/renderer/platform/wtf/text/unicode.h"
@@ -314,6 +315,15 @@ class PLATFORM_EXPORT LazyLineBreakIterator final {
     return iterator_;
   }
 
+  template <typename CharacterType>
+  bool IsOtherSpaceSeparator(UChar ch) const {
+    return Character::IsOtherSpaceSeparator(ch);
+  }
+  template <LChar>
+  bool IsOtherSpaceSeparator(UChar ch) const {
+    return false;
+  }
+
   template <typename CharacterType, LineBreakType, BreakSpaceType>
   int NextBreakablePosition(int pos, const CharacterType* str, int len) const;
   template <typename CharacterType, LineBreakType>

third_party/blink/web_tests/FlagExpectations/disable-layout-ng

@@ -221,17 +221,6 @@ crbug.com/591099 external/wpt/css/css-text/white-space/control-chars-00C.html [
 crbug.com/591099 external/wpt/css/css-text/white-space/line-edge-white-space-collapse-001.html [ Failure ]
 crbug.com/591099 external/wpt/css/css-text/white-space/line-edge-white-space-collapse-002.html [ Failure ]
 crbug.com/591099 external/wpt/css/css-text/white-space/pre-line-with-space-and-newline.html [ Failure ]
-crbug.com/991413 external/wpt/css/css-text/white-space/trailing-ideographic-space-003.html [ Failure ]
-crbug.com/991413 external/wpt/css/css-text/white-space/trailing-ideographic-space-004.html [ Failure ]
-crbug.com/991413 external/wpt/css/css-text/white-space/trailing-ideographic-space-005.html [ Failure ]
-crbug.com/991413 external/wpt/css/css-text/white-space/trailing-ideographic-space-006.html [ Failure ]
-crbug.com/991413 external/wpt/css/css-text/white-space/trailing-ideographic-space-008.html [ Failure ]
-crbug.com/991413 external/wpt/css/css-text/white-space/trailing-ideographic-space-010.html [ Failure ]
-crbug.com/991413 external/wpt/css/css-text/white-space/trailing-ideographic-space-011.html [ Failure ]
-crbug.com/991413 external/wpt/css/css-text/white-space/trailing-ideographic-space-013.html [ Failure ]
-crbug.com/991413 external/wpt/css/css-text/white-space/trailing-ideographic-space-014.html [ Failure ]
-crbug.com/991413 external/wpt/css/css-text/white-space/trailing-ideographic-space-015.html [ Failure ]
-crbug.com/991413 external/wpt/css/css-text/white-space/trailing-ideographic-space-016.html [ Failure ]
 
 ### external/wpt/css/css-text/word-break/
 crbug.com/591099 external/wpt/css/css-text/word-break/word-break-break-all-004.html [ Failure ]