ComplexTextController: Control characters should be rendered as visib…

…le glyphs https://bugs.webkit.org/show_bug.cgi?id=271869 Reviewed by Vitor Roriz. Control characters weren't rendered as visible glyphs in the complex text code path in some condition. The spec <https://www.w3.org/TR/css-text-3/#white-space-processing> says: > Control characters (Unicode category Cc)—other than tabs (U+0009), > line feeds (U+000A), carriage returns (U+000D) and sequences that form > a segment break—must be rendered as a visible glyph https://webkit.org/b/149128 fixed the simple text code path. https://webkit.org/b/153941 fixed a part of the complex text code path. We need one more fix. ComplexTextController constructor is using FontCascade::isCharacterWhoseGlyphsShouldBeDeletedForTextRendering to determine a character is visible. It returned false for control characters. Changed isCharacterWhoseGlyphsShouldBeDeletedForTextRendering to return false for control characters other than tabs, line feeds, and carriage returns. WidthIterator::applyCSSVisibilityRules is also using isCharacterWhoseGlyphsShouldBeDeletedForTextRendering. Changed it to keep the original behavior. * LayoutTests/fast/text/control-characters/complex-1-expected.txt: Added. * LayoutTests/fast/text/control-characters/complex-1.html: Added. * Source/WebCore/platform/graphics/FontCascade.h: (WebCore::FontCascade::isCharacterWhoseGlyphsShouldBeDeletedForTextRendering): * Source/WebCore/platform/graphics/WidthIterator.cpp: (WebCore::WidthIterator::applyCSSVisibilityRules): Canonical link: https://commits.webkit.org/277574@main
WebKit · Apr 16, 2024 · 3baa72d · 3baa72d
1 parent e8a1f77
commit 3baa72d
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 11 deletions.
diff --git a/LayoutTests/fast/text/control-characters/complex-1-expected.txt b/LayoutTests/fast/text/control-characters/complex-1-expected.txt
@@ -0,0 +1,12 @@
+A control character should be rendered as a visible glyph. "ਅ\u0001ਆ" should be longer than "ਅਆ"
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS target.clientWidth is > ref.clientWidth
+PASS successfullyParsed is true
+
+TEST COMPLETE
+ਅਆ
+ਅਆ
+
diff --git a/LayoutTests/fast/text/control-characters/complex-1.html b/LayoutTests/fast/text/control-characters/complex-1.html
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<script src="../../../resources/js-test.js"></script>
+<style>
+  #ref, #target {
+      background: lightblue;
+      display: inline-block;
+      font-size: 100px;
+  }
+</style>
+<body>
+  <span id="target"></span><br>
+  <span id="ref"></span><br>
+  <script>
+    description('A control character should be rendered as a visible glyph. "\u0A05\\u0001\u0A06" should be longer than "\u0A05\u0A06"');
+    document.getElementById("target").textContent = '\u0A05\u0001\u0A06';
+    document.getElementById("ref").textContent = '\u0A05\u0A06';
+    shouldBeGreaterThan("target.clientWidth", "ref.clientWidth");
+  </script>
+</body>
+</html>
diff --git a/Source/WebCore/platform/graphics/FontCascade.h b/Source/WebCore/platform/graphics/FontCascade.h
@@ -285,9 +285,17 @@ class FontCascade : public CanMakeWeakPtr<FontCascade>, public CanMakeCheckedPtr
     static bool treatAsSpace(char32_t c) { return c == space || c == tabCharacter || c == newlineCharacter || c == noBreakSpace; }
     static bool isCharacterWhoseGlyphsShouldBeDeletedForTextRendering(char32_t character)
     {
-        // https://drafts.csswg.org/css-text-3/#white-space-processing
+        // https://www.w3.org/TR/css-text-3/#white-space-processing
+        // "Control characters (Unicode category Cc)—other than tabs (U+0009), line feeds (U+000A), carriage returns (U+000D) and sequences that form a segment break—must be rendered as a visible glyph"
+        if (character == tabCharacter || character == newlineCharacter || character == carriageReturn)
+            return true;
+        // Also, we're omitting Null (U+0000) from this set because Chrome and Firefox do so and it's needed for compat. See https://github.com/w3c/csswg-drafts/pull/6983.
+        if (character == nullCharacter)
+            return true;
+        if (isControlCharacter(character))
+            return false;
         // "Unsupported Default_ignorable characters must be ignored for text rendering."
-        return isControlCharacter(character) || isDefaultIgnorableCodePoint(character) || isInvisibleReplacementObjectCharacter(character);
+        return isDefaultIgnorableCodePoint(character) || isInvisibleReplacementObjectCharacter(character);
     }
     // FIXME: Callers of treatAsZeroWidthSpace() and treatAsZeroWidthSpaceInComplexScript() should probably be calling isCharacterWhoseGlyphsShouldBeDeletedForTextRendering() instead.
     static bool treatAsZeroWidthSpace(char32_t c) { return treatAsZeroWidthSpaceInComplexScript(c) || c == zeroWidthNonJoiner || c == zeroWidthJoiner; }

diff --git a/Source/WebCore/platform/graphics/WidthIterator.cpp b/Source/WebCore/platform/graphics/WidthIterator.cpp
@@ -756,9 +756,13 @@ void WidthIterator::applyCSSVisibilityRules(GlyphBuffer& glyphBuffer, unsigned g
         }
 
         // https://www.w3.org/TR/css-text-3/#white-space-processing
+        // "Unsupported Default_ignorable characters must be ignored for text rendering."
+        if (FontCascade::isCharacterWhoseGlyphsShouldBeDeletedForTextRendering(characterResponsibleForThisGlyph)) {
+            deleteGlyph(i);
+            continue;
+        }
         // "Control characters (Unicode category Cc)—other than tabs (U+0009), line feeds (U+000A), carriage returns (U+000D) and sequences that form a segment break—must be rendered as a visible glyph"
-        // Also, we're omitting Null (U+0000) from this set because Chrome and Firefox do so and it's needed for compat. See https://github.com/w3c/csswg-drafts/pull/6983.
-        if (characterResponsibleForThisGlyph != nullCharacter && isControlCharacter(characterResponsibleForThisGlyph)) {
+        if (isControlCharacter(characterResponsibleForThisGlyph)) {
             // Let's assume that .notdef is visible.
             GlyphBufferGlyph visibleGlyph = 0;
             clobberGlyph(i, visibleGlyph);
@@ -767,13 +771,6 @@ void WidthIterator::applyCSSVisibilityRules(GlyphBuffer& glyphBuffer, unsigned g
         }
 
         adjustForSyntheticBold(i);
-
-        // https://drafts.csswg.org/css-text-3/#white-space-processing
-        // "Unsupported Default_ignorable characters must be ignored for text rendering."
-        if (FontCascade::isCharacterWhoseGlyphsShouldBeDeletedForTextRendering(characterResponsibleForThisGlyph)) {
-            deleteGlyph(i);
-            continue;
-        }
     }
 }