Bug 206253 – AX: Support ARIA 1.3 property aria-description

• Component: Accessibility
• Status: RESOLVED FIXED

[Aaron Leventhal]

The new ARIA 1.3 (still in draft) aria-description property can be used to provide a flat string description.
It's implemented in Chrome 81, currently behind a flag:
--enable-features=AccessibilityExposeARIAAnnotations

References:
ARIA spec issue: https://github.com/w3c/aria/issues/891
ARIA spec pull request: https://github.com/w3c/aria/pull/1137
Chrome issue: https://bugs.chromium.org/p/chromium/issues/detail?id=1006767

[Radar WebKit Bug Importer]

<rdar://problem/58577548>

[Aaron Leventhal]

aria-description has now landed in ARIA 1.3 master branch (draft)
https://w3c.github.io/aria/#aria-description

[Tommy McHugh]

Created attachment 435909 [details]
Patch

[chris fleizach]

Comment on attachment 435909 [details]
Patch

I don't think aria-description is an approved attribute for ARIA

[chris fleizach]

(In reply to chris fleizach from comment #4)
> Comment on attachment 435909 [details]
> Patch
> 
> I don't think aria-description is an approved attribute for ARIA

I think it might have been removed from 1.2

https://www.w3.org/TR/wai-aria-1.2/

[Tommy McHugh]

(In reply to chris fleizach from comment #5)
> (In reply to chris fleizach from comment #4)
> > Comment on attachment 435909 [details]
> > Patch
> > 
> > I don't think aria-description is an approved attribute for ARIA
> 
> I think it might have been removed from 1.2
> 
> https://www.w3.org/TR/wai-aria-1.2/

Ah, you're right, my bad. It's part of 1.3, which I don't believe is standardized yet. The title misled me :).

[Joanmarie Diggs]

FWIW, we (the ARIA WG) don't anticipate it being removed from the spec.

[chris fleizach]

(In reply to Joanmarie Diggs (irc: joanie) from comment #7)
> FWIW, we (the ARIA WG) don't anticipate it being removed from the spec.

@James - should we add this now?

[Aaron Leventhal]

Here's my argument for using AXHelp:

It's the parallel of aria-label, e.g. you *could* use aria-labelledby on hidden content to generate a name, but aria-label is just easier.
In this case, aria-description is the shorter way of using aria-describedby with hidden content.

It would be weird if these created different results:
<textarea aria-describedby="hidden-description"></textarea>
<div style="display: none" id="hidden-description">My descriptions</div>
vs.
<textarea aria-descriptjon="My description"></textarea>

Looking at the WebKit tests, it looks like aria-describedby maps to AXHelp, e.g. in this expectations file. See https://github.com/WebKit/WebKit/blob/8afe31a018b11741abdf9b4d5bb973d7c1d9ff05/LayoutTests/accessibility/mac/aria-describedby-fieldset-expected.txt

Thanks for any feedback on this.

[Charlie Croom]

Hi! My friendly bump from Twitter that we think this would be a great and quick way to make accessibility easier to support and implement and could push the web forward. Firefox + Chromium have supported this for quite some time now, and given it's unlikely to be removed according to the comment above. This brings real benefits to devs and users!

[chris fleizach]

(In reply to Charlie Croom from comment #10)
> Hi! My friendly bump from Twitter that we think this would be a great and
> quick way to make accessibility easier to support and implement and could
> push the web forward. Firefox + Chromium have supported this for quite some
> time now, and given it's unlikely to be removed according to the comment
> above. This brings real benefits to devs and users!

FYI we're thinking we can put this in AXCustomContent, so it doesn't conflict with existing help attributes

[Aaron Leventhal]

Thanks for the update.

From an authoring/user POV, will it act differently than when you use aria-describedby to point to a hidden div? If so, that would be weird. It would be like aria-label working differently than aria-labelledby pointing to a hidden div.

[chris fleizach]

(In reply to Aaron Leventhal from comment #12)
> Thanks for the update.
> 
> From an authoring/user POV, will it act differently than when you use
> aria-describedby to point to a hidden div? If so, that would be weird. It
> would be like aria-label working differently than aria-labelledby pointing
> to a hidden div.

Right now described-by is jammed into the help text. I don't know if that's the right place for it, now that we have the AXCustomContent paradigm. So I think a change for this property would also change aria-describedby

[Aaron Leventhal]

That makes sense, as long as it's the same as aria-describedby.

What versions of VoiceOver will work with AXCustomContent? I mean, if we change Chrome to using that for aria-describedby, will we break existing content?
Would we need to check the OS version before deciding how to expose it?

Thanks again for the dialogue.

[chris fleizach]

(In reply to Aaron Leventhal from comment #14)
> That makes sense, as long as it's the same as aria-describedby.
> 
> What versions of VoiceOver will work with AXCustomContent? I mean, if we
> change Chrome to using that for aria-describedby, will we break existing
> content?
> Would we need to check the OS version before deciding how to expose it?
> 

iOS 14 and macOS 11 should support AXCustomContent

> Thanks again for the dialogue.

[Aaron Leventhal]

MacOS 11+ should probably be enough.
Blink doesn't run on iOS, Chrome uses WebKit there.

[Aaron Leventhal]

Chris, does the strategy in comment 15 mean that we should expose aria-description/describedby as AXCustomContent when MacOS version >= 11, otherwise expose as AXHelp, for backward compatibility?

[chris fleizach]

(In reply to Aaron Leventhal from comment #17)
> Chris, does the strategy in comment 15 mean that we should expose
> aria-description/describedby as AXCustomContent when MacOS version >= 11,
> otherwise expose as AXHelp, for backward compatibility?

Yes that's the plan

[RobLi]

This is supported in most popular browsers and screen readers by now: https://a11ysupport.io/tech/aria/aria-description_attribute#support-table-0

[chris fleizach]

Created attachment 461239 [details]
Patch

[Andres Gonzalez]

(In reply to chris fleizach from comment #20)
> Created attachment 461239 [details]
> Patch

--- a/Source/WebCore/PAL/PAL.xcodeproj/project.pbxproj
+++ a/Source/WebCore/PAL/PAL.xcodeproj/project.pbxproj
@@ -1804,6 +1811,7 @@
                 DD20DE4527BC90D80093D175 /* PopupMenu.h in Headers */,
                 E34F26F62846D0D90076E549 /* PowerLogSPI.h in Headers */,
                 DD20DE0427BC90D80093D175 /* pthreadSPI.h in Headers */,
+                5CB898B2286274FA00CA3485 /* QuarantineSPI.h in Headers */,
                 DD20DE0527BC90D80093D175 /* QuartzCoreSPI.h in Headers */,
                 DD20DE3F27BC90D80093D175 /* QuickLookMacSPI.h in Headers */,
                 DD20DDC127BC90D70093D175 /* QuickLookSoftLink.h in Headers */,
@@ -1991,7 +1999,6 @@
                 DD20DDAE27BC90D70093D175 /* WebGPUTextureAspect.h in Headers */,
                 DD20DDAF27BC90D70093D175 /* WebGPUTextureBindingLayout.h in Headers */,
                 DD20DDB027BC90D70093D175 /* WebGPUTextureDescriptor.h in Headers */,
-                5CB898B2286274FA00CA3485 /* QuarantineSPI.h in Headers */,
                 DD20DDB127BC90D70093D175 /* WebGPUTextureDimension.h in Headers */,
                 DD20DDB227BC90D70093D175 /* WebGPUTextureFormat.h in Headers */,
                 DD20DD3E27BC90D60093D175 /* WebGPUTextureImpl.h in Headers */,

Artifact from editing the proj file?

--- a/Source/WebCore/PAL/pal/cocoa/AccessibilitySoftLink.mm
+++ a/Source/WebCore/PAL/pal/cocoa/AccessibilitySoftLink.mm

+/*
+ * Copyright (C) 2019 Apple Inc. All rights reserved.

Shouldn't it be 2022 for a new file?

--- a/Source/WebCore/accessibility/AccessibilityObjectInterface.h
+++ a/Source/WebCore/accessibility/AccessibilityObjectInterface.h

+    virtual String ariaDescription() const = 0;

Name it just description(). In the interface we already have accessibilityDescription(), descriptionAttributeValue()... can we consolidate all of this in a single description() method?

After looking at the rest of the change, perhaps we can call this customContent()?

--- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
+++ a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp

+    if (object.ariaDescription().length())
+        setProperty(AXPropertyName::ARIADescription, object.ariaDescription());

object.ariaDescription().isolatedCopy()

Also I would call it only once by keeping it in a temp var, in case it grows into a non-trivial call, it is not an inline already.

Eventually we want to move the check for the 0 length into the setProperty itself, but that would do for now.

--- a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm
+++ a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm

+#if HAVE(ACCESSIBILITY_FRAMEWORK)
+- (NSArray<AXCustomContent *> *)accessibilityCustomContent
+{
+    [self baseUpdateBackingStore];

auto* backingObject = [self baseUpdateBackingStore];
if (!backingObject)
    return nil;

+    NSMutableArray<AXCustomContent *> *accessibilityCustomContent = nil;
+    auto ariaDescription = self.axBackingObject->ariaDescription();

auto ariaDescription = backingObject->ariaDescription();

--- a/Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl
+++ a/Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl

+    DOMString customContent();

readonly attribute DOMString customContent();

--- a/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm
+++ a/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm

+JSRetainPtr<JSStringRef> AccessibilityUIElement::customContent() const
+{
+#if HAVE(ACCESSIBILITY_FRAMEWORK)
+    RetainPtr<NSMutableArray> customContent = adoptNS([[NSMutableArray alloc] init]);
+    s_controller->executeOnAXThreadAndWait([&customContent, this] {

We are not doing AX thread on iOS.

--- a/LayoutTests/accessibility/aria-description.html
+++ a/LayoutTests/accessibility/aria-description.html

+<script src="../resources/accessibility-helper.js"></script>

Don't need this, since not using any of that.

+        const slider = accessibilityController.accessibleElementById("button");

slider -> button

[chris fleizach]

(In reply to Andres Gonzalez from comment #21)
> (In reply to chris fleizach from comment #20)
> > Created attachment 461239 [details]
> > Patch
> 
> Artifact from editing the proj file?

Yes I've learned to just let Xcode do what it wants to do

> Name it just description(). In the interface we already have
> accessibilityDescription(), descriptionAttributeValue()... can we
> consolidate all of this in a single description() method?
> 
> After looking at the rest of the change, perhaps we can call this
> customContent()?

The way that I saw this is that the core web provides an attribute called aria-description. Each platform is going to decide how to expose that differently. on Mac/iOS we're able to use this existing, generic API, (custom content). But on GTK they might want to do something else.

So description() seemed too generic (it's also conflated with AXDescription which is in heavy use on the Mac). But customContent() is too specific.

ariaDescription() is the most accurate for what it does, but open to other ideas

Rest of comments make sense

[Andres Gonzalez]

(In reply to chris fleizach from comment #22)
> (In reply to Andres Gonzalez from comment #21)
> > (In reply to chris fleizach from comment #20)
> > > Created attachment 461239 [details]
> > > Patch
> > 
> > Artifact from editing the proj file?
> 
> Yes I've learned to just let Xcode do what it wants to do
> 
> > Name it just description(). In the interface we already have
> > accessibilityDescription(), descriptionAttributeValue()... can we
> > consolidate all of this in a single description() method?
> > 
> > After looking at the rest of the change, perhaps we can call this
> > customContent()?
> 
> The way that I saw this is that the core web provides an attribute called
> aria-description. Each platform is going to decide how to expose that
> differently. on Mac/iOS we're able to use this existing, generic API,
> (custom content). But on GTK they might want to do something else.
> 
> So description() seemed too generic (it's also conflated with AXDescription
> which is in heavy use on the Mac). But customContent() is too specific.
> 
> ariaDescription() is the most accurate for what it does, but open to other
> ideas

All what it does is getAttribute(aria_descriptionAttr), so if we want platform wrapper code to be able to retrieve HTmL attributes directly, we would be better off moving getAttribute back to the AXCoreObject interface. That way we don't pollute the core object interface with a method for each HTML attribute that we want to provide direct access to. We can go ahead with this change now though, and we can rework that later. 

In a more general comment, I would hope that we can abstract the AX API enough that we don't need to expose the underlying HTML attributes too often. Not clear why, for instance, we need to expose this particular HTML attribute separate from all the properties we already have to describe an element.

[chris fleizach]

Created attachment 461351 [details]
Patch

[Andres Gonzalez]

(In reply to chris fleizach from comment #24)
> Created attachment 461351 [details]
> Patch

--- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
+++ a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
@@ -397,6 +397,10 @@ void AXIsolatedObject::initializeProperties(Ref<AXCoreObject> coreObject, IsRoot

+    auto extendedDescription = object.extendedDescription().isolatedCopy();
+    if (extendedDescription.length())
+        setProperty(AXPropertyName::ExtendedDescription, extendedDescription);

I would make the isolatedCopy in the call to setProperty().

--- a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm
+++ a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm

+- (NSArray<AXCustomContent *> *)accessibilityCustomContent
+{
+    auto* backingObject = [self baseUpdateBackingStore];
+    if (!backingObject)
+        return nil;
+
+    NSMutableArray<AXCustomContent *> *accessibilityCustomContent = nil;
+    auto extendedDescription = backingObject->extendedDescription();
+    if (extendedDescription.length()) {
+        if (!accessibilityCustomContent)

This check is unnecessary, it will be nil.

[chris fleizach]

(In reply to Andres Gonzalez from comment #25)
> (In reply to chris fleizach from comment #24)
> > Created attachment 461351 [details]
> > Patch
> 
> --- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
> +++ a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
> @@ -397,6 +397,10 @@ void
> AXIsolatedObject::initializeProperties(Ref<AXCoreObject> coreObject, IsRoot
> 
> +    auto extendedDescription = object.extendedDescription().isolatedCopy();
> +    if (extendedDescription.length())
> +        setProperty(AXPropertyName::ExtendedDescription,
> extendedDescription);
> 
> I would make the isolatedCopy in the call to setProperty().
> 
> --- a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm
> +++ a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm
> 
> +- (NSArray<AXCustomContent *> *)accessibilityCustomContent
> +{
> +    auto* backingObject = [self baseUpdateBackingStore];
> +    if (!backingObject)
> +        return nil;
> +
> +    NSMutableArray<AXCustomContent *> *accessibilityCustomContent = nil;
> +    auto extendedDescription = backingObject->extendedDescription();
> +    if (extendedDescription.length()) {
> +        if (!accessibilityCustomContent)
> 
> This check is unnecessary, it will be nil.

Thanks, will fix

[chris fleizach]

Created attachment 461371 [details]
Patch

[chris fleizach]

Created attachment 461393 [details]
Patch

[chris fleizach]

Created attachment 461395 [details]
Patch

[chris fleizach]

iOS-wk2 failure

Thread 0 Crashed::   Dispatch queue: com.apple.main-thread
0   libobjc.A.dylib                          	       0x1a834448a objc_msgSend + 10
1   com.apple.WebKitTestRunner.InjectedBundle	       0x1e1c06c9c WTR::AccessibilityUIElement::customContent() const
2   com.apple.WebKitTestRunner.InjectedBundle	       0x1e1c25d2c WTR::JSAccessibilityUIElement::customContent(OpaqueJSContext const*, OpaqueJSValue*, OpaqueJSString*, OpaqueJSValue const**)
3   com.apple.JavaScriptCore                 	       0x1b4c44de2 JSC::JSCallbackObject<JSC::JSNonFinalObject>::getStaticValue(JSC::JSGlobalObject*, JSC::PropertyName)
4   com.apple.JavaScriptCore                 	       0x1b4c3c97a JSC::JSCallbackObject<JSC::JSNonFinalObject>::getOwnPropertySlot(JSC::JSObject*, JSC::JSGlobalObject*, JSC::PropertyName, JSC::PropertySlot&)
5   com.apple.JavaScriptCore                 	       0x1b5451d7e JSC::LLInt::performLLIntGetByID(JSC::BytecodeIndex, JSC::CodeBlock*, JSC::JSGlobalObject*, JSC::JSValue, JSC::Identifier const&, JSC::GetByIdModeMetadata&)
6   com.apple.JavaScriptCore                 	       0x1b5451150 llint_slow_path_get_by_id
7   com.apple.JavaScriptCore                 	       0x1b490bc46 llint_entry
8   com.apple.JavaScriptCore                 	       0x1b49017f8 vmEntryToJavaScript

[chris fleizach]

Created attachment 461435 [details]
Patch

[EWS]

Committed 253184@main (7c414e7250fb): <https://commits.webkit.org/253184@main>

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