[Webkit-unassigned] [Bug 248572] AX: Include AXKeyShortcutsValue in accessibilityAttributeNames when there is an aria-keyshortcuts attribute

Thu Dec 1 06:39:34 PST 2022

https://bugs.webkit.org/show_bug.cgi?id=248572

--- Comment #3 from Andres Gonzalez <andresg_22 at apple.com> ---
(In reply to Tommy McHugh from comment #2)
> Created attachment 463819 [details]
> Patch

--- a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
+++ b/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm

+#ifndef NSAccessibilityKeyShortcutsValue
+#define NSAccessibilityKeyShortcutsValue @"AXKeyShortcutsValue"
+#endif

While naming these constants, please drop the "Value", it doesn't add anything to the name, just makes it longer. Also The NSAccessibility names have traditionally ended in "Attribute", so if we keep tradition this should be:

+#ifndef NSAccessibilityKeyShortcutsAttribute
+#define NSAccessibilityKeyShortcutsAttribute @"AXKeyShortcuts"
+#endif

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

@@ -3747,6 +3749,9 @@ void AXObjectCache::updateIsolatedTree(const Vector<std::pair<RefPtr<Accessibili

+        case AXKeyShortcutsChanged:
+            tree->updateNodeProperties(*notification.first, { AXPropertyName::KeyShortcutsValue, AXPropertyName::SupportsKeyShortcutsValue });
+            break;

Does this compile? It should be:

+            tree->updateNodeProperties(*notification.first, AXPropertyName::KeyShortcutsValue);

--- a/Source/WebCore/accessibility/AccessibilityObjectInterface.h
+++ b/Source/WebCore/accessibility/AccessibilityObjectInterface.h
@@ -321,6 +321,7 @@ public:

+    virtual bool supportsKeyShortcutsValue() const = 0;

Drop Value from the name: supportsKeyShortcuts()

--- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp
+++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp
@@ -498,6 +498,9 @@ void AXIsolatedTree::updateNodeProperties(AXCoreObject& axObject, const Vector<A

+        case AXPropertyName::SupportsKeyShortcutsValue:
+            propertyMap.set(AXPropertyName::SupportsKeyShortcutsValue, axObject.supportsKeyShortcutsValue());
+            break;

It should be:

+        case AXPropertyName::KeyShortcuts:
+            propertyMap.set(AXPropertyName::SupportsKeyShortcutsValue, axObject.supportsKeyShortcutsValue());

and

+            propertyMap.set(AXPropertyName::KeyShortcuts, axObject.keyShortcutsValue().isolatedCopy());
+            break;

--- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h
+++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h
@@ -245,6 +245,7 @@ enum class AXPropertyName : uint16_t {

+    SupportsKeyShortcutsValue,

Drop Value from the name.

--- /dev/null
+++ b/LayoutTests/accessibility/aria-keyshortcuts.html

+<body id="body">

No need for id in <body>.

+    <div id="test1" role="button" tabindex="0">X</div>
+    <div id="test2" role="button" tabindex="0" aria-keyshortcuts="Shift+2">X</div>
+    <div id="test3" role="button" tabindex="0" aria-keyshortcuts="Shift+3 Option+4">X</div>

No need for tabindex in any of these elements.

+
+        output += expect("axItem2.stringAttributeValue('AXKeyShortcutsValue')", "'Shift+2'");
+        output += expect("axItem3.stringAttributeValue('AXKeyShortcutsValue')", "'Shift+3 Option+4'");
+

Should we test retrieving the shortcuts for test1 even when it is not supported to make sure we fail gracefully?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20221201/738ee458/attachment-0001.htm>
