Skip to content

Commit

Permalink
y nAX: Move selectedTextRange off of the main-thread
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=263940
rdar://117568720

Reviewed by Andres Gonzalez.

We currently use the main thread to get the selectedTextRange when in isolated tree mode. In the
effort to completely sever ITM, this should be cached to be served on the AX thread.

This patch caches that value on text input objects, and updates the property every time text
selection changes. This update is debounced to account for text selection in-progress and
minimize computation.

* LayoutTests/accessibility/content-editable-as-textarea.html:
* LayoutTests/accessibility/textbox-role-reports-selection.html:
* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::AXObjectCache):
(WebCore::AXObjectCache::~AXObjectCache):
(WebCore::AXObjectCache::postTextStateChangeNotification):
(WebCore::AXObjectCache::updateIsolatedTree):
(WebCore::AXObjectCache::selectedTextRangeTimerFired):
* Source/WebCore/accessibility/AXObjectCache.h:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::initializeProperties):
(WebCore::AXIsolatedObject::selectedTextRange const): Deleted.
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::updateNodeProperties):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:
* Source/WebCore/accessibility/mac/AXObjectCacheMac.mm:
(WebCore::AXObjectCache::platformSelectedTextRangeDebounceInterval const):

Canonical link: https://commits.webkit.org/270340@main
  • Loading branch information
hoffmanjoshua authored and AndresGonzalezApple committed Nov 7, 2023
1 parent e2dca80 commit ae0dcb5
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 27 deletions.
13 changes: 8 additions & 5 deletions LayoutTests/accessibility/content-editable-as-textarea.html
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,15 @@
var sel = window.getSelection();
sel.addRange(range);

testOutput += `Selected text range: ${textArea.selectedTextRange}\n`;
testOutput += `Selected text: ${textArea.stringAttributeValue("AXSelectedText")}\n`;
setTimeout(async () => {
await waitFor(() => textArea.selectedTextRange == "{0, 3}");
testOutput += `Selected text range: ${textArea.selectedTextRange}\n`;
testOutput += `Selected text: ${textArea.stringAttributeValue("AXSelectedText")}\n`;

// Send a value change.
document.getElementById("content").focus();
eventSender.keyDown(targetCharacter, []);
// Send a value change.
document.getElementById("content").focus();
eventSender.keyDown(targetCharacter, []);
}, 0);
}
</script>
</body>
Expand Down
27 changes: 18 additions & 9 deletions LayoutTests/accessibility/textbox-role-reports-selection.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<html>
<head>
<script src="../resources/js-test.js"></script>
<script src="../resources/accessibility-helper.js"></script>
</head>
<body>
This tests that the AXSelection property is correctly reported for non-native text boxes.<br>
Expand All @@ -22,7 +23,7 @@
debug("FAIL: " + actual + " should be " + expected + ", got " + actualValue + (message ? " (" + message + ")" : ""));
}

function assertCorrectAXSelection(element, selection, message) {
async function assertCorrectAXSelection(element, selection, message) {
element.focus();
var selectionValues = /\{(\d+), (\d+)\}/.exec(selection);
var selectionStart = eval(selectionValues[1]);
Expand All @@ -31,20 +32,28 @@

window.getSelection().setBaseAndExtent(element.firstChild, selectionStart, element.firstChild, selectionEnd);
var axElement = accessibilityController.focusedElement;
axSelection = axElement.selectedTextRange;
await waitFor(() => {
axSelection = axElement.selectedTextRange;
return axSelection == selection;
});
assertEvaluatesTo("axSelection", selection, message);
}

if (window.testRunner && window.accessibilityController) {
window.jsTestIsAsync = true;
var ariaTextBox = document.getElementById("ariaTextBox");
var textLength = ariaTextBox.textContent.length;

assertCorrectAXSelection(ariaTextBox, "{0, 0}", "Collapsed selection at start");
assertCorrectAXSelection(ariaTextBox, "{" + textLength + ", 0}", "Collapsed selection at end");
assertCorrectAXSelection(ariaTextBox, "{15, 0}", "Collapsed selection in the middle");
assertCorrectAXSelection(ariaTextBox, "{15, 2}", "Non-collapsed selection in the middle");
assertCorrectAXSelection(ariaTextBox, "{0, 15}", "Non-collapsed selection at the start");
assertCorrectAXSelection(ariaTextBox, "{15, "+ (textLength - 15) + "}", "Non-collapsed selection at the end");

setTimeout(async () => {
await assertCorrectAXSelection(ariaTextBox, "{0, 0}", "Collapsed selection at start");
await assertCorrectAXSelection(ariaTextBox, "{" + textLength + ", 0}", "Collapsed selection at end");
await assertCorrectAXSelection(ariaTextBox, "{15, 0}", "Collapsed selection in the middle");
await assertCorrectAXSelection(ariaTextBox, "{15, 2}", "Non-collapsed selection in the middle");
await assertCorrectAXSelection(ariaTextBox, "{0, 15}", "Non-collapsed selection at the start");
await assertCorrectAXSelection(ariaTextBox, "{15, "+ (textLength - 15) + "}", "Non-collapsed selection at the end");

finishJSTest();
}, 0);
}

</script>
Expand Down
31 changes: 31 additions & 0 deletions Source/WebCore/accessibility/AXObjectCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ AXObjectCache::AXObjectCache(Document& document)
#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
, m_buildIsolatedTreeTimer(*this, &AXObjectCache::buildIsolatedTree)
, m_geometryManager(AXGeometryManager::create(*this))
, m_selectedTextRangeTimer(*this, &AXObjectCache::selectedTextRangeTimerFired, platformSelectedTextRangeDebounceInterval())
#endif
{
AXTRACE(makeString("AXObjectCache::AXObjectCache 0x"_s, hex(reinterpret_cast<uintptr_t>(this))));
Expand Down Expand Up @@ -279,6 +280,8 @@ AXObjectCache::~AXObjectCache()
object->detach(AccessibilityDetachmentType::CacheDestroyed);

#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
m_selectedTextRangeTimer.stop();

if (m_pageID) {
if (auto tree = AXIsolatedTree::treeForPageID(*m_pageID))
tree->setPageActivityState({ });
Expand Down Expand Up @@ -1997,6 +2000,12 @@ void AXObjectCache::postTextStateChangeNotification(AccessibilityObject* object,
if (object) {
const AXTextStateChangeIntent& newIntent = (intent.type == AXTextStateChangeTypeUnknown || (m_isSynchronizingSelection && m_textSelectionIntent.type != AXTextStateChangeTypeUnknown)) ? m_textSelectionIntent : intent;
postTextStateChangePlatformNotification(object, newIntent, selection);

#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
m_lastDebouncedTextRangeObject = object->objectID();
if (!m_selectedTextRangeTimer.isActive())
m_selectedTextRangeTimer.restart();
#endif
}
#if PLATFORM(MAC)
onSelectedTextChanged(selection);
Expand Down Expand Up @@ -4200,6 +4209,9 @@ void AXObjectCache::updateIsolatedTree(const Vector<std::pair<RefPtr<Accessibili
case AXPopoverTargetChanged:
tree->updateNodeProperties(*notification.first, { AXPropertyName::SupportsExpanded, AXPropertyName::IsExpanded });
break;
case AXSelectedTextChanged:
tree->updateNodeProperty(*notification.first, AXPropertyName::SelectedTextRange);
break;
case AXSortDirectionChanged:
tree->updateNodeProperty(*notification.first, AXPropertyName::SortDirection);
break;
Expand Down Expand Up @@ -4877,6 +4889,25 @@ AXTextChange AXObjectCache::textChangeForEditType(AXTextEditType type)
}
#endif

#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
void AXObjectCache::selectedTextRangeTimerFired()
{
if (!accessibilityEnabled())
return;

m_selectedTextRangeTimer.stop();

if (m_lastDebouncedTextRangeObject.isValid()) {
for (auto* axObject = objectForID(m_lastDebouncedTextRangeObject); axObject; axObject = axObject->parentObject()) {
if (axObject->isTextControl())
postNotification(axObject, nullptr, AXSelectedTextChanged);
}
}

m_lastDebouncedTextRangeObject = { };
}
#endif

} // namespace WebCore

#endif // ENABLE(ACCESSIBILITY)
7 changes: 7 additions & 0 deletions Source/WebCore/accessibility/AXObjectCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,11 @@ class AXObjectCache : public CanMakeWeakPtr<AXObjectCache>, public CanMakeChecke
bool enqueuePasswordValueChangeNotification(AccessibilityObject*);
void passwordNotificationPostTimerFired();

#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
void selectedTextRangeTimerFired();
Seconds platformSelectedTextRangeDebounceInterval() const;
#endif

void deferRowspanChange(AccessibilityObject*);
void handleChildrenChanged(AccessibilityObject&);
void handleAllDeferredChildrenChanged();
Expand Down Expand Up @@ -707,6 +712,8 @@ class AXObjectCache : public CanMakeWeakPtr<AXObjectCache>, public CanMakeChecke
Timer m_buildIsolatedTreeTimer;
bool m_deferredRegenerateIsolatedTree { false };
Ref<AXGeometryManager> m_geometryManager;
DeferrableOneShotTimer m_selectedTextRangeTimer;
AXID m_lastDebouncedTextRangeObject;
#endif
bool m_isSynchronizingSelection { false };
bool m_performingDeferredCacheUpdate { false };
Expand Down
15 changes: 3 additions & 12 deletions Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,9 @@ void AXIsolatedObject::initializeProperties(const Ref<AccessibilityObject>& axOb
if (descriptor.length())
setProperty(AXPropertyName::ExtendedDescription, descriptor.isolatedCopy());

if (object.isTextControl())
setProperty(AXPropertyName::SelectedTextRange, object.selectedTextRange());

// These properties are only needed on the AXCoreObject interface due to their use in ATSPI,
// so only cache them for ATSPI.
#if PLATFORM(ATSPI)
Expand Down Expand Up @@ -1261,18 +1264,6 @@ bool AXIsolatedObject::isNativeTextControl() const
return false;
}

CharacterRange AXIsolatedObject::selectedTextRange() const
{
if (shouldReturnEmptySelectedText())
return { };

return Accessibility::retrieveValueFromMainThread<CharacterRange>([this] () -> CharacterRange {
if (auto* object = associatedAXObject())
return object->selectedTextRange();
return { };
});
}

int AXIsolatedObject::insertionPointLineNumber() const
{
return Accessibility::retrieveValueFromMainThread<int>([this] () -> int {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ class AXIsolatedObject final : public AXCoreObject {
#endif

// CharacterRange support.
CharacterRange selectedTextRange() const final;
CharacterRange selectedTextRange() const final { return propertyValue<CharacterRange>(AXPropertyName::SelectedTextRange); }
int insertionPointLineNumber() const final;
CharacterRange doAXRangeForLine(unsigned) const final;
String doAXStringForRange(const CharacterRange&) const final;
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,9 @@ void AXIsolatedTree::updateNodeProperties(AXCoreObject& axObject, const Vector<A
case AXPropertyName::CellScope:
propertyMap.set(AXPropertyName::CellScope, axObject.cellScope().isolatedCopy());
break;
case AXPropertyName::SelectedTextRange:
propertyMap.set(AXPropertyName::SelectedTextRange, axObject.selectedTextRange());
break;
case AXPropertyName::SetSize:
propertyMap.set(AXPropertyName::SetSize, axObject.setSize());
break;
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ enum class AXPropertyName : uint16_t {
RowIndexRange,
ScreenRelativePosition,
SelectedChildren,
SelectedTextRange,
SetSize,
SortDirection,
SpeechHint,
Expand Down
7 changes: 7 additions & 0 deletions Source/WebCore/accessibility/mac/AXObjectCacheMac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,13 @@ static bool isTestAXClientType(AXClientType client)
#endif
}

#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
Seconds AXObjectCache::platformSelectedTextRangeDebounceInterval() const
{
return 100_ms;
}
#endif

// TextMarker and TextMarkerRange funcstions.
// FIXME: TextMarker and TextMarkerRange should become classes wrapping the system objects.

Expand Down

0 comments on commit ae0dcb5

Please sign in to comment.