Skip to content

AX: Implement support for serving AXVisibleCharacterRange off the main-thread#46860

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
twilco:eng/AX-Implement-support-for-serving-AXVisibleCharacterRange-off-the-main-thread
Jun 18, 2025
Merged

AX: Implement support for serving AXVisibleCharacterRange off the main-thread#46860
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
twilco:eng/AX-Implement-support-for-serving-AXVisibleCharacterRange-off-the-main-thread

Conversation

@twilco
Copy link
Copy Markdown
Contributor

@twilco twilco commented Jun 17, 2025

87a311b

AX: Implement support for serving AXVisibleCharacterRange off the main-thread
https://bugs.webkit.org/show_bug.cgi?id=294619
rdar://153652827

Reviewed by Joshua Hoffman.

With this commit, we implement support for serving AXVisibleCharacterRange off the main-thread. The high level approach is:

  1. Accessibility paints already paint all InlineIterator::TextBoxes that are visible. When this happens, keep track
     of the line indices of the painted text boxes, and associate that with a RenderText, which in turn is associated
     with an accessibility object.

  2. Cache the first painted line and last painted line index for each RenderText in a HashMap (m_mostRecentlyPaintedText),
     and sync this HashMap to the accessibility thread.

  3. Implement AXIsolatedObject::visibleCharacterRange using this HashMap. This is trivial for leaf nodes. For non-leaf
     nodes, we iterate over the descendants until we find the first painted one. The `location` of the visible NSRange
     is the text of all descendants (which are unpainted) up to the first painted one. The `length` of the visible range
     is then the length from the first painted line, to the last painted line of the last painted descendant.

This algorithm produces very different results from the main-thread visible character range implementation, which is
based on VisiblePosition line-by-line iteration to fit an objects element rect into the viewport rect. In general, the
new algorithm seems much more accurate, and this can be seen in the layout tests modified in this patch (I left code
comments in each test).

This patch also stops outputting AXVisibleCharacterRange in tests that dump every attribute for every element. Because
the algorithms produce slightly different results in ways that are not clearly relevant to any user-facing behavior,
not outputting AXVisibleCharacterRange in these tests allows us to keep the tests running in both configurations. Some
tests have also been re-written to include better test corpus text — logging "1: Lineone lineone", "2: Linetwo linetwo"
rather than the word "test" thousands of times, which is harder to debug with the --show-window test runner flag.

* LayoutTests/accessibility/image-link-expected.txt:
* LayoutTests/accessibility/image-map2-expected.txt:
* LayoutTests/accessibility/internal-link-anchors2-expected.txt:
* LayoutTests/accessibility/mac/document-attributes-expected.txt:
* LayoutTests/accessibility/mac/document-links-expected.txt:
* LayoutTests/accessibility/mac/visible-character-range-width-changes-expected.txt: Added.
* LayoutTests/accessibility/mac/visible-character-range-width-changes.html: Added.
* LayoutTests/accessibility/mixed-contenteditable-double-br-visible-character-range-hang-expected.txt:
* LayoutTests/accessibility/mixed-contenteditable-double-br-visible-character-range-hang.html:
* LayoutTests/accessibility/mixed-contenteditable-visible-character-range-hang-expected.txt:
* LayoutTests/accessibility/mixed-contenteditable-visible-character-range-hang.html:
* LayoutTests/accessibility/table-attributes-expected.txt:
* LayoutTests/accessibility/table-multiple-tbodies-expected.txt:
* LayoutTests/accessibility/table-one-cell-expected.txt:
* LayoutTests/accessibility/table-thead-tfoot-expected.txt:
* LayoutTests/accessibility/table-with-rules-expected.txt:
* LayoutTests/accessibility/transformed-element-expected.txt:
* LayoutTests/accessibility/visible-character-range-basic-expected.txt: Added.
* LayoutTests/accessibility/visible-character-range-basic.html:
* LayoutTests/accessibility/visible-character-range-height-changes-expected.txt: Added.
* LayoutTests/accessibility/visible-character-range-height-changes.html:
* LayoutTests/accessibility/visible-character-range-scrolling.html:
* LayoutTests/accessibility/visible-character-range-vertical-rl-expected.txt:
* LayoutTests/accessibility/visible-character-range-vertical-rl.html:
* LayoutTests/accessibility/visible-character-range-width-changes.html: Removed.
* LayoutTests/platform/ios/TestExpectations:
* LayoutTests/platform/ios/accessibility/mixed-contenteditable-double-br-visible-character-range-hang-expected.txt: Removed.
* LayoutTests/platform/ios/accessibility/mixed-contenteditable-visible-character-range-hang-expected.txt: Removed.
* LayoutTests/platform/ios/accessibility/visible-character-range-basic-expected.txt: Removed.
* LayoutTests/platform/ios/accessibility/visible-character-range-height-changes-expected.txt: Removed.
* LayoutTests/platform/ios/accessibility/visible-character-range-width-changes-expected.txt: Removed.
* LayoutTests/platform/mac/accessibility/lists-expected.txt:
* LayoutTests/platform/mac/accessibility/math-multiscript-attributes-expected.txt:
* LayoutTests/platform/mac/accessibility/visible-character-range-basic-expected.txt: Removed.
* LayoutTests/platform/mac/accessibility/visible-character-range-height-changes-expected.txt: Removed.
* LayoutTests/platform/mac/accessibility/visible-character-range-scrolling-expected.txt:
* LayoutTests/platform/mac/accessibility/visible-character-range-width-changes-expected.txt: Removed.
* LayoutTests/resources/accessibility-helper.js:
* Source/WebCore/accessibility/AXCoreObject.h:
(WebCore::LineRange::LineRange):
(WebCore::LineRange::debugDescription const):
* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::mostRecentlyPaintedText):
(WebCore::AXObjectCache::onAccessibilityPaintStarted):
(WebCore::AXObjectCache::onAccessibilityPaintFinished):
(WebCore::AXObjectCache::onPaint):
* Source/WebCore/accessibility/AXObjectCache.h:
* Source/WebCore/accessibility/AXTextRun.h:
(WebCore::AXTextRun::characterAt const):
(WebCore::AXTextRun::textLength const):
* Source/WebCore/accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::previousLineStartBoundaryPoints const): Deleted.
(WebCore::AccessibilityObject::lastBoundaryPointContainedInRect const): Deleted.
(WebCore::textStartPoint): Deleted.
(WebCore::textEndPoint): Deleted.
(WebCore::AccessibilityObject::boundaryPointsContainedInRect const): Deleted.
(WebCore::AccessibilityObject::visibleCharacterRange const): Deleted.
(WebCore::AccessibilityObject::visibleCharacterRangeInternal const): Deleted.
* Source/WebCore/accessibility/AccessibilityObject.h:
* Source/WebCore/accessibility/cocoa/AccessibilityObjectCocoa.mm:
(WebCore::textStartPoint):
(WebCore::textEndPoint):
(WebCore::AccessibilityObject::previousLineStartBoundaryPoints const):
(WebCore::AccessibilityObject::lastBoundaryPointContainedInRect const):
(WebCore::AccessibilityObject::boundaryPointsContainedInRect const):
(WebCore::AccessibilityObject::visibleCharacterRange const):
(WebCore::AccessibilityObject::visibleCharacterRangeInternal const):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::visibleCharacterRange const): Deleted.
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::relatedObjectIDsFor):
(WebCore::AXIsolatedTree::applyPendingChanges):
(WebCore::AXIsolatedTree::sortedNonRootWebAreasDidChange):
(WebCore::AXIsolatedTree::processQueuedNodeUpdates):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:
(WebCore::AXIsolatedTree::markMostRecentlyPaintedTextDirty):
(WebCore::AXIsolatedTree::mostRecentlyPaintedText const):
* Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm:
(WebCore::AXIsolatedObject::visibleCharacterRange const):
* Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm:
(-[WebAccessibilityObjectWrapperBase accessibilityVisibleCharacterRange]):
* Source/WebCore/dom/BoundaryPoint.cpp:
(WebCore::BoundaryPoint::debugDescription const):
* Source/WebCore/dom/BoundaryPoint.h:
* Source/WebCore/dom/SimpleRange.cpp:
(WebCore::SimpleRange::debugDescription const):
* Source/WebCore/dom/SimpleRange.h:
* Source/WebCore/page/Page.cpp:
(WebCore::Page::doAfterUpdateRendering):
* Source/WebCore/rendering/AccessibilityRegionContext.cpp:
(WebCore::AccessibilityRegionContext::~AccessibilityRegionContext):
(WebCore::AccessibilityRegionContext::takeBounds):
* Source/WebCore/rendering/AccessibilityRegionContext.h:
* Source/WebCore/rendering/TextBoxPainter.cpp:
(WebCore::TextBoxPainter::paint):
* Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:
(WTR::AccessibilityUIElement::allAttributes):

Canonical link: https://commits.webkit.org/296359@main

e53ec8b

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@twilco twilco self-assigned this Jun 17, 2025
@twilco twilco added the Accessibility For bugs related to accessibility. label Jun 17, 2025
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 17, 2025
@twilco twilco force-pushed the eng/AX-Implement-support-for-serving-AXVisibleCharacterRange-off-the-main-thread branch from b626677 to 7acb363 Compare June 17, 2025 18:15
@webkit-ews-buildbot
Copy link
Copy Markdown
Collaborator

Safer C++ Build #40719 (b626677)

❌ Found 1 failing file with 1 issue. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

@twilco twilco force-pushed the eng/AX-Implement-support-for-serving-AXVisibleCharacterRange-off-the-main-thread branch 2 times, most recently from bb93be0 to e57a6d2 Compare June 17, 2025 19:56
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious, if you use display:none instead can you eliminate the extra whitespace at the end of the results?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Done in the latest commit.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For any range data type I think it's helpful to include a comment saying whether the end index is inclusive or exclusive

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! Done in latest commit.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a radar for this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just filed:

https://bugs.webkit.org/show_bug.cgi?id=294632

And added a link to this bug in code comments in both places that will need changes.

@twilco twilco force-pushed the eng/AX-Implement-support-for-serving-AXVisibleCharacterRange-off-the-main-thread branch from e57a6d2 to f769618 Compare June 17, 2025 21:13
@twilco twilco force-pushed the eng/AX-Implement-support-for-serving-AXVisibleCharacterRange-off-the-main-thread branch from f769618 to 814a567 Compare June 17, 2025 22:25
@twilco twilco force-pushed the eng/AX-Implement-support-for-serving-AXVisibleCharacterRange-off-the-main-thread branch from 814a567 to 116559e Compare June 17, 2025 22:55
@twilco twilco force-pushed the eng/AX-Implement-support-for-serving-AXVisibleCharacterRange-off-the-main-thread branch from 116559e to b1074f6 Compare June 17, 2025 23:08
@twilco twilco force-pushed the eng/AX-Implement-support-for-serving-AXVisibleCharacterRange-off-the-main-thread branch from b1074f6 to e53ec8b Compare June 18, 2025 01:04
@twilco twilco added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Jun 18, 2025
…n-thread

https://bugs.webkit.org/show_bug.cgi?id=294619
rdar://153652827

Reviewed by Joshua Hoffman.

With this commit, we implement support for serving AXVisibleCharacterRange off the main-thread. The high level approach is:

  1. Accessibility paints already paint all InlineIterator::TextBoxes that are visible. When this happens, keep track
     of the line indices of the painted text boxes, and associate that with a RenderText, which in turn is associated
     with an accessibility object.

  2. Cache the first painted line and last painted line index for each RenderText in a HashMap (m_mostRecentlyPaintedText),
     and sync this HashMap to the accessibility thread.

  3. Implement AXIsolatedObject::visibleCharacterRange using this HashMap. This is trivial for leaf nodes. For non-leaf
     nodes, we iterate over the descendants until we find the first painted one. The `location` of the visible NSRange
     is the text of all descendants (which are unpainted) up to the first painted one. The `length` of the visible range
     is then the length from the first painted line, to the last painted line of the last painted descendant.

This algorithm produces very different results from the main-thread visible character range implementation, which is
based on VisiblePosition line-by-line iteration to fit an objects element rect into the viewport rect. In general, the
new algorithm seems much more accurate, and this can be seen in the layout tests modified in this patch (I left code
comments in each test).

This patch also stops outputting AXVisibleCharacterRange in tests that dump every attribute for every element. Because
the algorithms produce slightly different results in ways that are not clearly relevant to any user-facing behavior,
not outputting AXVisibleCharacterRange in these tests allows us to keep the tests running in both configurations. Some
tests have also been re-written to include better test corpus text — logging "1: Lineone lineone", "2: Linetwo linetwo"
rather than the word "test" thousands of times, which is harder to debug with the --show-window test runner flag.

* LayoutTests/accessibility/image-link-expected.txt:
* LayoutTests/accessibility/image-map2-expected.txt:
* LayoutTests/accessibility/internal-link-anchors2-expected.txt:
* LayoutTests/accessibility/mac/document-attributes-expected.txt:
* LayoutTests/accessibility/mac/document-links-expected.txt:
* LayoutTests/accessibility/mac/visible-character-range-width-changes-expected.txt: Added.
* LayoutTests/accessibility/mac/visible-character-range-width-changes.html: Added.
* LayoutTests/accessibility/mixed-contenteditable-double-br-visible-character-range-hang-expected.txt:
* LayoutTests/accessibility/mixed-contenteditable-double-br-visible-character-range-hang.html:
* LayoutTests/accessibility/mixed-contenteditable-visible-character-range-hang-expected.txt:
* LayoutTests/accessibility/mixed-contenteditable-visible-character-range-hang.html:
* LayoutTests/accessibility/table-attributes-expected.txt:
* LayoutTests/accessibility/table-multiple-tbodies-expected.txt:
* LayoutTests/accessibility/table-one-cell-expected.txt:
* LayoutTests/accessibility/table-thead-tfoot-expected.txt:
* LayoutTests/accessibility/table-with-rules-expected.txt:
* LayoutTests/accessibility/transformed-element-expected.txt:
* LayoutTests/accessibility/visible-character-range-basic-expected.txt: Added.
* LayoutTests/accessibility/visible-character-range-basic.html:
* LayoutTests/accessibility/visible-character-range-height-changes-expected.txt: Added.
* LayoutTests/accessibility/visible-character-range-height-changes.html:
* LayoutTests/accessibility/visible-character-range-scrolling.html:
* LayoutTests/accessibility/visible-character-range-vertical-rl-expected.txt:
* LayoutTests/accessibility/visible-character-range-vertical-rl.html:
* LayoutTests/accessibility/visible-character-range-width-changes.html: Removed.
* LayoutTests/platform/ios/TestExpectations:
* LayoutTests/platform/ios/accessibility/mixed-contenteditable-double-br-visible-character-range-hang-expected.txt: Removed.
* LayoutTests/platform/ios/accessibility/mixed-contenteditable-visible-character-range-hang-expected.txt: Removed.
* LayoutTests/platform/ios/accessibility/visible-character-range-basic-expected.txt: Removed.
* LayoutTests/platform/ios/accessibility/visible-character-range-height-changes-expected.txt: Removed.
* LayoutTests/platform/ios/accessibility/visible-character-range-width-changes-expected.txt: Removed.
* LayoutTests/platform/mac/accessibility/lists-expected.txt:
* LayoutTests/platform/mac/accessibility/math-multiscript-attributes-expected.txt:
* LayoutTests/platform/mac/accessibility/visible-character-range-basic-expected.txt: Removed.
* LayoutTests/platform/mac/accessibility/visible-character-range-height-changes-expected.txt: Removed.
* LayoutTests/platform/mac/accessibility/visible-character-range-scrolling-expected.txt:
* LayoutTests/platform/mac/accessibility/visible-character-range-width-changes-expected.txt: Removed.
* LayoutTests/resources/accessibility-helper.js:
* Source/WebCore/accessibility/AXCoreObject.h:
(WebCore::LineRange::LineRange):
(WebCore::LineRange::debugDescription const):
* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::mostRecentlyPaintedText):
(WebCore::AXObjectCache::onAccessibilityPaintStarted):
(WebCore::AXObjectCache::onAccessibilityPaintFinished):
(WebCore::AXObjectCache::onPaint):
* Source/WebCore/accessibility/AXObjectCache.h:
* Source/WebCore/accessibility/AXTextRun.h:
(WebCore::AXTextRun::characterAt const):
(WebCore::AXTextRun::textLength const):
* Source/WebCore/accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::previousLineStartBoundaryPoints const): Deleted.
(WebCore::AccessibilityObject::lastBoundaryPointContainedInRect const): Deleted.
(WebCore::textStartPoint): Deleted.
(WebCore::textEndPoint): Deleted.
(WebCore::AccessibilityObject::boundaryPointsContainedInRect const): Deleted.
(WebCore::AccessibilityObject::visibleCharacterRange const): Deleted.
(WebCore::AccessibilityObject::visibleCharacterRangeInternal const): Deleted.
* Source/WebCore/accessibility/AccessibilityObject.h:
* Source/WebCore/accessibility/cocoa/AccessibilityObjectCocoa.mm:
(WebCore::textStartPoint):
(WebCore::textEndPoint):
(WebCore::AccessibilityObject::previousLineStartBoundaryPoints const):
(WebCore::AccessibilityObject::lastBoundaryPointContainedInRect const):
(WebCore::AccessibilityObject::boundaryPointsContainedInRect const):
(WebCore::AccessibilityObject::visibleCharacterRange const):
(WebCore::AccessibilityObject::visibleCharacterRangeInternal const):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::visibleCharacterRange const): Deleted.
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::relatedObjectIDsFor):
(WebCore::AXIsolatedTree::applyPendingChanges):
(WebCore::AXIsolatedTree::sortedNonRootWebAreasDidChange):
(WebCore::AXIsolatedTree::processQueuedNodeUpdates):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:
(WebCore::AXIsolatedTree::markMostRecentlyPaintedTextDirty):
(WebCore::AXIsolatedTree::mostRecentlyPaintedText const):
* Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm:
(WebCore::AXIsolatedObject::visibleCharacterRange const):
* Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm:
(-[WebAccessibilityObjectWrapperBase accessibilityVisibleCharacterRange]):
* Source/WebCore/dom/BoundaryPoint.cpp:
(WebCore::BoundaryPoint::debugDescription const):
* Source/WebCore/dom/BoundaryPoint.h:
* Source/WebCore/dom/SimpleRange.cpp:
(WebCore::SimpleRange::debugDescription const):
* Source/WebCore/dom/SimpleRange.h:
* Source/WebCore/page/Page.cpp:
(WebCore::Page::doAfterUpdateRendering):
* Source/WebCore/rendering/AccessibilityRegionContext.cpp:
(WebCore::AccessibilityRegionContext::~AccessibilityRegionContext):
(WebCore::AccessibilityRegionContext::takeBounds):
* Source/WebCore/rendering/AccessibilityRegionContext.h:
* Source/WebCore/rendering/TextBoxPainter.cpp:
(WebCore::TextBoxPainter::paint):
* Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:
(WTR::AccessibilityUIElement::allAttributes):

Canonical link: https://commits.webkit.org/296359@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/AX-Implement-support-for-serving-AXVisibleCharacterRange-off-the-main-thread branch from e53ec8b to 87a311b Compare June 18, 2025 04:20
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 296359@main (87a311b): https://commits.webkit.org/296359@main

Reviewed commits have been landed. Closing PR #46860 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 87a311b into WebKit:main Jun 18, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Accessibility For bugs related to accessibility.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants