AX: NSApplicationAccessibilityFocusedUIElement is sometimes an ignore…

…d object which breaks functionality for Voice Control in Mail https://bugs.webkit.org/show_bug.cgi?id=260191 rdar://113689647 Reviewed by Andres Gonzalez. Mail uses WebPage::setEditable(true) (indirectly through IPC) to make the entire webpage of a Mail compose window be editable. One side-effect of this is that the `body` element gains focus. In https://bugs.webkit.org/show_bug.cgi?id=257739, we made the assumption that any focused object is inherently unignored by way of having focus, and removed this code from AXObjectCache::focusedObjectForNode: if (focus->accessibilityIsIgnored()) return focus->parentObjectUnignored(); This bug reveals two pieces of information: 1. Our assumption (any focused object is inherently unignored) is wrong, as the AXGroup associated with the `body` is ignored despite it being focused 2. Even if this assumption were true, we would still experience the bug, as Voice Control very specifically expects the web area to be focused when editing a Mail message (arguably a Voice Control bug that should be addressed later) With this patch, we restore the above check to AXObjectCache::focusedObjectForNode, restoring behavior for Voice Control. We may want to re-evaluate this again in the future, as our original assumption still seems sound but clearly has some kinks that need working out. * LayoutTests/accessibility/editable-webpage-focused-ui-element-expected.txt: Added. * LayoutTests/accessibility/editable-webpage-focused-ui-element.html: Added. * LayoutTests/platform/glib/TestExpectations: Disable new test. * LayoutTests/platform/ios/TestExpectations: Enable new test. * Source/WebCore/accessibility/AXObjectCache.cpp: (WebCore::AXObjectCache::focusedObjectForNode): * Source/WebCore/testing/Internals.cpp: (WebCore::Internals::setSelectionFromNone): * Source/WebCore/testing/Internals.h: * Source/WebCore/testing/Internals.idl: Canonical link: https://commits.webkit.org/266917@main
WebKit · Aug 15, 2023 · 30959c8 · 30959c8
1 parent 9b47de8
commit 30959c8
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 0 deletions.
diff --git a/LayoutTests/accessibility/editable-webpage-focused-ui-element-expected.txt b/LayoutTests/accessibility/editable-webpage-focused-ui-element-expected.txt
@@ -0,0 +1,9 @@
+This test ensures that the focused UI element is not an ignored element.
+
+internals.setSelectionFromNone();
+PASS: accessibilityController.focusedElement.role.toLowerCase().includes('webarea') === true
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/accessibility/editable-webpage-focused-ui-element.html b/LayoutTests/accessibility/editable-webpage-focused-ui-element.html
@@ -0,0 +1,34 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<script src="../resources/accessibility-helper.js"></script>
+<script src="../resources/js-test.js"></script>
+</head>
+<body dir="auto" style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;">
+
+<script>
+var output = "This test ensures that the focused UI element is not an ignored element.\n\n";
+
+// Note that ideally this test would use <!-- webkit-test-runner [ editable=true ] --> to simulate
+// a fully-editable webpage, i.e. in the style of Mail message bodies. However, there is a bug in
+// that option that prevents it from being set in the web process (it returns early in WebPageProxy::setEditable
+// because there is no running process to send the message to at the time the test options are processed). For now,
+// use Internals::setSelectionFromNone to force the <body> to be the focused element, as happens in WebPage::setEditable(true).
+
+if (window.accessibilityController) {
+    window.jsTestIsAsync = true;
+
+    output += evalAndReturn("internals.setSelectionFromNone();");
+    setTimeout(async function() {
+        // Wait for focus to sync to the <body> and update the AX tree as a result of setSelectionFromNone().
+        await sleep(100);
+
+        output += expect("accessibilityController.focusedElement.role.toLowerCase().includes('webarea')", "true");
+        debug(output);
+        finishJSTest();
+    }, 0);
+}
+</script>
+</body>
+</html>
+
diff --git a/LayoutTests/platform/glib/TestExpectations b/LayoutTests/platform/glib/TestExpectations
@@ -507,6 +507,8 @@ accessibility/dynamically-changing-iframe-remains-accessible.html [ Skip ]
 accessibility/ignore-modals-without-any-content.html [ Skip ]
 accessibility/iframe-with-role.html [ Skip ]
 
+accessibility/editable-webpage-focused-ui-element.html [ Skip ]
+
 # Missing `AccessibilityUIElement::stringDescriptionOfAttributeValue` implementation.
 accessibility/visible-character-range-basic.html [ Skip ]
 accessibility/visible-character-range-height-changes.html [ Skip ]

diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations
@@ -2186,6 +2186,7 @@ accessibility/display-contents/element-roles.html [ Pass ]
 accessibility/display-contents/object-ordering.html [ Pass ]
 accessibility/display-contents/table.html [ Pass ]
 accessibility/dynamically-ignored-canvas.html [ Pass ]
+accessibility/editable-webpage-focused-ui-element.html [ Pass ]
 accessibility/element-haspopup.html [ Pass ]
 accessibility/empty-text-under-element-cached.html [ Pass ]
 accessibility/heading-level.html [ Pass ]

diff --git a/Source/WebCore/accessibility/AXObjectCache.cpp b/Source/WebCore/accessibility/AXObjectCache.cpp
@@ -504,6 +504,9 @@ AccessibilityObject* AXObjectCache::focusedObjectForNode(Node* focusedNode)
         if (auto* descendant = focus->activeDescendant())
             return descendant;
     }
+
+    if (focus->accessibilityIsIgnored())
+        return focus->parentObjectUnignored();
     return focus;
 }
 

diff --git a/Source/WebCore/testing/Internals.cpp b/Source/WebCore/testing/Internals.cpp
@@ -4453,6 +4453,11 @@ void Internals::setSelectionWithoutValidation(Ref<Node> baseNode, unsigned baseO
         VisiblePosition { makeDeprecatedLegacyPosition(extentNode.get(), extentOffset) });
 }
 
+void Internals::setSelectionFromNone()
+{
+    contextDocument()->frame()->selection().setSelectionFromNone();
+}
+
 ExceptionOr<bool> Internals::isPluginUnavailabilityIndicatorObscured(Element& element)
 {
     if (!is<HTMLPlugInElement>(element))

diff --git a/Source/WebCore/testing/Internals.h b/Source/WebCore/testing/Internals.h
@@ -763,6 +763,7 @@ class Internals final : public RefCounted<Internals>, private ContextDestruction
     ExceptionOr<Ref<DOMRect>> selectionBounds();
     ExceptionOr<RefPtr<StaticRange>> selectedRange();
     void setSelectionWithoutValidation(Ref<Node> baseNode, unsigned baseOffset, RefPtr<Node> extentNode, unsigned extentOffset);
+    void setSelectionFromNone();
 
     ExceptionOr<bool> isPluginUnavailabilityIndicatorObscured(Element&);
     ExceptionOr<String> unavailablePluginReplacementText(Element&);

diff --git a/Source/WebCore/testing/Internals.idl b/Source/WebCore/testing/Internals.idl
@@ -844,6 +844,7 @@ typedef (FetchRequest or FetchResponse) FetchObject;
     DOMRect selectionBounds();
     StaticRange? selectedRange();
     undefined setSelectionWithoutValidation(Node baseNode, unsigned long baseOffset, Node? extentNode, unsigned long extentOffset);
+    undefined setSelectionFromNone();
 
     [Conditional=MEDIA_SOURCE] undefined initializeMockMediaSource();
     [Conditional=MEDIA_SOURCE] Promise<sequence<DOMString>> bufferedSamplesForTrackId(SourceBuffer buffer, [AtomString] DOMString trackId);