Bug 237817 - [Cocoa] When accessibility bold is enabled, font-family:system-ui is supposed to be bold
Summary: [Cocoa] When accessibility bold is enabled, font-family:system-ui is supposed...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
: 239364 239365 239367 239368 239369 239371 239373 239374 (view as bug list)
Depends on: 241112 241329 241360 241404 241454 241506 241537 241770 241995 242021 242023
Blocks:
  Show dependency treegraph
 
Reported: 2022-03-12 22:31 PST by Myles C. Maxfield
Modified: 2022-06-27 12:42 PDT (History)
12 users (show)

See Also:


Attachments
Patch (18.43 KB, patch)
2022-03-12 22:44 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (23.72 KB, patch)
2022-03-23 14:37 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (23.66 KB, patch)
2022-03-23 14:44 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (23.72 KB, patch)
2022-03-23 23:45 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2022-03-12 22:31:18 PST
[Cocoa] When accessibility bold is enabled, font-family:system-ui is supposed to be bold
Comment 1 Myles C. Maxfield 2022-03-12 22:44:54 PST
Created attachment 454551 [details]
Patch
Comment 2 Myles C. Maxfield 2022-03-12 22:46:13 PST
<rdar://problem/88249241>
Comment 3 Myles C. Maxfield 2022-03-22 17:12:07 PDT
I ran the same command the bots ran (except with a debug build instead of a release build) and it reported:

=> Results: 100/100 tests passed (100.0%)

🤔
Comment 4 Myles C. Maxfield 2022-03-23 11:37:30 PDT
Hang on, the test didn't fail on iOS (which is the one platform with accessibility bold). I think I know what's happening.
Comment 5 Myles C. Maxfield 2022-03-23 13:42:38 PDT
Oh, USE(NON_VARIABLE_SYSTEM_FONT) is enabled on Big Sur. That explains it.
Comment 6 Myles C. Maxfield 2022-03-23 14:37:52 PDT
Created attachment 455557 [details]
Patch
Comment 7 Myles C. Maxfield 2022-03-23 14:44:01 PDT
Created attachment 455561 [details]
Patch
Comment 8 Myles C. Maxfield 2022-03-23 23:45:10 PDT
Created attachment 455613 [details]
Patch
Comment 9 EWS 2022-03-24 21:09:14 PDT
Committed r291846 (248858@main): <https://commits.webkit.org/248858@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455613 [details].
Comment 10 Alexey Proskuryakov 2022-04-14 18:07:51 PDT
Looks like this was reverted in bug 239323.
Comment 11 Myles C. Maxfield 2022-04-14 18:58:28 PDT
Patch was reverted - reopening.
Comment 12 Myles C. Maxfield 2022-06-10 21:19:13 PDT
After some internal deliberations, we think the right way to handle this is to do it in a way that is similar to the previous shipping way (when that worked) and not like accessibility font sizing.

Accessibility font sizing makes applyValueFontSize() aware of the accessibility font sizes. However, we want to implement accessibility bold at a lower level (rather than applyValueFontWeight) because:

1. It's compatible with shipping behavior. getComputedStyle() reports a font weight of 400 when accessibility font sizes are enabled, so it would be nice if we could preserve that behavior
2. Accessibility bold needs to happen for all system fonts, even ones triggered by "font-family: system-ui". If we did this at the style resolution level, this would mean one property (font-family) would have an affect on another property (font-weight). We _do_ have precedent for this (where "font-family: monospace" affects the font-size property) but this precedent is generally considered a mistake and a wart on the web platform.
3. Accessibility bold is supposed to bolden just the characters/glyphs from the system font, not all characters/glyphs in the element. If the content said "font-family: someFont, system-ui" then if we did this at the CSS level, then either A) the whole element would get bold, even the glyphs coming from someFont, or B) none of the element would get bold, even the glyphs coming from the system font.

Because of that logic, we're going to handle this inside preparePlatformFont() rather than in applyValueFontFamily.
Comment 13 Myles C. Maxfield 2022-06-27 10:50:58 PDT
Pull request: https://github.com/WebKit/WebKit/pull/1823
Comment 14 Myles C. Maxfield 2022-06-27 11:00:26 PDT
*** Bug 239364 has been marked as a duplicate of this bug. ***
Comment 15 Myles C. Maxfield 2022-06-27 11:00:29 PDT
*** Bug 239365 has been marked as a duplicate of this bug. ***
Comment 16 Myles C. Maxfield 2022-06-27 11:00:33 PDT
*** Bug 239367 has been marked as a duplicate of this bug. ***
Comment 17 Myles C. Maxfield 2022-06-27 11:00:37 PDT
*** Bug 239368 has been marked as a duplicate of this bug. ***
Comment 18 Myles C. Maxfield 2022-06-27 11:00:40 PDT
*** Bug 239369 has been marked as a duplicate of this bug. ***
Comment 19 Myles C. Maxfield 2022-06-27 11:00:44 PDT
*** Bug 239371 has been marked as a duplicate of this bug. ***
Comment 20 Myles C. Maxfield 2022-06-27 11:00:50 PDT
*** Bug 239373 has been marked as a duplicate of this bug. ***
Comment 21 Myles C. Maxfield 2022-06-27 11:00:55 PDT
*** Bug 239374 has been marked as a duplicate of this bug. ***
Comment 22 EWS 2022-06-27 12:42:35 PDT
Committed 251883@main (3e3f9ab0f79b): <https://commits.webkit.org/251883@main>

Reviewed commits have been landed. Closing PR #1823 and removing active labels.