- Revision
- 290630
- Author
- tyle...@apple.com
- Date
- 2022-02-28 23:46:09 -0800 (Mon, 28 Feb 2022)
Log Message
AX: WebKit should ignore empty modals rather than trapping focus inside them
https://bugs.webkit.org/show_bug.cgi?id=237163
Reviewed by Chris Fleizach and Andres Gonzalez.
Source/WebCore:
Given this markup:
<div role="dialog" aria-modal="true">
<div aria-hidden="true">
<button>Close modal (inside modal)</button>
</div>
</div>
There is no accessible content inside this modal, but WebKit traps user focus inside,
making the rest of the page completely inaccessible.
With this patch we ignore modals that don't have accessible content.
We do this by walking the DOM to find any non-AX-ignored objects.
Because determining whether or not an element is ignored is dependent
on modals present on the page, this patch moves the call to `ignoredFromModalPresence`
out of `AccessibilityObject::defaultObjectInclusion`, as this function is
downstream of `computeAccessibilityIsIgnored`, and we need to call
that on objects inside modal candidates. Without this move, we would
recurse infinitely in `AXObjectCache::modalElementHasAccessibleContent`.
We now check whether an object is ignored due to modal presence in
`AccessibilityObject::accessibilityIsIgnored()`, so that function should
be used as the final say in determining whether an object is ignored.
Test: accessibility/ignore-modals-without-any-content.html
* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::modalElementHasAccessibleContent):
(WebCore::AXObjectCache::currentModalNode):
* accessibility/AXObjectCache.h:
* accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::defaultObjectInclusion const):
(WebCore::AccessibilityObject::accessibilityIsIgnored const):
* accessibility/AccessibilityObject.h:
(WebCore::AccessibilityObject::computeAccessibilityIsIgnored const):
LayoutTests:
* accessibility/ignore-modals-without-any-content-expected.txt: Added.
* accessibility/ignore-modals-without-any-content.html: Added.
* platform/glib/TestExpectations: Skip new test.
* platform/ios/TestExpectations: Enable new test.
* platform/ios/accessibility/ignore-modals-without-any-content-expected.txt: Added.
* platform/win/TestExpectations: Skip new test.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (290629 => 290630)
--- trunk/LayoutTests/ChangeLog 2022-03-01 06:12:27 UTC (rev 290629)
+++ trunk/LayoutTests/ChangeLog 2022-03-01 07:46:09 UTC (rev 290630)
@@ -1,3 +1,17 @@
+2022-02-28 Tyler Wilcock <tyle...@apple.com>
+
+ AX: WebKit should ignore empty modals rather than trapping focus inside them
+ https://bugs.webkit.org/show_bug.cgi?id=237163
+
+ Reviewed by Chris Fleizach and Andres Gonzalez.
+
+ * accessibility/ignore-modals-without-any-content-expected.txt: Added.
+ * accessibility/ignore-modals-without-any-content.html: Added.
+ * platform/glib/TestExpectations: Skip new test.
+ * platform/ios/TestExpectations: Enable new test.
+ * platform/ios/accessibility/ignore-modals-without-any-content-expected.txt: Added.
+ * platform/win/TestExpectations: Skip new test.
+
2022-02-28 Simon Fraser <simon.fra...@apple.com>
Compositing/paint invalidation with transforms
Added: trunk/LayoutTests/accessibility/ignore-modals-without-any-content-expected.txt (0 => 290630)
--- trunk/LayoutTests/accessibility/ignore-modals-without-any-content-expected.txt (rev 0)
+++ trunk/LayoutTests/accessibility/ignore-modals-without-any-content-expected.txt 2022-03-01 07:46:09 UTC (rev 290630)
@@ -0,0 +1,22 @@
+This test ensures that we ignore modals that don't have any accessible content.
+
+Beginning search from #container element.
+
+AXRole: AXStaticText
+AXValue: Foo text, before modal
+
+AXRole: AXStaticText
+AXValue: Foo text, after modal
+
+Appending a text node to modal to make it accessible.
+PASS: #container AX element is no longer accessible after adding accessible content to modal.
+
+Beginning search from #modal element.
+
+AXRole: AXStaticText
+AXValue: Foo text, inside modal
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/accessibility/ignore-modals-without-any-content.html (0 => 290630)
--- trunk/LayoutTests/accessibility/ignore-modals-without-any-content.html (rev 0)
+++ trunk/LayoutTests/accessibility/ignore-modals-without-any-content.html 2022-03-01 07:46:09 UTC (rev 290630)
@@ -0,0 +1,81 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+
+<div id="container" role="group">
+ <div>Foo text, before modal</div>
+
+ <div id="modal" role="dialog" aria-modal="true">
+ <!-- Wrap the aria-hidden content in arbitrary div layers to ensure we don't consider the arbitrary divs accessible content. -->
+ <div>
+ <div>
+ <div aria-hidden="true">
+ <button>Hidden button (inside modal)</button>
+ </div>
+ <div style="display: none;">
+ Text inside display: none
+ </div>
+ </div>
+ </div>
+ </div>
+
+ <div>Foo text, after modal</div>
+</div>
+
+<script>
+ var testOutput = "This test ensures that we ignore modals that don't have any accessible content.\n";
+
+ async function findTextInsideId(id) {
+ const containerElement = accessibilityController.accessibleElementById(id);
+ testOutput += `\nBeginning search from #${id} element.\n`;
+
+ let searchResult = null;
+ while (true) {
+ searchResult = containerElement.uiElementForSearchPredicate(searchResult, true, "AXAnyTypeSearchKey", "", false);
+ if (!searchResult)
+ break;
+
+ if (searchResult.role.includes("StaticText")) {
+ testOutput += `\n${searchResult.role}`;
+ const textContent = accessibilityController.platformName === "ios" ? searchResult.description : searchResult.stringValue;
+ testOutput += `\n${textContent}\n`;
+ }
+ }
+ }
+
+ if (window.accessibilityController) {
+ window.jsTestIsAsync = true;
+
+ setTimeout(async function() {
+ await findTextInsideId("container");
+
+ testOutput += "\nAppending a text node to modal to make it accessible.";
+ let text = document.createTextNode("Foo text, inside modal");
+ document.getElementById("modal").appendChild(text);
+
+ 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);
+ });
+
+ let containerElement = accessibilityController.accessibleElementById("container");
+ if (containerElement)
+ testOutput += "\nFAIL: #container AX element was still accessible after adding accessible content to modal.\n";
+ else
+ testOutput += "\nPASS: #container AX element is no longer accessible after adding accessible content to modal.\n";
+
+ await findTextInsideId("modal");
+
+ document.getElementById("container").style.visibility = "hidden";
+ debug(testOutput);
+ finishJSTest();
+ }, 0);
+ }
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/glib/TestExpectations (290629 => 290630)
--- trunk/LayoutTests/platform/glib/TestExpectations 2022-03-01 06:12:27 UTC (rev 290629)
+++ trunk/LayoutTests/platform/glib/TestExpectations 2022-03-01 07:46:09 UTC (rev 290630)
@@ -342,6 +342,7 @@
# Missing AccessibilityUIElement::uiElementForSearchPredicate implementation.
accessibility/table-search-traversal.html [ Skip ]
accessibility/dynamically-changing-iframe-remains-accessible.html [ Skip ]
+accessibility/ignore-modals-without-any-content.html [ Skip ]
webkit.org/b/212805 accessibility/svg-text.html [ Failure ]
Modified: trunk/LayoutTests/platform/ios/TestExpectations (290629 => 290630)
--- trunk/LayoutTests/platform/ios/TestExpectations 2022-03-01 06:12:27 UTC (rev 290629)
+++ trunk/LayoutTests/platform/ios/TestExpectations 2022-03-01 07:46:09 UTC (rev 290630)
@@ -2101,6 +2101,7 @@
accessibility/table-search-traversal.html [ Pass ]
accessibility/dynamically-changing-iframe-remains-accessible.html [ Pass ]
+accessibility/ignore-modals-without-any-content.html [ Pass ]
accessibility/selected-state-changed-notifications.html [ Pass ]
accessibility/element-line-rects-and-text.html [ Pass ]
Added: trunk/LayoutTests/platform/ios/accessibility/ignore-modals-without-any-content-expected.txt (0 => 290630)
--- trunk/LayoutTests/platform/ios/accessibility/ignore-modals-without-any-content-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/ios/accessibility/ignore-modals-without-any-content-expected.txt 2022-03-01 07:46:09 UTC (rev 290630)
@@ -0,0 +1,22 @@
+This test ensures that we ignore modals that don't have any accessible content.
+
+Beginning search from #container element.
+
+StaticText
+AXLabel: Foo text, before modal
+
+StaticText
+AXLabel: Foo text, after modal
+
+Appending a text node to modal to make it accessible.
+PASS: #container AX element is no longer accessible after adding accessible content to modal.
+
+Beginning search from #modal element.
+
+StaticText
+AXLabel: Foo text, inside modal
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Modified: trunk/LayoutTests/platform/win/TestExpectations (290629 => 290630)
--- trunk/LayoutTests/platform/win/TestExpectations 2022-03-01 06:12:27 UTC (rev 290629)
+++ trunk/LayoutTests/platform/win/TestExpectations 2022-03-01 07:46:09 UTC (rev 290630)
@@ -476,6 +476,7 @@
# Missing AccessibilityUIElement::uiElementForSearchPredicate implementation.
accessibility/table-search-traversal.html [ Skip ]
accessibility/dynamically-changing-iframe-remains-accessible.html [ Skip ]
+accessibility/ignore-modals-without-any-content.html [ Skip ]
# TODO Conic gradients
http/wpt/css/css-images-4/conic-gradient-parsing.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (290629 => 290630)
--- trunk/Source/WebCore/ChangeLog 2022-03-01 06:12:27 UTC (rev 290629)
+++ trunk/Source/WebCore/ChangeLog 2022-03-01 07:46:09 UTC (rev 290630)
@@ -1,3 +1,47 @@
+2022-02-28 Tyler Wilcock <tyle...@apple.com>
+
+ AX: WebKit should ignore empty modals rather than trapping focus inside them
+ https://bugs.webkit.org/show_bug.cgi?id=237163
+
+ Reviewed by Chris Fleizach and Andres Gonzalez.
+
+ Given this markup:
+
+ <div role="dialog" aria-modal="true">
+ <div aria-hidden="true">
+ <button>Close modal (inside modal)</button>
+ </div>
+ </div>
+
+ There is no accessible content inside this modal, but WebKit traps user focus inside,
+ making the rest of the page completely inaccessible.
+
+ With this patch we ignore modals that don't have accessible content.
+ We do this by walking the DOM to find any non-AX-ignored objects.
+
+ Because determining whether or not an element is ignored is dependent
+ on modals present on the page, this patch moves the call to `ignoredFromModalPresence`
+ out of `AccessibilityObject::defaultObjectInclusion`, as this function is
+ downstream of `computeAccessibilityIsIgnored`, and we need to call
+ that on objects inside modal candidates. Without this move, we would
+ recurse infinitely in `AXObjectCache::modalElementHasAccessibleContent`.
+
+ We now check whether an object is ignored due to modal presence in
+ `AccessibilityObject::accessibilityIsIgnored()`, so that function should
+ be used as the final say in determining whether an object is ignored.
+
+ Test: accessibility/ignore-modals-without-any-content.html
+
+ * accessibility/AXObjectCache.cpp:
+ (WebCore::AXObjectCache::modalElementHasAccessibleContent):
+ (WebCore::AXObjectCache::currentModalNode):
+ * accessibility/AXObjectCache.h:
+ * accessibility/AccessibilityObject.cpp:
+ (WebCore::AccessibilityObject::defaultObjectInclusion const):
+ (WebCore::AccessibilityObject::accessibilityIsIgnored const):
+ * accessibility/AccessibilityObject.h:
+ (WebCore::AccessibilityObject::computeAccessibilityIsIgnored const):
+
2022-02-28 Simon Fraser <simon.fra...@apple.com>
Compositing/paint invalidation with transforms
Modified: trunk/Source/WebCore/accessibility/AXObjectCache.cpp (290629 => 290630)
--- trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2022-03-01 06:12:27 UTC (rev 290629)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2022-03-01 07:46:09 UTC (rev 290630)
@@ -269,6 +269,31 @@
m_modalNodesInitialized = true;
}
+bool AXObjectCache::modalElementHasAccessibleContent(Element& element)
+{
+ // Unless you're trying to compute the new modal node, determining whether an element
+ // has accessible content is as easy as !getOrCreate(element)->children().isEmpty().
+ // So don't call this method on anything besides modal elements.
+ ASSERT(isModalElement(element));
+
+ // Because computing any object's children() is dependent on whether a modal is on the page,
+ // we'll need to walk the DOM and find non-ignored AX objects manually.
+ Vector<Node*> nodeStack = { element.firstChild() };
+ while (!nodeStack.isEmpty()) {
+ for (auto* node = nodeStack.takeLast(); node; node = node->nextSibling()) {
+ if (auto* axObject = getOrCreate(node)) {
+ if (!axObject->computeAccessibilityIsIgnored())
+ return true;
+ }
+
+ // Don't descend into subtrees for non-visible nodes.
+ if (isNodeVisible(node))
+ nodeStack.append(node->firstChild());
+ }
+ }
+ return false;
+}
+
Element* AXObjectCache::currentModalNode()
{
// There might be multiple modal dialog nodes.
@@ -288,14 +313,20 @@
RefPtr<Element> focusedElement = document().focusedElement();
RefPtr<Element> lastVisible;
for (auto& element : m_modalElementsSet) {
- if (isNodeVisible(element)) {
- if (focusedElement && focusedElement->isDescendantOf(element)) {
- m_currentModalElement = element;
- break;
- }
+ // Elements in m_modalElementsSet may have become un-modal since we added them, but not yet removed
+ // as part of the asynchronous m_deferredModalChangedList handling. Skip these.
+ if (!element || !isModalElement(*element))
+ continue;
- lastVisible = element;
+ // To avoid trapping users in an empty modal, skip any non-visible element, or any element without accessible content.
+ if (!isNodeVisible(element) || !modalElementHasAccessibleContent(*element))
+ continue;
+
+ if (focusedElement && focusedElement->isDescendantOf(element)) {
+ m_currentModalElement = element;
+ break;
}
+ lastVisible = element;
}
if (!m_currentModalElement)
Modified: trunk/Source/WebCore/accessibility/AXObjectCache.h (290629 => 290630)
--- trunk/Source/WebCore/accessibility/AXObjectCache.h 2022-03-01 06:12:27 UTC (rev 290629)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.h 2022-03-01 07:46:09 UTC (rev 290630)
@@ -474,6 +474,7 @@
Element* currentModalNode();
bool isNodeVisible(Node*) const;
void handleModalChange(Element&);
+ bool modalElementHasAccessibleContent(Element&);
Document& m_document;
const std::optional<PageIdentifier> m_pageID; // constant for object's lifetime.
Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.cpp (290629 => 290630)
--- trunk/Source/WebCore/accessibility/AccessibilityObject.cpp 2022-03-01 06:12:27 UTC (rev 290629)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.cpp 2022-03-01 07:46:09 UTC (rev 290630)
@@ -3616,7 +3616,7 @@
if (useParentData ? m_isIgnoredFromParentData.isAXHidden : isAXHidden())
return AccessibilityObjectInclusion::IgnoreObject;
- if ((renderer() && renderer()->style().effectiveInert()) || ignoredFromModalPresence())
+ if (renderer() && renderer()->style().effectiveInert())
return AccessibilityObjectInclusion::IgnoreObject;
if (useParentData ? m_isIgnoredFromParentData.isPresentationalChildOfAriaRole : isPresentationalChildOfAriaRole())
@@ -3648,13 +3648,15 @@
}
}
- bool result = computeAccessibilityIsIgnored();
+ bool ignored = ignoredFromModalPresence();
+ if (!ignored)
+ ignored = computeAccessibilityIsIgnored();
// In case computing axIsIgnored disables attribute caching, we should refetch the object to see if it exists.
if (cache && (attributeCache = cache->computedObjectAttributeCache()))
- attributeCache->setIgnored(objectID(), result ? AccessibilityObjectInclusion::IgnoreObject : AccessibilityObjectInclusion::IncludeObject);
+ attributeCache->setIgnored(objectID(), ignored ? AccessibilityObjectInclusion::IgnoreObject : AccessibilityObjectInclusion::IncludeObject);
- return result;
+ return ignored;
}
void AccessibilityObject::elementsFromAttribute(Vector<Element*>& elements, const QualifiedName& attribute) const
Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.h (290629 => 290630)
--- trunk/Source/WebCore/accessibility/AccessibilityObject.h 2022-03-01 06:12:27 UTC (rev 290629)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.h 2022-03-01 07:46:09 UTC (rev 290630)
@@ -276,6 +276,9 @@
Element* element() const override;
Node* node() const override { return nullptr; }
RenderObject* renderer() const override { return nullptr; }
+ // Note: computeAccessibilityIsIgnored does not consider whether an object is ignored due to presence of modals.
+ // Use accessibilityIsIgnored as the word of law when determining if an object is ignored.
+ virtual bool computeAccessibilityIsIgnored() const { return true; }
bool accessibilityIsIgnored() const override;
AccessibilityObjectInclusion defaultObjectInclusion() const override;
bool accessibilityIsIgnoredByDefault() const override;
@@ -793,7 +796,6 @@
void setIsIgnoredFromParentData(AccessibilityIsIgnoredFromParentData& data) override { m_isIgnoredFromParentData = data; }
- virtual bool computeAccessibilityIsIgnored() const { return true; }
bool isAccessibilityObject() const override { return true; }
// If this object itself scrolls, return its ScrollableArea.
