From 3baa72dbb8d513476b27bcfae8d2dd02c6514541 Mon Sep 17 00:00:00 2001 From: Fujii Hironori Date: Tue, 16 Apr 2024 15:01:18 -0700 Subject: [PATCH] ComplexTextController: Control characters should be rendered as visible glyphs https://bugs.webkit.org/show_bug.cgi?id=271869 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed by Vitor Roriz. Control characters weren't rendered as visible glyphs in the complex text code path in some condition. The spec 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 --- .../control-characters/complex-1-expected.txt | 12 +++++++++++ .../text/control-characters/complex-1.html | 20 +++++++++++++++++++ .../WebCore/platform/graphics/FontCascade.h | 12 +++++++++-- .../platform/graphics/WidthIterator.cpp | 15 ++++++-------- 4 files changed, 48 insertions(+), 11 deletions(-) create mode 100644 LayoutTests/fast/text/control-characters/complex-1-expected.txt create mode 100644 LayoutTests/fast/text/control-characters/complex-1.html diff --git a/LayoutTests/fast/text/control-characters/complex-1-expected.txt b/LayoutTests/fast/text/control-characters/complex-1-expected.txt new file mode 100644 index 000000000000..f089cbc3fe22 --- /dev/null +++ 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 new file mode 100644 index 000000000000..b1b4b59c441c --- /dev/null +++ b/LayoutTests/fast/text/control-characters/complex-1.html @@ -0,0 +1,20 @@ + + + + +
+
+ + + diff --git a/Source/WebCore/platform/graphics/FontCascade.h b/Source/WebCore/platform/graphics/FontCascade.h index b3e665e1dd9c..650ba4a4e16b 100644 --- a/Source/WebCore/platform/graphics/FontCascade.h +++ b/Source/WebCore/platform/graphics/FontCascade.h @@ -285,9 +285,17 @@ class FontCascade : public CanMakeWeakPtr, 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 index ba6ae0ef4bd7..a80cd28df44d 100644 --- 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; - } } }