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
Created attachment 359029 [details] Patch
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
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
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
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
Comment on attachment 359029 [details] Patch New failing tests: fast/text/soft-hyphen-min-preferred-width.html fast/text/format-control.html
Created attachment 359033 [details] Patch Made the change specific to FreeType to fix the test failures in mac.
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?
(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.
Created attachment 359677 [details] Patch
Committed r240231: <https://trac.webkit.org/changeset/240231>