Bug 249688 - AX: Expose bold and italic font styling as text attributes for macOS
Summary: AX: Expose bold and italic font styling as text attributes for macOS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: Safari 16
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-12-20 17:02 PST by Tommy McHugh
Modified: 2022-12-21 13:31 PST (History)
10 users (show)

See Also:


Attachments
Patch (6.25 KB, patch)
2022-12-20 17:12 PST, Tommy McHugh
no flags Details | Formatted Diff | Diff
Patch (6.98 KB, patch)
2022-12-20 22:23 PST, Tommy McHugh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tommy McHugh 2022-12-20 17:02:21 PST
Right now we expose bold and italic font styling for iOS but not macOS which prevents VoiceOver from knowing if some web-based fonts are bold or italic.
Comment 1 Radar WebKit Bug Importer 2022-12-20 17:02:33 PST
<rdar://problem/103577386>
Comment 2 Tommy McHugh 2022-12-20 17:12:11 PST
Created attachment 464129 [details]
Patch
Comment 3 chris fleizach 2022-12-20 17:24:00 PST
Comment on attachment 464129 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=464129&action=review

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm:658
> +        [fontAttributes setValue:@YES forKey:@"AXFontBold"];

do we not have keys for these already? 
does iOS not use AXAttributedStringSetFont ?
Comment 4 Tommy McHugh 2022-12-20 17:33:23 PST
(In reply to chris fleizach from comment #3)
> do we not have keys for these already? 
> does iOS not use AXAttributedStringSetFont ?
iOS does use AXttributedStringSetFont and uses UIAccessibilityTokenBold/Italic. MacOS doesn't have an existing key so we could either add one like this patch currently does that follows macOS attribute naming style or we could just untarget the iOS code so that it compiles for macOS and iOS and expose the iOS keys as those are defined in this file for all platforms.
Comment 5 chris fleizach 2022-12-20 21:58:53 PST
(In reply to Tommy McHugh from comment #4)
> (In reply to chris fleizach from comment #3)
> > do we not have keys for these already? 
> > does iOS not use AXAttributedStringSetFont ?
> iOS does use AXttributedStringSetFont and uses
> UIAccessibilityTokenBold/Italic. MacOS doesn't have an existing key so we
> could either add one like this patch currently does that follows macOS
> attribute naming style or we could just untarget the iOS code so that it
> compiles for macOS and iOS and expose the iOS keys as those are defined in
> this file for all platforms.

On macOS, it looks like the proper way to get bold status is use the font text attributes

[attributedString addAttribute:NSAccessibilityFontTextAttribute value:fontAttributes.get() range:range];

and then recreate your font to get the bold status

 CTFontRef fontRef = _copyFontRefForFontAttributes(fontAttributes);
    if (fontRef != NULL)
    {
        CTFontSymbolicTraits fontTraits = CTFontGetSymbolicTraits(fontRef);
        bold = (fontTraits & kCTFontBoldTrait);
Comment 6 chris fleizach 2022-12-20 22:05:27 PST
new test failure

+++ /Volumes/Data/worker/macOS-BigSur-Release-WK2-Tests-EWS/build/layout-test-results/accessibility/content-editable-as-textarea-actual.txt
@@ -19,6 +19,7 @@
 }worl{
     AXBackgroundColor = " [ (kCGColorSpaceICCBased; kCGColorSpaceModelRGB; sRGB IEC61966-2.1)] ( 0 0 0 0 )";
     AXFont =     {
+        AXFontBold = 1;
         AXFontFamily = Times;
         AXFontName = "Times-Bold";
         AXFontSize = 16;
Comment 7 Tommy McHugh 2022-12-20 22:23:07 PST
Created attachment 464143 [details]
Patch
Comment 8 Tommy McHugh 2022-12-21 12:14:57 PST
All the tests are now passing. Chris, if you think this is ready, could you please cq+?
Comment 9 EWS 2022-12-21 13:31:48 PST
Committed 258212@main (9a11844523ca): <https://commits.webkit.org/258212@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 464143 [details].