Bug 236359

Summary: Expose the correct role, subrole and role description properties for the <dialog> element.
Product: WebKit Reporter: Andres Gonzalez <andresg_22>
Component: AccessibilityAssignee: Andres Gonzalez <andresg_22>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, andresg_22, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, jdiggs, ntim, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Andres Gonzalez 2022-02-09 05:33:06 PST
Expose the AXRoleDescription property for objects with role alert and dialog.
Comment 1 Radar WebKit Bug Importer 2022-02-09 05:33:18 PST
<rdar://problem/88684714>
Comment 2 Andres Gonzalez 2022-02-09 05:40:33 PST
Created attachment 451363 [details]
Patch
Comment 3 James Craig 2022-02-09 09:05:15 PST
Comment on attachment 451363 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Expose the AXRoleDescription property for objects with role alert and dialog.

Clarity: “alertdialog and dialog” not “alert and dialog” role. This isn’t for the transient non-dialog “alert” role.

> Source/WebCore/platform/LocalizedStrings.cpp:520
> +    return WEB_UI_STRING("alert", "accessibility role description for an alert");

Role desc of “alert” (e.g. not the more verbose “alert dialog”) seems okay for alert dialogs though, so I think the diff itself is good to go.
Comment 4 James Craig 2022-02-09 09:09:18 PST
Would be good to include a layout test of <div role=dialog> and <dialog open>
Comment 5 Andres Gonzalez 2022-02-10 19:23:30 PST
Created attachment 451630 [details]
Patch
Comment 6 Andres Gonzalez 2022-02-10 19:25:50 PST
In further investigation turns out that the behavior is already correct for elements with role="dialog". Thus this patch implements the same behavior for <dialog> elements.
Comment 7 Andres Gonzalez 2022-02-10 19:28:16 PST
(In reply to James Craig from comment #4)
> Would be good to include a layout test of <div role=dialog> and <dialog open>

Added a new layout test for <dialog>. role="dialog" is already implemented and covered in tests. Thanks.
Comment 8 chris fleizach 2022-02-10 21:37:18 PST
Comment on attachment 451630 [details]
Patch

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

> Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm:152
> +    // INclude <dialog> elements and elements with role="dialog".

INclude -> Include

> Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm:153
> +    if (roleValue() == AccessibilityRole::ApplicationDialog)

is this Mac only? seems like it might be good for this to be accessibilityIsIgnored()==true in the base class
Comment 9 Andres Gonzalez 2022-02-11 06:26:39 PST
Created attachment 451684 [details]
Patch
Comment 10 Andres Gonzalez 2022-02-11 06:29:06 PST
(In reply to chris fleizach from comment #8)
> Comment on attachment 451630 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=451630&action=review
> 
> > Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm:152
> > +    // INclude <dialog> elements and elements with role="dialog".
> 
> INclude -> Include
> 
> > Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm:153
> > +    if (roleValue() == AccessibilityRole::ApplicationDialog)
> 
> is this Mac only? seems like it might be good for this to be
> accessibilityIsIgnored()==true in the base class

Fixed typo and moved it to the platform-independent code.
Comment 11 chris fleizach 2022-02-11 07:27:20 PST
Need to update the GTK results I think for this test too
Comment 12 Andres Gonzalez 2022-02-11 08:23:59 PST
Created attachment 451700 [details]
Patch
Comment 13 EWS 2022-02-13 08:01:47 PST
Committed r289713 (247198@main): <https://commits.webkit.org/247198@main>

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