Bug 255707 – AX: CSS speak-as, AXAccessKey, aria-owns, and URL AX APIs don't work for display:contents elements

• Component: Accessibility
• Status: RESOLVED FIXED

[Tyler Wilcock]

...

[Radar WebKit Bug Importer]

<rdar://problem/108301432>

[Tyler Wilcock]

Created attachment 465999 [details]
Patch

[Tyler Wilcock]

Created attachment 466001 [details]
Patch

[Tyler Wilcock]

Created attachment 466002 [details]
Patch

[Andres Gonzalez]

(In reply to Tyler Wilcock from comment #4)
> Created attachment 466002 [details]
> Patch

--- a/Source/WebCore/accessibility/AccessibilityObject.cpp
+++ b/Source/WebCore/accessibility/AccessibilityObject.cpp

+String AccessibilityObject::accessKey() const
+{
+    auto* element = this->element();
+    return element ? element->attributeWithoutSynchronization(accesskeyAttr) : String();
+}

As we re-org the code, shouldn't this one that requires an Element be in AXNodeObject?

+bool AccessibilityObject::supportsARIAOwns() const
+{
+    return !getAttribute(aria_ownsAttr).isEmpty();
+}

Same here.


diff --git a/LayoutTests/accessibility/display-contents-aria-owns.html b/LayoutTests/accessibility/display-contents-aria-owns.html
new file mode 100644
index 000000000000..4fcf823664ca
--- /dev/null
+++ b/LayoutTests/accessibility/display-contents-aria-owns.html

If we are going to have many tests named display-contents-*, maybe it makes sense to put them under a display-contents dir. On the other hand, it makes me wonder if we should add the display:contents test cases as part of the existing test, i.e., added to the aria-owns tests, like you did for the accesskey and the other tests.

--- a/LayoutTests/accessibility/mac/accesskey.html
+++ b/LayoutTests/accessibility/mac/accesskey.html

+    var link = window.accessibilityController.accessibleElementById("link");

Almost everywhere else we reference the accessibilityController as a global without window.

+    output += expect("input.allAttributes().indexOf('AXAccessKey: q') !== -1", "true");

Why do we need allAttributes() and not get the accesskey alone?

[Tyler Wilcock]

> +String AccessibilityObject::accessKey() const
> +{
> +    auto* element = this->element();
> +    return element ?
> element->attributeWithoutSynchronization(accesskeyAttr) : String();
> +}
> 
> As we re-org the code, shouldn't this one that requires an Element be in
> AXNodeObject?
> 
> +bool AccessibilityObject::supportsARIAOwns() const
> +{
> +    return !getAttribute(aria_ownsAttr).isEmpty();
> +}
Fixed.

> diff --git a/LayoutTests/accessibility/display-contents-aria-owns.html
> b/LayoutTests/accessibility/display-contents-aria-owns.html
> new file mode 100644
> index 000000000000..4fcf823664ca
> --- /dev/null
> +++ b/LayoutTests/accessibility/display-contents-aria-owns.html
> 
> If we are going to have many tests named display-contents-*, maybe it makes
> sense to put them under a display-contents dir. On the other hand, it makes
> me wonder if we should add the display:contents test cases as part of the
> existing test, i.e., added to the aria-owns tests, like you did for the
> accesskey and the other tests.
I did try to add a display contents case to an existing aria-owns test, but it was a bit clunky so opted to just make a separate one. A separate display-contents folder makes sense -- will consider for a future patch.

> --- a/LayoutTests/accessibility/mac/accesskey.html
> +++ b/LayoutTests/accessibility/mac/accesskey.html
> 
> +    var link = window.accessibilityController.accessibleElementById("link");
> 
> Almost everywhere else we reference the accessibilityController as a global
> without window.
Fixed.

> +    output += expect("input.allAttributes().indexOf('AXAccessKey: q') !==
> -1", "true");
> 
> Why do we need allAttributes() and not get the accesskey alone?
I thought that existing behavior was weird too, but it actually has the advantage of checking that the attribute is supported in addition to having the right value (because allAttributes() only queries for supported attributes).

Further more, there is no existing AccessibilityUIElement functionality for accesskey querying, and this existed before my patch, so let's leave it as-is with all the other display:contents work we have to go.

[Tyler Wilcock]

Created attachment 466017 [details]
Patch

[EWS]

Committed 263205@main (204cc1e1e3a3): <https://commits.webkit.org/263205@main>

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