Bug 193395 – REGRESSION(r239915): about 130 test failures on WPE

• Status: RESOLVED
• Resolution: FIXED
• Component: WPE WebKit

[Zan Dobersek]

Change introduced in r239915 caused about 130 layout test failures for the WPE port.

Example failure:
https://build.webkit.org/results/WPE%20Linux%2064-bit%20Release%20(Tests)/r239915%20(11656)/imported/w3c/i18n/bidi/bidi-embed-001-diffs.html

[Carlos Garcia Campos]

Created attachment 359029 [details]
Patch

[EWS Watchlist]

Comment on attachment 359029 [details]
Patch

Attachment 359029 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10743145

New failing tests:
fast/text/soft-hyphen-min-preferred-width.html
fast/text/format-control.html

[EWS Watchlist]

Created attachment 359031 [details]
Archive of layout-test-results from ews103 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 359029 [details]
Patch

Attachment 359029 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10743172

New failing tests:
fast/text/soft-hyphen-min-preferred-width.html
fast/text/format-control.html

[EWS Watchlist]

Created attachment 359032 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Michael Catanzaro]

Comment on attachment 359029 [details]
Patch

New failing tests:
fast/text/soft-hyphen-min-preferred-width.html
fast/text/format-control.html

[Carlos Garcia Campos]

Created attachment 359033 [details]
Patch

Made the change specific to FreeType to fix the test failures in mac.

[Zan Dobersek]

Comment on attachment 359033 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=359033&action=review

> Source/WebCore/platform/graphics/WidthIterator.cpp:206
> +        bool characterMustDrawSomething = !isDefaultIgnorableCodePoint(character);
> +#if USE(FREETYPE)
> +        if (!characterMustDrawSomething) {
> +            textIterator.advance(advanceLength);
> +            continue;
> +        }
> +#endif

This treats ignorable characters as if there's no glyph representing such characters in font data. Can't this logic be used in GlyphPage::fill(), enforcing no glyph representation for characters that are ignorable?

[Carlos Garcia Campos]

(In reply to Zan Dobersek from comment #8)
> Comment on attachment 359033 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=359033&action=review
> 
> > Source/WebCore/platform/graphics/WidthIterator.cpp:206
> > +        bool characterMustDrawSomething = !isDefaultIgnorableCodePoint(character);
> > +#if USE(FREETYPE)
> > +        if (!characterMustDrawSomething) {
> > +            textIterator.advance(advanceLength);
> > +            continue;
> > +        }
> > +#endif
> 
> This treats ignorable characters as if there's no glyph representing such
> characters in font data. Can't this logic be used in GlyphPage::fill(),
> enforcing no glyph representation for characters that are ignorable?

No, that's what broke emoji sequences, see bug #177040. In GlyphPage::fill() we now replace ignorable characters with zero width space only when the font doesn't have a glyph, assuming that they will be handled or ignored later when shaping (as harfbuzz does). But the simple text code path is only ignoring them when they don't have a glyph. I think they should never be rendered even if they have a glyph.

[Carlos Garcia Campos]

Created attachment 359677 [details]
Patch

[Carlos Garcia Campos]

Committed r240231: <https://trac.webkit.org/changeset/240231>