AX: An unnecessary group is created for every block-flow box with no …

…other useful AX semantics https://bugs.webkit.org/show_bug.cgi?id=243373 Reviewed by Chris Fleizach. At the end of AccessibilityRenderObject::determineAccessibilityRole, we have this logic: if (m_renderer->isRenderBlockFlow()) return m_renderer->isAnonymousBlock() ? AccessibilityRole::TextGroup : AccessibilityRole::Group; This causes us to create a group for every block-flow box that doesn't otherwise have any useful AX semantics. This is problematic because: 1. It makes the AX tree diverge from the DOM in a way that makes inserting node-only children (e.g. those with display:contents) at the right index in AccessibilityRenderObject::addNodeOnlyChildren hard or impossible 2. Causes wasted work for WebKit by having to created isolated objects for these unnecessary groups 3. Causes wasted work for AX clients who have to filter through these semantics-less groups This patch removes this fallback (for Cocoa platforms). Because these groups are so pervasive throughout our tests, this required a lot of test changes, mostly removing now-unnecessary nested calls to `childAtIndex()` and removing the groups from accessibility tree text dumps. This change is required to fix: https://bugs.webkit.org/show_bug.cgi?id=242779 AX: display:contents elements are inserted in the wrong position when they have inline renderer siblings Because the reason we insert node-only objects in the wrong position is due to the existence of the anonymous block boxes created implicitly by inline renderers. The groups created by these boxes cause a restructuring of the AX tree that is difficult to work around, as the inline renderer AX objects become children of the anonymous block box AX object and are therefore no longer siblings in the AX tree to their DOM tree siblings. rdar://problem/97840402 * LayoutTests/accessibility/aria-disabled.html: * LayoutTests/accessibility/aria-hidden-false-works-in-subtrees.html: * LayoutTests/accessibility/aria-used-on-image-maps.html: * LayoutTests/accessibility/deleting-iframe-destroys-axcache.html: * LayoutTests/accessibility/double-nested-inline-element-missing-from-tree-expected.txt: * LayoutTests/accessibility/dynamically-changing-iframe-remains-accessible-expected.txt: * LayoutTests/accessibility/dynamically-changing-iframe-remains-accessible.html: * LayoutTests/accessibility/hidden-th-still-column-header-expected.txt: * LayoutTests/accessibility/hidden-th-still-column-header.html: * LayoutTests/accessibility/iframe-bastardization-expected.txt: * LayoutTests/accessibility/iframe-bastardization.html: * LayoutTests/accessibility/ignore-modals-without-any-content.html: * LayoutTests/accessibility/ignore-spacer-elements.html: * LayoutTests/accessibility/ignored-aria-role-description-expected.txt: * LayoutTests/accessibility/img-aria-button-alt-tag.html: * LayoutTests/accessibility/img-fallsback-to-title.html: * LayoutTests/accessibility/mac/aria-tree-item-children-expected.txt: * LayoutTests/accessibility/mac/aria-tree-item-children.html: * LayoutTests/accessibility/mac/canvas.html: * LayoutTests/accessibility/mac/element-focus-expected.txt: * LayoutTests/accessibility/mac/element-focus.html: * LayoutTests/accessibility/mac/element-level-expected.txt: * LayoutTests/accessibility/mac/element-level.html: * LayoutTests/accessibility/mac/figure-element-expected.txt: * LayoutTests/accessibility/mac/figure-element.html: * LayoutTests/accessibility/mac/html-section-elements-expected.txt: * LayoutTests/accessibility/mac/html-section-elements.html: * LayoutTests/accessibility/mac/iframe-aria-hidden-expected.txt: * LayoutTests/accessibility/mac/iframe-aria-hidden.html: * LayoutTests/accessibility/mac/iframe-pdf-expected.txt: * LayoutTests/accessibility/mac/iframe-pdf.html: * LayoutTests/accessibility/mac/label-element-with-hidden-control.html: * LayoutTests/accessibility/mac/math-alttext.html: * LayoutTests/accessibility/mac/nested-inline-elements-children.html: * LayoutTests/accessibility/mac/pseudo-element-text-markers-expected.txt: * LayoutTests/accessibility/mac/pseudo-element-text-markers.html: * LayoutTests/accessibility/mac/slider-allows-title-ui-element.html: * LayoutTests/accessibility/mac/stale-textmarker-crash.html: * LayoutTests/accessibility/mac/supports-focus-setting.html: * LayoutTests/accessibility/nochildren-elements.html: * LayoutTests/accessibility/table-cell-display-block.html: * LayoutTests/accessibility/deleting-iframe-destroys-axcache-expected.txt * LayoutTests/accessibility/mac/figure-element-expected.txt * LayoutTests/accessibility/mac/figure-element.html * LayoutTests/accessibility/mac/html-slider-indicator-expected.txt * LayoutTests/accessibility/mac/search-predicate-visited-links-expected.txt * LayoutTests/accessibility/mac/search-predicate-visited-links.html * LayoutTests/platform/ios/accessibility/aria-roles-unignored-expected.txt * LayoutTests/platform/mac-wk1/accessibility/dynamically-changing-iframe-remains-accessible-expected.txt * LayoutTests/platform/mac-wk1/accessibility/roles-exposed-expected.txt * LayoutTests/platform/mac/TestExpectations * LayoutTests/platform/mac/accessibility/deleting-iframe-destroys-axcache-expected.txt -> LayoutTests/platform/mac-wk1/accessibility/deleting-iframe-destroys-axcache-expected.txt * LayoutTests/platform/mac-wk2/accessibility/deleting-iframe-destroys-axcache-expected.txt: * LayoutTests/platform/mac-wk2/accessibility/roles-exposed-expected.txt: * LayoutTests/platform/mac/accessibility/aria-hidden-false-works-in-subtrees-expected.txt: * LayoutTests/platform/mac/accessibility/aria-menubar-menuitems-expected.txt: * LayoutTests/platform/mac/accessibility/generated-content-with-display-table-crash-expected.txt: * LayoutTests/platform/mac/accessibility/image-with-alt-and-map-expected.txt: * LayoutTests/platform/mac/accessibility/img-fallsback-to-title-expected.txt: * LayoutTests/platform/mac/accessibility/lists-expected.txt: Remove now-unnecessary nested calls to `childAtIndex()` and remove groups from accessibility tree text dumps. * LayoutTests/accessibility/aria-roles-unignored-expected.txt: Added. * LayoutTests/accessibility/aria-roles-unignored.html: Added. We originally started exposing every block-flow box as a group here: 1dffe62 (AX: GTK: ARIA role is not respected on <p> <label> <div> and <form>) I've added this test to ensure we don't regress this behavior with this patch. * LayoutTests/platform/win/TestExpectations: * LayoutTests/platform/glib/TestExpectations: Disable new test aria-roles-unignored.html. * LayoutTests/platform/ios/TestExpectations Enable new test aria-roles-unignored.html. * Source/WebCore/accessibility/AccessibilityRenderObject.cpp: (WebCore::AccessibilityRenderObject::determineAccessibilityRole): Stop giving a role to block-box flows with no other useful AX semantics on Cocoa platforms. This change should be made for other platforms, but doing so causes lots of failing tests that need to be worked through. Canonical link: https://commits.webkit.org/253038@main
WebKit · Aug 2, 2022 · 2c57ad2 · 2c57ad2
1 parent 6087b92
commit 2c57ad2
Show file tree

Hide file tree

Showing 66 changed files with 271 additions and 202 deletions.
diff --git a/LayoutTests/accessibility/aria-disabled.html b/LayoutTests/accessibility/aria-disabled.html
@@ -17,11 +17,20 @@
 
           var body = document.getElementById("body");
           body.focus();
-          var textField = accessibilityController.focusedElement.childAtIndex(0).childAtIndex(0);
+
+          var textField;
+          if (accessibilityController.platformName === "atspi" || accessibilityController.platformName === "win")
+              textField = accessibilityController.focusedElement.childAtIndex(0).childAtIndex(0);
+          else
+              textField = accessibilityController.focusedElement.childAtIndex(0);
+
           var succeeded = textField.isEnabled;
           shouldBe("succeeded", "false");
 
-          textField = accessibilityController.focusedElement.childAtIndex(0).childAtIndex(1);
+          if (accessibilityController.platformName === "atspi" || accessibilityController.platformName === "win")
+              textField = accessibilityController.focusedElement.childAtIndex(0).childAtIndex(1);
+          else
+              textField = accessibilityController.focusedElement.childAtIndex(1);
           succeeded = textField.isEnabled;
           shouldBe("succeeded", "true");
     }

diff --git a/LayoutTests/accessibility/aria-hidden-false-works-in-subtrees.html b/LayoutTests/accessibility/aria-hidden-false-works-in-subtrees.html
@@ -67,7 +67,7 @@
             // Test that aria-hidden=false does NOT expose iframe fallback text.
             let iframe = accessibilityController.accessibleElementById("iframe").childAtIndex(0).childAtIndex(0);
 
-            debug("Non-rendered iframe content should not be visible when aria-hidden=true. The first child should be a group and NOT static text.");
+            debug("Non-rendered iframe content should not be visible when aria-hidden=true. The first child should NOT be static text.");
             let iframeChild = null;
             await waitFor(() => {
                 return iframeChild = iframe.childAtIndex(0);

diff --git a/LayoutTests/accessibility/aria-roles-unignored-expected.txt b/LayoutTests/accessibility/aria-roles-unignored-expected.txt
@@ -0,0 +1,31 @@
+This tests that ARIA roles are not ignored for 'p', 'label', 'form' and 'div' elements.
+
+AXRole: AXWebArea
+    AXRole: AXGroup
+        AXRole: AXStaticText
+        AXValue: Simple paragraph
+    AXRole: AXTable
+        AXRole: AXGroup
+    AXRole: AXStaticText
+    AXValue: A label
+        AXRole: AXStaticText
+        AXValue: A label
+    AXRole: AXHeading
+        AXRole: AXStaticText
+        AXValue: Who said label? It's a heading!
+    AXRole: AXGroup
+        AXRole: AXStaticText
+        AXValue: A form with a button
+        AXRole: AXButton
+    AXRole: AXButton
+    AXRole: AXGroup
+        AXRole: AXStaticText
+        AXValue: Just some text inside a div
+    AXRole: AXTextField
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+
+
diff --git a/LayoutTests/accessibility/aria-roles-unignored.html b/LayoutTests/accessibility/aria-roles-unignored.html
@@ -0,0 +1,54 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/accessibility-helper.js"></script>
+<script src="../resources/js-test.js"></script>
+</head>
+<body>
+
+<div id="content">
+    <p>Simple paragraph</p>
+    <p role="grid">A paragraph pretending to be a table</p>
+
+    <label>A label</label>
+    <label role="heading">Who said label? It's a heading!</label>
+
+    <form>A form with a button <button name="button" value="Button">Click me!</button></form>
+    <form role="button">Just a button <button name="button" value="Button">Click me!</button></form>
+
+    <div>Just some text inside a div</div>
+    <div role="textbox">This div is contains a textbox (an entry)</div>
+</div>
+
+<script>
+
+var testOutput = "This tests that ARIA roles are not ignored for 'p', 'label', 'form' and 'div' elements.\n\n";
+
+function dumpTree(axElement, indent = 0) {
+    if (!axElement)
+        return "";
+
+    let indentStr = "";
+    for (let i = 0; i < indent; i++)
+        indentStr += "    ";
+
+    let str = `${indentStr}${axElement.role}\n`
+    if (axElement.role && axElement.role.includes("StaticText"))
+        str += `${indentStr}${accessibilityController.platformName === "ios" ? axElement.description : axElement.stringValue}\n`
+
+    for (let i = 0; i < axElement.childrenCount; ++i)
+        str += dumpTree(axElement.childAtIndex(i), indent + 1);
+
+    return str;
+}
+
+if (window.accessibilityController) {
+    testOutput += dumpTree(accessibilityController.rootElement.childAtIndex(0));
+
+    document.getElementById("content").style.visibility = "hidden";
+    debug(testOutput);
+}
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/accessibility/aria-used-on-image-maps.html b/LayoutTests/accessibility/aria-used-on-image-maps.html
@@ -20,7 +20,9 @@
 
     if (window.accessibilityController) {
 
-          var group = accessibilityController.rootElement.childAtIndex(0).childAtIndex(0);
+          var group = accessibilityController.rootElement.childAtIndex(0);
+          if (accessibilityController.platformName === "atspi" || accessibilityController.platformName === "win")
+              group = accessibilityController.rootElement.childAtIndex(0).childAtIndex(0);
           shouldBe("group.childAtIndex(0).role", "'AXRole: AXButton'");
           shouldBe("group.childAtIndex(1).role", "'AXRole: AXButton'");
     }

diff --git a/LayoutTests/accessibility/deleting-iframe-destroys-axcache-expected.txt b/LayoutTests/accessibility/deleting-iframe-destroys-axcache-expected.txt
diff --git a/LayoutTests/accessibility/deleting-iframe-destroys-axcache.html b/LayoutTests/accessibility/deleting-iframe-destroys-axcache.html
@@ -18,11 +18,7 @@
             window.iframe = body.childAtIndex(1).childAtIndex(0);
             window.after = body.childAtIndex(2);
 
-            window.frameBody = window.iframe.childAtIndex(0);
-            window.frameBodyRole = window.frameBody.role;
-            window.frameGroup = window.frameBody.childAtIndex(0);
-            window.frameGroupRole = window.frameGroup.role;
-            window.frameButton = window.frameGroup.childAtIndex(0);
+            window.frameButton = window.iframe.childAtIndex(0);
             window.frameButtonRole = window.frameButton.role;
 
             document.getElementById("tree").innerText += "\nBefore:\n";
@@ -43,8 +39,6 @@
             // Make sure that the accessibility objects from the iframe's nodes
             // are now invalid by checking that their role is changed - this
             // is because they've been deleted.
-            shouldBeFalse("frameBodyRole == frameBody.role");
-            shouldBeFalse("frameGroupRole == frameGroup.role");
             shouldBeFalse("frameButtonRole == frameButton.role");
 
             // Make sure that the other nodes are unchanged.

diff --git a/LayoutTests/accessibility/double-nested-inline-element-missing-from-tree-expected.txt b/LayoutTests/accessibility/double-nested-inline-element-missing-from-tree-expected.txt
@@ -7,10 +7,9 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 
 AXRole: AXWebArea AXValue:
+  AXRole: AXStaticText AXValue: test1
   AXRole: AXGroup AXValue:
-    AXRole: AXStaticText AXValue: test1
-    AXRole: AXGroup AXValue:
-      AXRole: AXStaticText AXValue: test2
+    AXRole: AXStaticText AXValue: test2
   AXRole: AXGroup AXValue:
 PASS successfullyParsed is true
 

diff --git a/LayoutTests/accessibility/dynamically-changing-iframe-remains-accessible-expected.txt b/LayoutTests/accessibility/dynamically-changing-iframe-remains-accessible-expected.txt
@@ -9,21 +9,17 @@ AXRole: AXScrollArea
 
 AXRole: AXWebArea
 
-AXRole: AXGroup
-
 AXRole: AXStaticText
 AXValue: Purple pineapple
 
 AXRole: AXScrollArea
 
 AXRole: AXWebArea
 
-AXRole: AXGroup
-
 AXRole: AXStaticText
 AXValue: Purple pineapple
 
-Traversed 9 elements.
+Traversed 7 elements.
 PASS successfullyParsed is true
 
 TEST COMPLETE

diff --git a/LayoutTests/accessibility/dynamically-changing-iframe-remains-accessible.html b/LayoutTests/accessibility/dynamically-changing-iframe-remains-accessible.html
@@ -4,7 +4,7 @@
 <script src="../resources/js-test.js"></script>
 <script src="../resources/accessibility-helper.js"></script>
 </head>
-<body id="body">
+<body id="body" role="group">
 
 <input type="text" id="text-input-outside-iframe">
 <iframe id="iframe-contentwindow" width=500 height=500></iframe>

diff --git a/LayoutTests/accessibility/hidden-th-still-column-header-expected.txt b/LayoutTests/accessibility/hidden-th-still-column-header-expected.txt
@@ -6,7 +6,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 
 PASS colHeaders.length is 2
-PASS colHeaders[0].childAtIndex(0).childAtIndex(0).stringValue is 'AXValue: header 1'
+PASS colHeaders[0].childAtIndex(0).stringValue is 'AXValue: header 1'
 PASS colHeaders[1].childAtIndex(0).stringValue is 'AXValue: header 3'
 PASS successfullyParsed is true
 

diff --git a/LayoutTests/accessibility/hidden-th-still-column-header.html b/LayoutTests/accessibility/hidden-th-still-column-header.html
@@ -22,7 +22,7 @@
         var table = accessibilityController.accessibleElementById("table");
         var colHeaders = table.columnHeaders();
         shouldBe("colHeaders.length", "2");
-        shouldBe("colHeaders[0].childAtIndex(0).childAtIndex(0).stringValue", "'AXValue: header 1'");
+        shouldBe("colHeaders[0].childAtIndex(0).stringValue", "'AXValue: header 1'");
         shouldBe("colHeaders[1].childAtIndex(0).stringValue", "'AXValue: header 3'");
     }
 </script>

diff --git a/LayoutTests/accessibility/iframe-bastardization-expected.txt b/LayoutTests/accessibility/iframe-bastardization-expected.txt
@@ -5,8 +5,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 
 PASS parentIframeWebArea.isEqual(iframeScrollArea) is true
-PASS parentIframeScrollArea.isEqual(group1) is true
-PASS parentGroup1.isEqual(webArea) is true
+PASS parentIframeScrollArea.isEqual(webArea) is true
 PASS successfullyParsed is true
 
 TEST COMPLETE

diff --git a/LayoutTests/accessibility/iframe-bastardization.html b/LayoutTests/accessibility/iframe-bastardization.html
@@ -1,5 +1,6 @@
 <head>
 <script src="../resources/js-test-pre.js"></script>
+<script src="../resources/accessibility-helper.js"></script>
 </head>
 <body id="body">
 
@@ -18,17 +19,14 @@
         body.focus();
         var webArea = accessibilityController.focusedElement;
 
-        var group1 = webArea.childAtIndex(0);
-        var iframeScrollArea = group1.childAtIndex(0);
+        var iframeScrollArea = webArea.childAtIndex(0);
         var iframeWebArea = iframeScrollArea.childAtIndex(0);
 
         var parentIframeWebArea = iframeWebArea.parentElement();
         var parentIframeScrollArea = parentIframeWebArea.parentElement();
-        var parentGroup1 = parentIframeScrollArea.parentElement();
 
         shouldBe("parentIframeWebArea.isEqual(iframeScrollArea)", "true");
-        shouldBe("parentIframeScrollArea.isEqual(group1)", "true");
-        shouldBe("parentGroup1.isEqual(webArea)", "true");
+        shouldBe("parentIframeScrollArea.isEqual(webArea)", "true");
     }
 
 </script>

diff --git a/LayoutTests/accessibility/ignore-modals-without-any-content.html b/LayoutTests/accessibility/ignore-modals-without-any-content.html
@@ -57,10 +57,9 @@
             let text = document.createTextNode("Foo text, inside modal");
             document.getElementById("modal").appendChild(text);
 
+            // Wait for text node to be added as an AX child.
             await waitFor(() => {
-                let innerModalGroup = accessibilityController.accessibleElementById("modal").childAtIndex(0);
-                // Wait for text node to be added as an AX child.
-                return innerModalGroup && innerModalGroup.childAtIndex(0);
+                return accessibilityController.accessibleElementById("modal").childAtIndex(0);
             });
 
             let containerElement = accessibilityController.accessibleElementById("container");

diff --git a/LayoutTests/accessibility/ignore-spacer-elements.html b/LayoutTests/accessibility/ignore-spacer-elements.html
@@ -22,7 +22,9 @@
 
             var body = document.getElementById("body");
             body.focus();
-            var container = accessibilityController.focusedElement.childAtIndex(0);
+            var container = accessibilityController.focusedElement;
+            if (accessibilityController.platformName === "atspi" || accessibilityController.platformName === "win")
+                container = container.childAtIndex(0);
 
             // The Gtk and EFL ports ATs expect the bold inline text to not have accessible objects.
             var expectedCount = accessibilityController.platformName == "atspi" ? 2 : 4;

diff --git a/LayoutTests/accessibility/ignored-aria-role-description-expected.txt b/LayoutTests/accessibility/ignored-aria-role-description-expected.txt
@@ -4,7 +4,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 
 div role: AXRoleDescription: group
-span text role: AXRoleDescription: group
+span text role: AXRoleDescription: text
 
 PASS successfullyParsed is true
 

diff --git a/LayoutTests/accessibility/img-aria-button-alt-tag.html b/LayoutTests/accessibility/img-aria-button-alt-tag.html
@@ -21,7 +21,9 @@
             var result = document.getElementById("result");
 
             var body = document.getElementById("body").focus();
-            var imgUIElement = accessibilityController.focusedElement.childAtIndex(0).childAtIndex(0);
+            var imgUIElement = accessibilityController.focusedElement.childAtIndex(0);
+            if (accessibilityController.platformName === "atspi" || accessibilityController.platformName === "win")
+                imgUIElement = accessibilityController.focusedElement.childAtIndex(0).childAtIndex(0);
             shouldBeEqualToString("platformValueForW3CName(imgUIElement)", "alternate");
         }
     </script>

diff --git a/LayoutTests/accessibility/img-fallsback-to-title.html b/LayoutTests/accessibility/img-fallsback-to-title.html
@@ -26,31 +26,37 @@
         document.getElementById("images").focus();
         var imagesGroup = accessibilityController.focusedElement;
 
+        const isAtspi = accessibilityController.platformName === "atspi";
+
         // First image should have a description of "test1" because there is no alt tag (it should use the title).
         // The title should NOT be in the help text.
-        var image1 = imagesGroup.childAtIndex(0).childAtIndex(0);
+        var image1 = isAtspi ? imagesGroup.childAtIndex(0).childAtIndex(0) : imagesGroup.childAtIndex(0);
         debug("Image1:");
         debug(platformTextAlternatives(image1));
 
         // Second image should use the description from the alt tag instead of the title.
         // The help text should reflect what's in the title.
-        var image2 = imagesGroup.childAtIndex(0).childAtIndex(1);
+        var image2 = isAtspi ? imagesGroup.childAtIndex(0).childAtIndex(1) : imagesGroup.childAtIndex(1);
         debug("Image2:");
         debug(platformTextAlternatives(image2));
 
         // Now do the same checks for ARIA type images.
-        var image3 = imagesGroup.childAtIndex(1);
+        var image3 = isAtspi ? imagesGroup.childAtIndex(1) : imagesGroup.childAtIndex(2);
         debug("Image3:");
         debug(platformTextAlternatives(image3));
 
         // Now do the same checks for ARIA type images.
-        var image4 = imagesGroup.childAtIndex(2);
+        var image4 = isAtspi ? imagesGroup.childAtIndex(2) : imagesGroup.childAtIndex(3);
         debug("Image4:");
         debug(platformTextAlternatives(image4));
 
-        // Verify that the first image (with an empty alt tag) is ignored
-        // by checking the children count of the group containing the native images == 2.
-        shouldBe("imagesGroup.childAtIndex(0).childrenCount", "2");
+        if (isAtspi)
+            shouldBe("imagesGroup.childAtIndex(0).childrenCount", "2");
+        else {
+            // Verify that the first image (with an empty alt tag) is ignored by checking the children count of
+            // the group is 4 (not 5).
+            shouldBe("imagesGroup.childrenCount", "4");
+        }
     }
 
 </script>

diff --git a/LayoutTests/accessibility/mac/aria-tree-item-children-expected.txt b/LayoutTests/accessibility/mac/aria-tree-item-children-expected.txt
@@ -6,7 +6,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 
 PASS tree.role is 'AXRole: AXOutline'
-PASS child1 == child2 is true
+PASS child1.stringValue == child2.stringValue && child1.role == child2.role is true
 PASS successfullyParsed is true
 
 TEST COMPLETE