Summary: | [Win][Uniscribe] Material icons containing underscore or numbers aren't shown because ScriptItemize splits them apart | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Fujii Hironori <Hironori.Fujii> | ||||||||||||||
Component: | Text | Assignee: | Fujii Hironori <Hironori.Fujii> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | achristensen, bfulgham, don.olmstead, mmaxfield, pfeldman, pvollan, ross.kirsling, webkit-bug-importer | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | 201213 | ||||||||||||||||
Bug Blocks: | |||||||||||||||||
Attachments: |
|
Description
Fujii Hironori
2019-08-28 02:25:17 PDT
Created attachment 377437 [details]
screenshot
Google News is also using Material icons. https://news.google.com/ 'star_border' shows 'star' icons. 'directions_bike' shows 'directions' icons. Screenshot of Google News: https://ibb.co/CWqJw7L Yikes. Are we calling ScriptItemize incorrectly? [BUG] Webkit font rendering (spacing / icon fonts) · Issue #2626 · microsoft/playwright https://github.com/microsoft/playwright/issues/2626 Playwright's downstream patch: https://github.com/pavelfeldman/webkit/commit/ee5fd605f6d9ea3391317e269ebb5d948e896464 Created attachment 404428 [details]
WIP patch
Created attachment 404481 [details]
Patch
Interestingly, mac and ios testing EWS failed. I created a ligature font to test the patch. I don't know this is a Mac bug or my font bug. I'm going to check it. Created attachment 404547 [details]
Patch
Created attachment 404697 [details]
Patch
Comment on attachment 404697 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404697&action=review r=me with nits > Source/WebCore/platform/graphics/win/ComplexTextControllerUniscribe.cpp:187 > + if (font->platformData().isSystemFont() || font->platformData().hasVariations()) Will variations never hit this same issue? If not then remove it. > LayoutTests/ChangeLog:10 > + * fonts/ligature.woff: Added. Do you maybe want to add where it came from in case of licensing questions in the future. Comment on attachment 404697 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404697&action=review >> Source/WebCore/platform/graphics/win/ComplexTextControllerUniscribe.cpp:187 >> + if (font->platformData().isSystemFont() || font->platformData().hasVariations()) > > Will variations never hit this same issue? If not then remove it. Will check. >> LayoutTests/ChangeLog:10 >> + * fonts/ligature.woff: Added. > > Do you maybe want to add where it came from in case of licensing questions in the future. Agreed. It was created by me. It contains only ▲ and ■ glyphs. Comment on attachment 404697 [details]
Patch
How is this supposed to work? What do non-browser Windows apps do when they want to use material icons?
Myles, you are right. ScriptItemize has a option (fMergeNeutralItems) to control it. https://docs.microsoft.com/en-us/windows/win32/api/usp10/ns-usp10-script_control Created attachment 404893 [details]
Patch
Comment on attachment 404893 [details] Patch Clearing flags on attachment: 404893 Committed r264722: <https://trac.webkit.org/changeset/264722> All reviewed patches have been landed. Closing bug. |