Bug 209591

Summary: [HarfBuzz] Not all CSS font features are applied
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aperez, bugs-noreply, ews-watchlist, mmaxfield, webkit-bug-importer
Priority: P2 Keywords: Gtk, InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=206885
Bug Depends on:    
Bug Blocks: 209593    
Attachments:
Description Flags
Patch
aperez: review-, aperez: commit-queue-
Patch aperez: review+

Carlos Garcia Campos
Reported 2020-03-26 03:47:39 PDT
We need to implement https://www.w3.org/TR/css-fonts-3/#feature-precedence
Attachments
Patch (27.09 KB, patch)
2020-03-26 03:53 PDT, Carlos Garcia Campos 		aperez: review-
aperez: commit-queue-
DetailsFormatted DiffDiff
Patch (26.86 KB, patch)
2020-03-26 05:40 PDT, Carlos Garcia Campos 		aperez: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1 2020-03-26 03:53:46 PDT
Created attachment 394590 [details] Patch
Adrian Perez
Comment 2 2020-03-26 04:54:33 PDT
Comment on attachment 394590 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394590&action=review Logic looks good. There is one thing that I find hard to believe that it would be working, please check below ⬇️ > Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp:570 > + char buffer[5]; If it was me, I would probably have done instead: const auto& tag = fontFaceFeature.tag(); const char buffer[] = { tag[0], tag[1], tag[2], tag[3], '\0' }; FcPatternAddString(resultPattern.get(), FC_FONT_FEATURES, reinterpret_cast<const FcChar*>(buffer)); But in the end both are equivalent, so no big deal 😉️ > Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp:577 > + FcPatternAddString(resultPattern.get(), FC_FONT_FEATURES, reinterpret_cast<const FcChar8*>(&buffer)); Due to array-to-pointer decay with “&buffer” jere you are passing a “char**” so I do not know how this can possibly work correctly. We need to plainly pass “buffer” here. Using “FcPatternAddString(..., buffer)” might render the cast as unneeded, too (I don't remember whether “FcChar8” is exactly the same as “char”, though). > Source/WebCore/platform/graphics/freetype/FontCustomPlatformDataFreeType.cpp:94 > + FcPatternAddString(pattern.get(), FC_FONT_FEATURES, reinterpret_cast<const FcChar8*>(&buffer)); Same here.
Carlos Garcia Campos
Comment 3 2020-03-26 05:40:11 PDT
Created attachment 394596 [details] Patch
Carlos Garcia Campos
Comment 4 2020-03-30 02:11:22 PDT
Committed r259188: <https://trac.webkit.org/changeset/259188>
Radar WebKit Bug Importer
Comment 5 2020-03-30 02:12:16 PDT
<rdar://problem/61046669>
Note You need to log in before you can comment on or make changes to this bug.