Skip to content

Commit bec672f

Browse files
twilcotwilco
committed
AX: Auto-expand details and hidden="until-found" elements containing search text provided in a AXUIElementsForSearchPredicate request
https://bugs.webkit.org/show_bug.cgi?id=298432 rdar://159913471 Reviewed by Joshua Hoffman. If the user is searching for a specific string, as indicated by search text being present in a AXUIElementsForSearchPredicate request, we should provide matches if said string is inside a collapsed details element or hidden="until-found" container, making sure to expand them as well. This gets a bit tricky in the case of the accessibility thread, which obviously cannot inherently synchronously expand an element. This patch adds a new `performFunctionOnMainThreadAndWaitWithTimeout` function to attempt allow the accessibility thread to attempt a synchronous action, but bail after a timeout if the main-thread is busy. We use this to try to expand revealable elements synchronously, but if we can't, we'll continue on with the search. If we do timeout trying to reveal a container, we won't perform any further synchronous trips to the main-thread for the remainder of the search, with the assumption being that the main-thread is busy doing other things. New property AXProperty::RevealableText is added and cached for text objects within an auto-revealable container. Another property, AXProperty::IsHiddenUntilFoundContainer, is also added to indicate that we should search within the given object for revealable text. * LayoutTests/accessibility/mac/text-search-auto-expands-revealable-elements-expected.txt: Added. * LayoutTests/accessibility/mac/text-search-auto-expands-revealable-elements.html: Added. * Source/WebCore/accessibility/AXCoreObject.cpp: (WebCore::AXCoreObject::revealableContainers): (WebCore::AXCoreObject::detailsAncestor const): * Source/WebCore/accessibility/AXCoreObject.h: (WebCore::Accessibility::performFunctionOnMainThreadAndWaitWithTimeout): * Source/WebCore/accessibility/AXLogger.cpp: (WebCore::operator<<): * Source/WebCore/accessibility/AXNotifications.h: * Source/WebCore/accessibility/AXObjectCache.cpp: (WebCore::AXObjectCache::handleAttributeChange): (WebCore::AXObjectCache::performDeferredCacheUpdate): (WebCore::AXObjectCache::updateIsolatedTree): (WebCore::documentNeedsLayoutOrStyleRecalc): Deleted. * Source/WebCore/accessibility/AXSearchManager.cpp: (WebCore::AXSearchManager::revealHiddenMatchWithTimeout): (WebCore::AXSearchManager::findMatchingObjectsInternal): * Source/WebCore/accessibility/AXSearchManager.h: (WebCore::AXSearchManager::lastRevealAttemptTimedOut): (WebCore::AXSearchManager::setLastRevealAttemptTimedOut): * Source/WebCore/accessibility/AXUtilities.cpp: (WebCore::needsLayoutOrStyleRecalc): * Source/WebCore/accessibility/AXUtilities.h: * Source/WebCore/accessibility/AccessibilityNodeObject.cpp: (WebCore::AccessibilityNodeObject::revealAncestors): (WebCore::AccessibilityNodeObject::isHiddenUntilFoundContainer const): (WebCore::AccessibilityNodeObject::revealableText const): * Source/WebCore/accessibility/AccessibilityNodeObject.h: * Source/WebCore/accessibility/AccessibilityObject.cpp: (WebCore::AccessibilityObject::defaultObjectInclusion const): * Source/WebCore/accessibility/AccessibilityObject.h: (WebCore::AccessibilityObject::revealAncestors): * Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h: * Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp: (WebCore::AXIsolatedTree::updateNodeProperties): (WebCore::convertToPropertyFlag): (WebCore::createIsolatedObjectData): * Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h: (WebCore::AXIsolatedTree::objectBecameIgnored): Canonical link: https://commits.webkit.org/299649@main
1 parent f98e5ce commit bec672f

18 files changed

Lines changed: 403 additions & 47 deletions

‎LayoutTests/accessibility/mac/text-search-auto-expands-revealable-elements-expected.txt‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
This test ensures that AXUIElementsForSearchPredicate queries with a search-text component auto-expands collapsed details and hidden='until-found' elements if a match is found within them.
2+
3+
PASS: resultElement.role === 'AXRole: AXStaticText'
4+
PASS: resultElement.stringValue === 'AXValue: First'
5+
PASS: document.getElementById('details').hasAttribute('open') === true
6+
PASS: !!resultElement === false
7+
PASS: document.getElementById('details-2').hasAttribute('open') === false
8+
PASS: resultElement.role === 'AXRole: AXStaticText'
9+
PASS: resultElement.stringValue === 'AXValue: Third'
10+
PASS: document.getElementById('hidden').hasAttribute('hidden') === false
11+
PASS: !!resultElement === false
12+
PASS: document.getElementById('permanent-hidden').hasAttribute('hidden') === true
13+
14+
PASS successfullyParsed is true
15+
16+
TEST COMPLETE
17+
Foo
18+
First
19+
Foo
20+
Third
21+

‎LayoutTests/accessibility/mac/text-search-auto-expands-revealable-elements.html‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
2+
<html>
3+
<head>
4+
<script src="../../resources/accessibility-helper.js"></script>
5+
<script src="../../resources/js-test.js"></script>
6+
</head>
7+
<body>
8+
9+
<details id="details">
10+
<summary>Foo</summary>
11+
First
12+
</details>
13+
14+
<details id="details-2">
15+
<summary>Foo</summary>
16+
Second
17+
</details>
18+
19+
<div id="hidden" hidden="until-found">
20+
Third
21+
</div>
22+
23+
<div aria-hidden="true">
24+
<div id="permanent-hidden" hidden="until-found">
25+
Fourth
26+
</div>
27+
</div>
28+
29+
<script>
30+
var output = "This test ensures that AXUIElementsForSearchPredicate queries with a search-text component auto-expands collapsed details and hidden='until-found' elements if a match is found within them.\n\n";
31+
32+
// Normally, tests that perform dynamic page changes must be async, allowing time for updates to the isolated tree.
33+
// However, the expansion behavior being tested here tries to perform the expansion via synchronous main-thread hit
34+
// with a timeout. We should never even get close to the timeout when running layout tests.
35+
36+
if (window.accessibilityController) {
37+
var webarea = accessibilityController.rootElement.childAtIndex(0);
38+
var resultElement = webarea.uiElementForSearchPredicate(webarea,
39+
/* isDirectionNext */ true,
40+
"AXAnyTypeSearchKey",
41+
"First",
42+
/* visibleOnly */ false,
43+
/* immediateDescendantsOnly */ false
44+
);
45+
output += expect("resultElement.role", "'AXRole: AXStaticText'");
46+
output += expect("resultElement.stringValue", "'AXValue: First'");
47+
output += expect("document.getElementById('details').hasAttribute('open')", "true");
48+
49+
// Ensure we respect the visibleOnly flag, which shouldn't auto-expand revealable elements.
50+
var resultElement = webarea.uiElementForSearchPredicate(webarea,
51+
/* isDirectionNext */ true,
52+
"AXAnyTypeSearchKey",
53+
"Second",
54+
/* visibleOnly */ true,
55+
/* immediateDescendantsOnly */ false
56+
);
57+
output += expect("!!resultElement", "false");
58+
output += expect("document.getElementById('details-2').hasAttribute('open')", "false");
59+
60+
// We should expand a hidden="until-found" container.
61+
var resultElement = webarea.uiElementForSearchPredicate(webarea,
62+
/* isDirectionNext */ true,
63+
"AXAnyTypeSearchKey",
64+
"Third",
65+
/* visibleOnly */ false,
66+
/* immediateDescendantsOnly */ false
67+
);
68+
output += expect("resultElement.role", "'AXRole: AXStaticText'");
69+
output += expect("resultElement.stringValue", "'AXValue: Third'");
70+
output += expect("document.getElementById('hidden').hasAttribute('hidden')", "false");
71+
72+
// Don't auto-expand aria-hidden hidden="until-found" containers.
73+
var resultElement = webarea.uiElementForSearchPredicate(webarea,
74+
/* isDirectionNext */ true,
75+
"AXAnyTypeSearchKey",
76+
"Fourth",
77+
/* visibleOnly */ false,
78+
/* immediateDescendantsOnly */ false
79+
);
80+
output += expect("!!resultElement", "false");
81+
output += expect("document.getElementById('permanent-hidden').hasAttribute('hidden')", "true");
82+
83+
debug(output);
84+
}
85+
</script>
86+
</body>
87+
</html>
88+

‎Source/WebCore/accessibility/AXCoreObject.cpp‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,6 +1099,26 @@ bool AXCoreObject::isReplacedElement() const
10991099
}
11001100
}
11011101

1102+
AXCoreObject::AccessibilityChildrenVector AXCoreObject::revealableContainers()
1103+
{
1104+
AXCoreObject::AccessibilityChildrenVector revealableContainers;
1105+
1106+
auto isCollapsedDetails = [this] {
1107+
return role() == AccessibilityRole::Details && !isExpanded();
1108+
};
1109+
if (isHiddenUntilFoundContainer() || isCollapsedDetails())
1110+
revealableContainers.append(*this);
1111+
1112+
if (role() == AccessibilityRole::Summary) {
1113+
// When rendered, the summary element is the thing that assistive technologies can actually
1114+
// navigate to. Return our containing details as the revealable container so we can search
1115+
// inside the details subtree for revealable text.
1116+
if (RefPtr details = detailsAncestor(); details && !details->isExpanded())
1117+
revealableContainers.append(details.releaseNonNull());
1118+
}
1119+
return revealableContainers;
1120+
}
1121+
11021122
bool AXCoreObject::containsOnlyStaticText() const
11031123
{
11041124
bool hasText = false;
@@ -1630,6 +1650,13 @@ AXCoreObject* AXCoreObject::parentObjectUnignored() const
16301650
});
16311651
}
16321652

1653+
AXCoreObject* AXCoreObject::detailsAncestor() const
1654+
{
1655+
return Accessibility::findAncestor<AXCoreObject>(*this, false, [&] (const AXCoreObject& object) {
1656+
return object.role() == AccessibilityRole::Details;
1657+
});
1658+
}
1659+
16331660
// This function implements a fast way to determine our ordering relative to |other|: find the
16341661
// ancestor we share, then compare the index-in-parent of the next lowest descendant of each us
16351662
// and |other|. Take this example:

‎Source/WebCore/accessibility/AXCoreObject.h‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
#include <wtf/RefCounted.h>
4949
#include <wtf/ThreadSafeWeakPtr.h>
5050
#include <wtf/WallTime.h>
51+
#include <wtf/threads/BinarySemaphore.h>
5152

5253
#if PLATFORM(WIN)
5354
#include "AccessibilityObjectWrapperWin.h"
@@ -247,6 +248,7 @@ enum class AccessibilityOrientation : uint8_t {
247248
Vertical
248249
};
249250

251+
enum class DidTimeout : bool { No, Yes };
250252
enum class IncludeListMarkerText : bool { No, Yes };
251253
enum class TrimWhitespace : bool { No, Yes };
252254

@@ -815,6 +817,13 @@ class AXCoreObject : public RefCountedAndCanMakeWeakPtr<AXCoreObject> {
815817
virtual String textUnderElement(TextUnderElementMode = { }) const = 0;
816818
virtual String text() const = 0;
817819
virtual unsigned textLength() const = 0;
820+
AccessibilityChildrenVector revealableContainers();
821+
// Text of objects within revealable containers (e.g. hidden="until-found" or collapsed details elements).
822+
// Returns empty string for text that is already revealed / visible.
823+
virtual String revealableText() const = 0;
824+
// The word "container" is significant in this method name. This should only be true for elements / objects
825+
// that actually have the hidden-until-found markup, not descendants of hidden-until-found elements / objects.
826+
virtual bool isHiddenUntilFoundContainer() const = 0;
818827
#if PLATFORM(COCOA)
819828
enum class SpellCheck : bool { No, Yes };
820829
virtual RetainPtr<NSAttributedString> attributedStringForTextMarkerRange(AXTextMarkerRange&&, SpellCheck) const = 0;
@@ -1195,6 +1204,7 @@ class AXCoreObject : public RefCountedAndCanMakeWeakPtr<AXCoreObject> {
11951204
virtual AXCoreObject* editableAncestor() const = 0;
11961205
virtual AXCoreObject* highestEditableAncestor() = 0;
11971206
virtual AXCoreObject* exposedTableAncestor(bool includeSelf = false) const = 0;
1207+
AXCoreObject* detailsAncestor() const;
11981208

11991209
virtual AccessibilityChildrenVector documentLinks() = 0;
12001210

@@ -1574,6 +1584,41 @@ template<typename U> inline void performFunctionOnMainThread(U&& lambda)
15741584
});
15751585
}
15761586

1587+
template<typename U>
1588+
inline DidTimeout performFunctionOnMainThreadAndWaitWithTimeout(U&& lambda, Seconds timeout)
1589+
{
1590+
if (isMainThread()) {
1591+
std::forward<U>(lambda)();
1592+
return DidTimeout::No;
1593+
}
1594+
1595+
// Because this is ref-counted, we can give it to the lambda to keep alive
1596+
// even if this thread gave up due to a timeout and moved on (which would normally destroy
1597+
// the semaphore, causing a use-after-free).
1598+
struct TimeoutSafeSemaphore : RefCounted<TimeoutSafeSemaphore> {
1599+
BinarySemaphore semaphore;
1600+
std::atomic<bool> shouldSignal { true };
1601+
1602+
void signal() { semaphore.signal(); }
1603+
bool wait(Seconds timeout) { return semaphore.waitFor(timeout); }
1604+
};
1605+
1606+
Ref<TimeoutSafeSemaphore> semaphore = adoptRef(*new TimeoutSafeSemaphore);
1607+
ensureOnMainThread([semaphore, lambda = std::forward<U>(lambda)] () mutable {
1608+
lambda();
1609+
// Only signal if the calling thread didn't timeout waiting for the main-thread to complete the lambda.
1610+
if (semaphore->shouldSignal.exchange(false, std::memory_order_acq_rel))
1611+
semaphore->signal();
1612+
});
1613+
1614+
bool completedInTime = semaphore->wait(timeout);
1615+
if (!completedInTime) {
1616+
// If we timed out, prevent a later signal attempt from the lambda.
1617+
semaphore->shouldSignal.exchange(false, std::memory_order_acq_rel);
1618+
}
1619+
return completedInTime ? DidTimeout::No : DidTimeout::Yes;
1620+
}
1621+
15771622
template<typename T, typename U> inline T retrieveValueFromMainThread(U&& lambda)
15781623
{
15791624
std::optional<T> value;

‎Source/WebCore/accessibility/AXLogger.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -815,6 +815,9 @@ TextStream& operator<<(WTF::TextStream& stream, AXProperty property)
815815
case AXProperty::IsEditableWebArea:
816816
stream << "IsEditableWebArea";
817817
break;
818+
case AXProperty::IsHiddenUntilFoundContainer:
819+
stream << "IsHiddenUntilFoundContainer";
820+
break;
818821
case AXProperty::IsSubscript:
819822
stream << "IsSubscript";
820823
break;
@@ -1106,6 +1109,9 @@ TextStream& operator<<(WTF::TextStream& stream, AXProperty property)
11061109
stream << "RemoteParent";
11071110
break;
11081111
#endif
1112+
case AXProperty::RevealableText:
1113+
stream << "RevealableText";
1114+
break;
11091115
case AXProperty::RolePlatformString:
11101116
stream << "RolePlatformString";
11111117
break;

‎Source/WebCore/accessibility/AXNotifications.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ namespace WebCore {
6565
macro(FrameLoadComplete) \
6666
macro(GrabbedStateChanged) \
6767
macro(HasPopupChanged) \
68+
macro(HiddenStateChanged) \
6869
macro(IdAttributeChanged) \
6970
macro(ImageOverlayChanged) \
7071
macro(InertOrVisibilityChanged) \

‎Source/WebCore/accessibility/AXObjectCache.cpp‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2970,6 +2970,8 @@ void AXObjectCache::handleAttributeChange(Element* element, const QualifiedName&
29702970
postNotification(element, AXNotification::DatetimeChanged);
29712971
else if (attrName == abbrAttr)
29722972
postNotification(element, AXNotification::AbbreviationChanged);
2973+
else if (attrName == hiddenAttr)
2974+
postNotification(element, AXNotification::HiddenStateChanged);
29732975

29742976
if (!attrName.localName().string().startsWith("aria-"_s))
29752977
return;
@@ -4515,16 +4517,6 @@ bool AXObjectCache::elementIsTextControl(const Element& element)
45154517
return axObject && axObject->isTextControl();
45164518
}
45174519

4518-
static bool documentNeedsLayoutOrStyleRecalc(Document& document)
4519-
{
4520-
if (RefPtr frameView = document.view()) {
4521-
if (frameView->needsLayout() || frameView->layoutContext().isLayoutPending())
4522-
return true;
4523-
}
4524-
4525-
return document.hasPendingStyleRecalc();
4526-
}
4527-
45284520
void AXObjectCache::performDeferredCacheUpdate(ForceLayout forceLayout)
45294521
{
45304522
AXTRACE(makeString("AXObjectCache::performDeferredCacheUpdate 0x"_s, hex(reinterpret_cast<uintptr_t>(this))));
@@ -4544,7 +4536,7 @@ void AXObjectCache::performDeferredCacheUpdate(ForceLayout forceLayout)
45444536
if (!document->view())
45454537
return;
45464538

4547-
if (documentNeedsLayoutOrStyleRecalc(*document)) {
4539+
if (needsLayoutOrStyleRecalc(*document)) {
45484540
// Layout became dirty while waiting to performDeferredCacheUpdate, and we require clean layout
45494541
// to update the accessibility tree correctly in this function.
45504542
if ((m_cacheUpdateDeferredCount >= 3 || forceLayout == ForceLayout::Yes) && !Accessibility::inRenderTreeOrStyleUpdate(*document)) {
@@ -4564,7 +4556,7 @@ void AXObjectCache::performDeferredCacheUpdate(ForceLayout forceLayout)
45644556
for (; frame; frame = frame->tree().traverseNext()) {
45654557
auto* localFrame = dynamicDowncast<LocalFrame>(frame.get());
45664558
RefPtr subDocument = localFrame ? localFrame->document() : nullptr;
4567-
if (subDocument && documentNeedsLayoutOrStyleRecalc(*subDocument))
4559+
if (subDocument && needsLayoutOrStyleRecalc(*subDocument))
45684560
subDocument->updateLayoutIgnorePendingStylesheets();
45694561
}
45704562
}
@@ -4883,6 +4875,9 @@ void AXObjectCache::updateIsolatedTree(const Vector<std::pair<Ref<AccessibilityO
48834875
case AXNotification::FontChanged:
48844876
tree->updatePropertiesForSelfAndDescendants(notification.first.get(), { AXProperty::Font });
48854877
break;
4878+
case AXNotification::HiddenStateChanged:
4879+
tree->queueNodeUpdate(notification.first->objectID(), { AXProperty::IsHiddenUntilFoundContainer });
4880+
break;
48864881
case AXNotification::InertOrVisibilityChanged:
48874882
tree->updatePropertiesForSelfAndDescendants(notification.first.get(), { AXProperty::IsIgnored });
48884883
break;

0 commit comments

Comments
 (0)