Bug 201214

Summary:

[Win][Uniscribe] Material icons containing underscore or numbers aren't shown because ScriptItemize splits them apart

Product:

WebKit

Reporter:

Fujii Hironori <fujii.hironori>

Component:

Text

Assignee:

Fujii Hironori <fujii.hironori>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, bfulgham, don.olmstead, mmaxfield, pfeldman, pvollan, ross.kirsling, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

201213

Bug Blocks:

Attachments:

Description	Flags
screenshot	none
WIP patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Fujii Hironori

Reported 2019-08-28 02:25:17 PDT

[Win][UniscribeController] Material icons containing underscore or numbers aren't shown because ScriptItemize splits them apart For example, 'accessible_forward' are split as three items (accessible _ forward). Then, 'accessible' icon is shown incorrectly. '3d_rotation' is split as four items (3 d _ rotation). https://material.io/resources/icons/

Attachments
screenshot (67.74 KB, image/png) 2019-08-28 02:26 PDT, Fujii Hironori	no flags	Details
WIP patch (2.29 KB, patch) 2020-07-16 00:54 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Patch (7.57 KB, patch) 2020-07-16 14:25 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Patch (7.51 KB, patch) 2020-07-17 00:52 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Patch (7.46 KB, patch) 2020-07-19 21:30 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Patch (5.64 KB, patch) 2020-07-21 19:49 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Fujii Hironori

Comment 1 2019-08-28 02:26:21 PDT

Created attachment 377437 [details] screenshot

Fujii Hironori

Comment 2 2019-08-28 02:32:24 PDT

Google News is also using Material icons. https://news.google.com/ 'star_border' shows 'star' icons. 'directions_bike' shows 'directions' icons.

Fujii Hironori

Comment 3 2019-08-28 02:47:57 PDT

Screenshot of Google News: https://ibb.co/CWqJw7L

Myles C. Maxfield

Comment 4 2019-08-28 10:40:37 PDT

Yikes. Are we calling ScriptItemize incorrectly?

Fujii Hironori

Comment 5 2020-06-18 12:48:12 PDT

[BUG] Webkit font rendering (spacing / icon fonts) · Issue #2626 · microsoft/playwright https://github.com/microsoft/playwright/issues/2626

Fujii Hironori

Comment 6 2020-07-16 00:54:29 PDT

Playwright's downstream patch: https://github.com/pavelfeldman/webkit/commit/ee5fd605f6d9ea3391317e269ebb5d948e896464

Fujii Hironori

Comment 7 2020-07-16 00:54:55 PDT

Created attachment 404428 [details] WIP patch

Fujii Hironori

Comment 8 2020-07-16 14:25:59 PDT

Created attachment 404481 [details] Patch

Fujii Hironori

Comment 9 2020-07-16 18:06:12 PDT

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.

Fujii Hironori

Comment 10 2020-07-17 00:52:44 PDT

Created attachment 404547 [details] Patch

Fujii Hironori

Comment 11 2020-07-19 21:30:25 PDT

Created attachment 404697 [details] Patch

Don Olmstead

Comment 12 2020-07-21 13:34:59 PDT

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.

Fujii Hironori

Comment 13 2020-07-21 13:57:00 PDT

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.

Myles C. Maxfield

Comment 14 2020-07-21 14:33:44 PDT

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?

Fujii Hironori

Comment 15 2020-07-21 19:24:27 PDT

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

Fujii Hironori

Comment 16 2020-07-21 19:49:05 PDT

Created attachment 404893 [details] Patch

Fujii Hironori

Comment 17 2020-07-22 13:04:23 PDT

Comment on attachment 404893 [details] Patch Clearing flags on attachment: 404893 Committed r264722: <https://trac.webkit.org/changeset/264722>

Fujii Hironori

Comment 18 2020-07-22 13:04:27 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 19 2020-07-22 13:05:16 PDT

<rdar://problem/65951341>

Note You need to log in before you can comment on or make changes to this bug.