Expose the correct role, subrole and role description properties for the <dialog> element.

[Andres Gonzalez]

Expose the AXRoleDescription property for objects with role alert and dialog.

[Radar WebKit Bug Importer]

<rdar://problem/88684714>

[Andres Gonzalez]

Created attachment 451363 [details]
Patch

[James Craig]

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.

[James Craig]

Would be good to include a layout test of <div role=dialog> and <dialog open>

[Andres Gonzalez]

Created attachment 451630 [details]
Patch

[Andres Gonzalez]

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.

[Andres Gonzalez]

(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.

[chris fleizach]

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

[Andres Gonzalez]

Created attachment 451684 [details]
Patch

[Andres Gonzalez]

(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.

[chris fleizach]

Need to update the GTK results I think for this test too

[Andres Gonzalez]

Created attachment 451700 [details]
Patch

[EWS]

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].