Changeset 288117 in webkit

Timestamp:

Jan 18, 2022 8:45:07 AM (6 months ago)

Author:

commit-queue@webkit.org

Message:

AX: Improve support for aria-owns in aria trees
​https://bugs.webkit.org/show_bug.cgi?id=233383

Patch by Sepand Parhami <​sparhami@apple.com> on 2022-01-18
Reviewed by Chris Fleizach.

Source/WebCore:

Better supports aria-owns in trees by following children specified through ownership similar
to how DOM children are handled. Previously, you could use aria-owns for only one level of
ownership. This change also fixes tree items being included twice (once as a DOM child and
another time via aria-owns) and supports using both DOM children and aria-owns on a single
element. This change only affects trees via a change to ariaTreeRows and does not change
aria-owns behavior in general.

Tests: accessibility/mac/treeitem-row-delegation.html

accessibility/mac/treeitem-row-delegation-poorly-formed.html

• accessibility/AccessibilityObject.cpp:

(WebCore::AccessibilityObject::ariaTreeRows):

• accessibility/AccessibilityObject.h:

LayoutTests:

Adds test cases for additional specified behavior of aria-owns in trees as well as tests to
make sure incorrect aria-owns usage does not cause crashes.

• accessibility/mac/treeitem-row-delegation-expected.txt:
• accessibility/mac/treeitem-row-delegation-poorly-formed-expected.txt: Added.
• accessibility/mac/treeitem-row-delegation-poorly-formed.html: Added.
• accessibility/mac/treeitem-row-delegation.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/accessibility/mac/treeitem-row-delegation-expected.txt (modified) (1 diff)
• LayoutTests/accessibility/mac/treeitem-row-delegation-poorly-formed-expected.txt (added)
• LayoutTests/accessibility/mac/treeitem-row-delegation-poorly-formed.html (added)
• LayoutTests/accessibility/mac/treeitem-row-delegation.html (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/accessibility/AccessibilityObject.cpp (modified) (1 diff)
• Source/WebCore/accessibility/AccessibilityObject.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2022-01-18  Sepand Parhami  <sparhami@apple.com>
+
+        AX: Improve support for aria-owns in aria trees
+        https://bugs.webkit.org/show_bug.cgi?id=233383
+
+        Reviewed by Chris Fleizach.
+
+        Adds test cases for additional specified behavior of aria-owns in trees as well as tests to
+        make sure incorrect aria-owns usage does not cause crashes.
+
+        * accessibility/mac/treeitem-row-delegation-expected.txt:
+        * accessibility/mac/treeitem-row-delegation-poorly-formed-expected.txt: Added.
+        * accessibility/mac/treeitem-row-delegation-poorly-formed.html: Added.
+        * accessibility/mac/treeitem-row-delegation.html:
+
 2022-01-18  Arcady Goldmints-Orlov  <agoldmints@igalia.com>
 

trunk/LayoutTests/accessibility/mac/treeitem-row-delegation-expected.txt

@@ -1 @@
-This tests that a treeitem was a group that uses aria-owns will report its disclosed rows correctly.
+This tests that a treeitem with a group that uses aria-owns will report its disclosed rows correctly.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS treeitem2.isEqual(accessibilityController.accessibleElementById('group1-item1')) is true
+PASS tree.childrenCount is 7
+PASS treeitem1.isEqual(accessibilityController.accessibleElementById('item-1')) is true
+PASS treeitem1_1.isEqual(accessibilityController.accessibleElementById('group1-item1')) is true
+PASS treeitem1_2.isEqual(accessibilityController.accessibleElementById('group1-item2')) is true
+PASS treeitem1_3.isEqual(accessibilityController.accessibleElementById('group1-item3')) is true
+PASS treeitem1_2_1.isEqual(accessibilityController.accessibleElementById('group2-item1')) is true
+PASS treeitem1_2_2.isEqual(accessibilityController.accessibleElementById('group2-item2')) is true
+PASS treeitem2.isEqual(accessibilityController.accessibleElementById('item-2')) is true
 PASS successfullyParsed is true
 
 TEST COMPLETE
+Jaguar
 Jungle Animals
-Lions
+Birds
+Reptiles
+Cats
+Lion
+Domestic Animals

trunk/LayoutTests/accessibility/mac/treeitem-row-delegation.html

@@ -9 +9 @@
 
 <div id="tree" role="tree">
+    <div id="group2-item2" role="treeitem" aria-level="3" aria-setsize="2" aria-posinset="2" aria-current="page">Jaguar</div>
     <div id="item-1" role="treeitem" aria-level="1" aria-expanded="true" tabindex="0">Jungle Animals
-        <div role="group" aria-owns="group1-item1 group1-item2 group1-item3"></div>
+        <div role="group" aria-owns="group1-item2 group1-item3 non-existent-item1">
+          <div id="group1-item3" role="treeitem" aria-level="2" aria-setsize="3" aria-posinset="3" aria-current="page">Birds</div>
+          <div id="group1-item1" role="treeitem" aria-level="2" aria-setsize="3" aria-posinset="1" aria-current="page">Reptiles</div>
+        </div>
     </div>
-    <div id="group1-item1" role="treeitem" aria-level="2" aria-setsize="3" aria-posinset="1" aria-current="page">Lions</div>
+    <div id="group1-item2" role="treeitem" aria-level="2" aria-expanded="true" aria-setsize="1" aria-posinset="2" aria-current="page">Cats
+        <div role="group" aria-owns="group2-item1 group2-item2"></div>
+    </div>
+    <div id="group2-item1" role="treeitem" aria-level="3" aria-posinset="1" aria-setsize="2" aria-current="page">Lion</div>
+    <div id="item-2" role="treeitem" aria-level="1" aria-expanded="false" tabindex="-1">Domestic Animals</div>
 </div>
 
 <script>
-    description("This tests that a treeitem was a group that uses aria-owns will report its disclosed rows correctly.");
+    description("This tests that a treeitem with a group that uses aria-owns will report its disclosed rows correctly.");
 
     if (window.accessibilityController) {
-       var treeitem = accessibilityController.accessibleElementById("item-1");
-       var treeitem2 = treeitem.disclosedRowAtIndex(0);
-       shouldBeTrue("treeitem2.isEqual(accessibilityController.accessibleElementById('group1-item1'))");
+       var tree = accessibilityController.accessibleElementById("tree");
+       var treeitem1 = tree.rowAtIndex(0);
+       var treeitem1_1 = treeitem1.disclosedRowAtIndex(0);
+       var treeitem1_2 = treeitem1.disclosedRowAtIndex(1);
+       var treeitem1_3 = treeitem1.disclosedRowAtIndex(4);
+       var treeitem1_2_1 = treeitem1_2.disclosedRowAtIndex(0);
+       var treeitem1_2_2 = treeitem1_2.disclosedRowAtIndex(1);
+       var treeitem2 = tree.rowAtIndex(6);
+       shouldBe("tree.childrenCount", "7");
+       shouldBeTrue("treeitem1.isEqual(accessibilityController.accessibleElementById('item-1'))");
+       shouldBeTrue("treeitem1_1.isEqual(accessibilityController.accessibleElementById('group1-item1'))");
+       shouldBeTrue("treeitem1_2.isEqual(accessibilityController.accessibleElementById('group1-item2'))");
+       shouldBeTrue("treeitem1_3.isEqual(accessibilityController.accessibleElementById('group1-item3'))");
+       shouldBeTrue("treeitem1_2_1.isEqual(accessibilityController.accessibleElementById('group2-item1'))");
+       shouldBeTrue("treeitem1_2_2.isEqual(accessibilityController.accessibleElementById('group2-item2'))");
+       shouldBeTrue("treeitem2.isEqual(accessibilityController.accessibleElementById('item-2'))");
     }
 </script>

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2022-01-18  Sepand Parhami  <sparhami@apple.com>
+
+        AX: Improve support for aria-owns in aria trees
+        https://bugs.webkit.org/show_bug.cgi?id=233383
+
+        Reviewed by Chris Fleizach.
+
+        Better supports aria-owns in trees by following children specified through ownership similar
+        to how DOM children are handled. Previously, you could use aria-owns for only one level of
+        ownership. This change also fixes tree items being included twice (once as a DOM child and
+        another time via aria-owns) and supports using both DOM children and aria-owns on a single
+        element. This change only affects trees via a change to ariaTreeRows and does not change
+        aria-owns behavior in general.
+
+        Tests: accessibility/mac/treeitem-row-delegation.html
+               accessibility/mac/treeitem-row-delegation-poorly-formed.html
+
+        * accessibility/AccessibilityObject.cpp:
+        (WebCore::AccessibilityObject::ariaTreeRows):
+        * accessibility/AccessibilityObject.h:
+
 2022-01-18  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/accessibility/AccessibilityObject.cpp

@@ -1894 +1894 @@
 }
 
-void AccessibilityObject::ariaTreeRows(AccessibilityChildrenVector& result)
-{
-    // If the element specifies its tree rows through aria-owns, return that first.
+void AccessibilityObject::ariaTreeRows(AccessibilityChildrenVector& rows, AccessibilityChildrenVector& ancestors)
+{
     AccessibilityChildrenVector ariaOwns;
     ariaOwnsElements(ariaOwns);
-    if (ariaOwns.size()) {
-        result.appendVector(ariaOwns);
-        return;
-    }
-    
+    
+    ancestors.append(this);
+
+    // The ordering of rows is first DOM children *not* in aria-owns, followed by all specified
+    // in aria-owns.
     for (const auto& child : children()) {
         // Add tree items as the rows.
-        if (child->roleValue() == AccessibilityRole::TreeItem)
-            result.append(child);
+        if (child->roleValue() == AccessibilityRole::TreeItem) {
+            // Child appears both as a direct child and aria-owns, we should use the ordering as
+            // described in aria-owns for this child.
+            if (ariaOwns.contains(child))
+                continue;
+            
+            // The result set may already contain the child through aria-owns. For example,
+            // a treeitem sitting under the tree root, which is owned elsewhere in the tree.
+            if (rows.contains(child))
+                continue;
+
+            rows.append(child);
+        }
 
         // Now see if this item also has rows hiding inside of it.
-        child->ariaTreeRows(result);
-    }
+        if (is<AccessibilityObject>(*child))
+            downcast<AccessibilityObject>(*child).ariaTreeRows(rows, ancestors);
+    }
+
+    // Now go through the aria-owns elements.
+    for (const auto& child : ariaOwns) {
+        // Avoid a circular reference via aria-owns by checking if our parent
+        // path includes this child. Currently, looking up the aria-owns parent
+        // path itself could be expensive, so we track it separately.
+        if (ancestors.contains(child))
+            continue;
+        
+        // Add tree items as the rows.
+        if (child->roleValue() == AccessibilityRole::TreeItem) {
+            // Hopefully a flow that does not occur often in practice, but if someone were to include
+            // the owned child ealier in the top level of the tree, then reference via aria-owns later,
+            // move it to the right place.
+            if (rows.contains(child))
+                rows.removeFirst(child);
+            
+            rows.append(child);
+        }
+
+        // Now see if this item also has rows hiding inside of it.
+        if (is<AccessibilityObject>(*child))
+            downcast<AccessibilityObject>(*child).ariaTreeRows(rows, ancestors);
+    }
+    
+    ancestors.removeLast();
+}
+
+void AccessibilityObject::ariaTreeRows(AccessibilityChildrenVector& rows    )
+{
+    AccessibilityChildrenVector ancestors;
+    ariaTreeRows(rows, ancestors);
 }
     

trunk/Source/WebCore/accessibility/AccessibilityObject.h

@@ -827 +827 @@
     std::optional<SimpleRange> visibleCharacterRange() const override;
 
+    void ariaTreeRows(AccessibilityChildrenVector& rows, AccessibilityChildrenVector& ancestors);
+    
 #if PLATFORM(COCOA) && ENABLE(MODEL_ELEMENT)
     Vector<RetainPtr<id>> modelElementChildren() override;