[LFC][IFC] Preserved tab is not a word separator

https://bugs.webkit.org/show_bug.cgi?id=231687 Reviewed by Antti Koivisto. Source/WebCore: The 'tab' character is not considered a word separator in the word-spacing sense. When the preserved whitespace sequence has the mix of word separators (regular space) and non-separators, we use incorrect 'x' position to compute the width of the 'tab' character (tab width is position sensitive). This patch fixes it by splitting the whitespace sequence at word-separator boundary (e.g [space][space][tab] will produce 2 individual InlineTextItems [space][space] and [tab]). Test: fast/text/preserved-white-space-with-word-spacing.html * layout/formattingContexts/inline/InlineLineBuilder.cpp: (WebCore::Layout::LineBuilder::candidateContentForLine): * layout/formattingContexts/inline/InlineTextItem.cpp: (WebCore::Layout::moveToNextNonWhitespacePosition): (WebCore::Layout::InlineTextItem::createAndAppendTextItems): LayoutTests: * fast/text/preserved-white-space-with-word-spacing-expected.html: Added. * fast/text/preserved-white-space-with-word-spacing.html: Added. Canonical link: https://commits.webkit.org/242947@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@284127 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Oct 13, 2021 · e17cf3e831a3c87379b8e875710cd59a7c62ba15 · e17cf3e
1 parent 7138c6f
commit e17cf3e831a3c87379b8e875710cd59a7c62ba15
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 5 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2021-10-13  Alan Bujtas  <zalan@apple.com>
+
+        [LFC][IFC] Preserved tab is not a word separator
+        https://bugs.webkit.org/show_bug.cgi?id=231687
+
+        Reviewed by Antti Koivisto.
+
+        * fast/text/preserved-white-space-with-word-spacing-expected.html: Added.
+        * fast/text/preserved-white-space-with-word-spacing.html: Added.
+
 2021-10-13  Jean-Yves Avenard  <jya@apple.com>
 
         Add mp3 and aac web audio test

diff --git a/LayoutTests/fast/text/preserved-white-space-with-word-spacing-expected.html b/LayoutTests/fast/text/preserved-white-space-with-word-spacing-expected.html
@@ -0,0 +1,10 @@
+<style>
+ body {
+   margin: 0px;
+ }
+ div {
+   font-family: Ahem;
+   font-size: 50px;
+ }
+</style>
+<div>XX</div>
diff --git a/LayoutTests/fast/text/preserved-white-space-with-word-spacing.html b/LayoutTests/fast/text/preserved-white-space-with-word-spacing.html
@@ -0,0 +1,16 @@
+<style>
+ body {
+   margin: 0px;
+ }
+ div {
+   white-space: pre;
+   font-family: Ahem;
+   font-size: 50px;
+   margin-left: -400px;
+ }
+ span {
+   word-spacing: 100px;
+ }
+</style>
+<!-- The preserved tab is not a word separator and should not trigger word-spacing -->
+<div>X<span> 	X</span>X</div>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2021-10-13  Alan Bujtas  <zalan@apple.com>
+
+        [LFC][IFC] Preserved tab is not a word separator
+        https://bugs.webkit.org/show_bug.cgi?id=231687
+
+        Reviewed by Antti Koivisto.
+
+        The 'tab' character is not considered a word separator in the word-spacing sense. When the preserved whitespace sequence
+        has the mix of word separators (regular space) and non-separators, we use incorrect 'x' position to compute the width of the 'tab' character (tab width is position sensitive).
+        This patch fixes it by splitting the whitespace sequence at word-separator boundary (e.g [space][space][tab] will produce 2 individual InlineTextItems [space][space] and [tab]). 
+
+        Test: fast/text/preserved-white-space-with-word-spacing.html
+
+        * layout/formattingContexts/inline/InlineLineBuilder.cpp:
+        (WebCore::Layout::LineBuilder::candidateContentForLine):
+        * layout/formattingContexts/inline/InlineTextItem.cpp:
+        (WebCore::Layout::moveToNextNonWhitespacePosition):
+        (WebCore::Layout::InlineTextItem::createAndAppendTextItems):
+
 2021-10-13  Sihui Liu  <sihui_liu@apple.com>
 
         Implement FileSystemHandle move()

diff --git a/Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.cpp b/Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.cpp
@@ -509,6 +509,10 @@ void LineBuilder::candidateContentForLine(LineCandidate& lineCandidate, size_t c
             auto logicalWidth = leadingLogicalWidth ? *std::exchange(leadingLogicalWidth, std::nullopt) : inlineItemWidth(inlineItem, currentLogicalRight);
             lineCandidate.inlineContent.appendInlineItem(inlineItem, style, logicalWidth);
             currentLogicalRight += logicalWidth;
+            if (is<InlineTextItem>(inlineItem) && downcast<InlineTextItem>(inlineItem).isWordSeparator()) {
+                // Word spacing does not make the run longer, but it produces an offset instead. See Line::appendTextContent as well.
+                currentLogicalRight += style.fontCascade().wordSpacing();
+            }
             continue;
         }
         if (inlineItem.isBox()) {

diff --git a/Source/WebCore/layout/formattingContexts/inline/InlineTextItem.cpp b/Source/WebCore/layout/formattingContexts/inline/InlineTextItem.cpp
@@ -42,18 +42,23 @@ struct WhitespaceContent {
     size_t length { 0 };
     bool isWordSeparator { true };
 };
-static std::optional<WhitespaceContent> moveToNextNonWhitespacePosition(StringView textContent, size_t startPosition, bool preserveNewline, bool preserveTab, bool treatNonBreakingSpaceAsRegularSpace)
+static std::optional<WhitespaceContent> moveToNextNonWhitespacePosition(StringView textContent, size_t startPosition, bool preserveNewline, bool preserveTab, bool treatNonBreakingSpaceAsRegularSpace, bool stopAtWordSeparatorBoundary)
 {
     auto hasWordSeparatorCharacter = false;
+    auto isWordSeparatorCharacter = false;
     auto isWhitespaceCharacter = [&](auto character) {
         // white space processing in CSS affects only the document white space characters: spaces (U+0020), tabs (U+0009), and segment breaks.
         auto isTreatedAsSpaceCharacter = character == space || (character == newlineCharacter && !preserveNewline) || (character == tabCharacter && !preserveTab) || (character == noBreakSpace && treatNonBreakingSpaceAsRegularSpace);
-        hasWordSeparatorCharacter = hasWordSeparatorCharacter || isTreatedAsSpaceCharacter;
+        isWordSeparatorCharacter = isTreatedAsSpaceCharacter;
+        hasWordSeparatorCharacter = hasWordSeparatorCharacter || isWordSeparatorCharacter;
         return isTreatedAsSpaceCharacter || character == tabCharacter;
     };
     auto nextNonWhiteSpacePosition = startPosition;
-    while (nextNonWhiteSpacePosition < textContent.length() && isWhitespaceCharacter(textContent[nextNonWhiteSpacePosition]))
+    while (nextNonWhiteSpacePosition < textContent.length() && isWhitespaceCharacter(textContent[nextNonWhiteSpacePosition])) {
+        if (UNLIKELY(stopAtWordSeparatorBoundary && hasWordSeparatorCharacter && !isWordSeparatorCharacter))
+            break;
         ++nextNonWhiteSpacePosition;
+    }
     return nextNonWhiteSpacePosition == startPosition ? std::nullopt : std::make_optional(WhitespaceContent { nextNonWhiteSpacePosition - startPosition, hasWordSeparatorCharacter });
 }
 
@@ -79,7 +84,8 @@ void InlineTextItem::createAndAppendTextItems(InlineItems& inlineContent, const
 
     auto& style = inlineTextBox.style();
     auto& fontCascade = style.fontCascade();
-    auto whitespaceContentIsTreatedAsSingleSpace = !TextUtil::shouldPreserveSpacesAndTabs(inlineTextBox);
+    auto shouldPreserveSpacesAndTabs = TextUtil::shouldPreserveSpacesAndTabs(inlineTextBox);
+    auto whitespaceContentIsTreatedAsSingleSpace = !shouldPreserveSpacesAndTabs;
     auto shouldPreserveNewline = TextUtil::shouldPreserveNewline(inlineTextBox);
     auto shouldTreatNonBreakingSpaceAsRegularSpace = style.nbspMode() == NBSPMode::Space;
     auto lineBreakIterator = LazyLineBreakIterator { text, style.computedLocale(), TextUtil::lineBreakIteratorMode(style.lineBreak()) };
@@ -104,7 +110,8 @@ void InlineTextItem::createAndAppendTextItems(InlineItems& inlineContent, const
             continue;
         }
 
-        if (auto whitespaceContent = moveToNextNonWhitespacePosition(text, currentPosition, shouldPreserveNewline, !whitespaceContentIsTreatedAsSingleSpace, shouldTreatNonBreakingSpaceAsRegularSpace)) {
+        auto stopAtWordSeparatorBoundary = shouldPreserveSpacesAndTabs && fontCascade.wordSpacing();
+        if (auto whitespaceContent = moveToNextNonWhitespacePosition(text, currentPosition, shouldPreserveNewline, shouldPreserveSpacesAndTabs, shouldTreatNonBreakingSpaceAsRegularSpace, stopAtWordSeparatorBoundary)) {
             ASSERT(whitespaceContent->length);
             auto appendWhitespaceItem = [&] (auto startPosition, auto itemLength) {
                 auto simpleSingleWhitespaceContent = inlineTextBox.canUseSimplifiedContentMeasuring() && (itemLength == 1 || whitespaceContentIsTreatedAsSingleSpace);