⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 288107 in webkit


Ignore:
Timestamp:
Jan 17, 2022, 7:02:17 PM (5 years ago)
Author:
mmaxfield@apple.com
Message:

REGRESSION(r281389): using font-variant-ligatures causes Unicode bidi isolation control characters to render
​https://bugs.webkit.org/show_bug.cgi?id=235053
<rdar://problem/87425066>

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

  • web-platform-tests/css/css-text/white-space/default-ignorable-complex-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/default-ignorable-complex.html: Added.
  • web-platform-tests/css/css-text/white-space/default-ignorable-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/default-ignorable.html: Added.

Source/WebCore:

The spec says (​https://drafts.csswg.org/css-text-3/#white-space-processing)

unsupported Default_ignorable characters must be ignored for text rendering.

Previously, we were handling this by mapping the characters to the zero width space character. r281389 changed
that to be more faithful to the original string (and not do the mapping). One somewhat unexpected result of
this patch was that, now that we're not mapping these code points to the zero width space character, fewer fonts
on the system will support those code points. It became much more common for no font at all to support the code
point in question. r281389 handled the situation where the code points were supported by a font but shouldn't be
rendered, but didn't handle the case where the code points *weren't* supported by any fonts (in the complex text
codepath). This patch updates that path to handle it.

This patch also replaces our previous hardcoded list of Default_ignorable characters with the proper call to ICU
to check the Unicode property. It does this in both the fast text codepath and the simple text codepath, to make
sure they match.

Tests: imported/w3c/web-platform-tests/css/css-text/white-space/default-ignorable-complex.html

imported/w3c/web-platform-tests/css/css-text/white-space/default-ignorable.html

  • platform/graphics/ComplexTextController.cpp:

(WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun):

  • platform/graphics/WidthIterator.cpp:

(WebCore::WidthIterator::applyCSSVisibilityRules):

Location:
trunk
Files:
4 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r288093 r288107  
     12022-01-17  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        REGRESSION(r281389): using font-variant-ligatures causes Unicode bidi isolation control characters to render
     4        https://bugs.webkit.org/show_bug.cgi?id=235053
     5        <rdar://problem/87425066>
     6
     7        Reviewed by Darin Adler.
     8
     9        * web-platform-tests/css/css-text/white-space/default-ignorable-complex-expected.html: Added.
     10        * web-platform-tests/css/css-text/white-space/default-ignorable-complex.html: Added.
     11        * web-platform-tests/css/css-text/white-space/default-ignorable-expected.html: Added.
     12        * web-platform-tests/css/css-text/white-space/default-ignorable.html: Added.
     13
    1142022-01-17  Youenn Fablet  <youenn@apple.com>
    215

  • trunk/Source/WebCore/ChangeLog

    r288104 r288107  
     12022-01-17  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        REGRESSION(r281389): using font-variant-ligatures causes Unicode bidi isolation control characters to render
     4        https://bugs.webkit.org/show_bug.cgi?id=235053
     5        <rdar://problem/87425066>
     6
     7        Reviewed by Darin Adler.
     8
     9        The spec says (https://drafts.csswg.org/css-text-3/#white-space-processing)
     10
     11        > unsupported Default_ignorable characters must be ignored for text rendering.
     12
     13        Previously, we were handling this by mapping the characters to the zero width space character. r281389 changed
     14        that to be more faithful to the original string (and not do the mapping). One somewhat unexpected result of
     15        this patch was that, now that we're not mapping these code points to the zero width space character, fewer fonts
     16        on the system will support those code points. It became much more common for no font at all to support the code
     17        point in question. r281389 handled the situation where the code points were supported by a font but shouldn't be
     18        rendered, but didn't handle the case where the code points *weren't* supported by any fonts (in the complex text
     19        codepath). This patch updates that path to handle it.
     20
     21        This patch also replaces our previous hardcoded list of Default_ignorable characters with the proper call to ICU
     22        to check the Unicode property. It does this in both the fast text codepath and the simple text codepath, to make
     23        sure they match.
     24
     25        Tests: imported/w3c/web-platform-tests/css/css-text/white-space/default-ignorable-complex.html
     26               imported/w3c/web-platform-tests/css/css-text/white-space/default-ignorable.html
     27
     28        * platform/graphics/ComplexTextController.cpp:
     29        (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun):
     30        * platform/graphics/WidthIterator.cpp:
     31        (WebCore::WidthIterator::applyCSSVisibilityRules):
     32
    1332022-01-17  ChangSeok Oh  <changseok@webkit.org>
    234

  • trunk/Source/WebCore/platform/graphics/ComplexTextController.cpp

    r286637 r288107  
    829829    unsigned r = m_indexBegin;
    830830    while (r < m_indexEnd) {
    831         m_coreTextIndices.uncheckedAppend(r);
     831        auto currentIndex = r;
    832832        UChar32 character;
    833833        U16_NEXT(m_characters, r, m_stringLength, character);
     834        // https://drafts.csswg.org/css-text-3/#white-space-processing
     835        // "Unsupported Default_ignorable characters must be ignored for text rendering."
     836        if (!FontCascade::isCharacterWhoseGlyphsShouldBeDeletedForTextRendering(character))
     837            m_coreTextIndices.uncheckedAppend(currentIndex);
    834838    }
    835839    m_glyphCount = m_coreTextIndices.size();

  • trunk/Source/WebCore/platform/graphics/FontCascade.h

    r287250 r288107  
    261261    FontSelector* fontSelector() const;
    262262    static bool treatAsSpace(UChar32 c) { return c == space || c == tabCharacter || c == newlineCharacter || c == noBreakSpace; }
     263    static bool isCharacterWhoseGlyphsShouldBeDeletedForTextRendering(UChar32 character)
     264    {
     265        // https://drafts.csswg.org/css-text-3/#white-space-processing
     266        // "Unsupported Default_ignorable characters must be ignored for text rendering."
     267        return (character >= nullCharacter && character < space)
     268            || (character >= deleteCharacter && character < noBreakSpace)
     269            || character == objectReplacementCharacter
     270            || u_hasBinaryProperty(character, UCHAR_DEFAULT_IGNORABLE_CODE_POINT);
     271    }
     272    // FIXME: Callers of treatAsZeroWidthSpace() and treatAsZeroWidthSpaceInComplexScript() should probably be calling isCharacterWhoseGlyphsShouldBeDeletedForTextRendering() instead.
    263273    static bool treatAsZeroWidthSpace(UChar32 c) { return treatAsZeroWidthSpaceInComplexScript(c) || c == zeroWidthNonJoiner || c == zeroWidthJoiner; }
    264274    static bool treatAsZeroWidthSpaceInComplexScript(UChar32 c) { return c < space || (c >= deleteCharacter && c < noBreakSpace) || c == softHyphen || c == zeroWidthSpace || (c >= leftToRightMark && c <= rightToLeftMark) || (c >= leftToRightEmbed && c <= rightToLeftOverride) || c == zeroWidthNoBreakSpace || c == objectReplacementCharacter; }

  • trunk/Source/WebCore/platform/graphics/WidthIterator.cpp

    r287724 r288107  
    617617        adjustForSyntheticBold(i);
    618618
    619         if ((characterResponsibleForThisGlyph >= nullCharacter && characterResponsibleForThisGlyph < space)
    620             || (characterResponsibleForThisGlyph >= deleteCharacter && characterResponsibleForThisGlyph < noBreakSpace)) {
    621             deleteGlyph(i);
    622             continue;
    623         }
    624 
    625         switch (characterResponsibleForThisGlyph) {
    626         case softHyphen:
    627         case leftToRightMark:
    628         case rightToLeftMark:
    629         case leftToRightEmbed:
    630         case rightToLeftEmbed:
    631         case leftToRightOverride:
    632         case rightToLeftOverride:
    633         case leftToRightIsolate:
    634         case rightToLeftIsolate:
    635         case zeroWidthNonJoiner:
    636         case zeroWidthJoiner:
    637         case popDirectionalFormatting:
    638         case popDirectionalIsolate:
    639         case firstStrongIsolate:
    640         case objectReplacementCharacter:
    641         case zeroWidthNoBreakSpace:
     619        // https://drafts.csswg.org/css-text-3/#white-space-processing
     620        // "Unsupported Default_ignorable characters must be ignored for text rendering."
     621        if (FontCascade::isCharacterWhoseGlyphsShouldBeDeletedForTextRendering(characterResponsibleForThisGlyph)) {
    642622            deleteGlyph(i);
    643623            continue;
Note: See TracChangeset for help on using the changeset viewer.