AX: Serve NSAccessibilityBoundsForRangeParameterizedAttribute and NSAccessibilityBoundsForTextMarkerRangeAttribute off the main-thread #40326

twilco · 2025-02-09T23:14:14Z

`c6bfb17`

AX: Serve NSAccessibilityBoundsForRangeParameterizedAttribute and NSAccessibilityBoundsForTextMarkerRangeAttribute off the main-thread
https://bugs.webkit.org/show_bug.cgi?id=287369
rdar://106041510

Reviewed by Chris Fleizach.

This commits implements the ability to serve NSAccessibilityBoundsForRangeParameterizedAttribute and
NSAccessibilityBoundsForTextMarkerRangeAttribute off the main-thread. We do this by measuring the size of each character
in the text run and dividing the result by the number of characters. This means we can compute a resonable frame for
any arbirtrary range of characters using this average character size.

This patch also implements conversion of rects to screen-relative space by generalizing the existing logic for doing
so in AXIsolatedObject::screenRelativePosition(). Both bounds-for-range APIs expect the returned rects to be in
screen-relative space.

In a future patch, we should implement a system that computes 100% accurate character sizes on-demand for text ranges
that ATs are actually interacting with via these APIs. So if an AT requests NSAccessibilityBoundsForTextMarkerRangeAttribute
for one range, we serve an estimate, and then compute the true per-character widths for that text and also nearby text
under the assumption that an AT is likely going to move there next. An example usecase might be line-by-line navigation,
where if one of these APIs is requested for one line, it's likely the user will continue moving down to the next lines.

This commit leaves other problems unresolved, too — specifically:
  - AXTextRuns::rect() likely does not do the right thing for vertical writing modes.
  - We probably need to special-case certain types of text-run-producing objects, like those from br elements.

We'll need to address these in later commits.

* Source/WebCore/accessibility/AXTextMarker.cpp:
(WebCore::viewportRelativeFrameFromRuns):
(WebCore::AXTextMarkerRange::viewportRelativeFrame const):
* Source/WebCore/accessibility/AXTextMarker.h:
* Source/WebCore/accessibility/AXTextRun.cpp:
(WebCore::AXTextRuns::rect const):
* Source/WebCore/accessibility/AXTextRun.h:
(WebCore::AXTextRuns::AXTextRuns):
* Source/WebCore/accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::textRuns):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:
* Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(-[WebAccessibilityObjectWrapper accessibilityAttributeValue:forParameter:]):

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

fac87ae

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

webkit-early-warning-system · 2025-02-09T23:14:21Z

EWS run on previous version of this PR (hash 0a79216)

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
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

minorninth · 2025-02-10T16:54:24Z

Source/WebCore/accessibility/AXTextMarker.h

@@ -393,6 +393,9 @@ class AXTextMarkerRange {
 #if ENABLE(AX_THREAD_TEXT_APIS)
    // Traverses from m_start to m_end, collecting all text along the way.
    String toString() const;
+    // Returns the bounds (frame) of the text in this range relative to the viewport.


What does this mean in the context of an iframe? Is it different for in-process vs for site isolation?

Good question!

In both cases, it returns a rect relative to the main, top-level viewport. We get this for free because the function relies on AXIsolatedObject::relativeFrame, which already bakes in any remoteFrameOffset() for process-isolated iframe WebContent processes.

minorninth · 2025-02-10T16:56:52Z

Source/WebCore/accessibility/AXTextRun.cpp

+                RELEASE_ASSERT(smallerOffset >= offsetOfFirstCharacterInRun);
+                // Measure the characters in this run (accomplished by smallerOffset - offsetOfFirstCharacterInRun)
+                // prior to the offset.
+                unsigned widthPriorToStart = (smallerOffset - offsetOfFirstCharacterInRun) * estimatedCharacterWidth;


Where did estimatedCharacterWidth come from?

Good point about the lack of m_ making it confusing where this comes from — will change in a follow-up commit.

Actually, when I went to make this change, I realized none of the structs in this file use m_, so it would be odd to change only one field, and changing all of them would be too large of an unrelated change for this PR IMO. Let's fix this in a follow-up.

minorninth · 2025-02-10T17:00:59Z

Source/WebCore/accessibility/AXTextRun.h

+    // APIs like AXBoundsForTextMarkerRangeAttribute quickly off the main-thread, and get more accurate
+    // sizes on-demand for runs that assistive technologies are actually requesting these APIs from.
+    static constexpr uint8_t defaultEstimatedCharacterWidth = 12;
+    uint8_t estimatedCharacterWidth { defaultEstimatedCharacterWidth };


Most other classes use m_ for member variables, would make the code above easier to read by making it clear that estimatedCharacterWidth is an object variable rather than a local

minorninth · 2025-02-10T17:01:59Z

Source/WebCore/accessibility/AXTextRun.h

@@ -138,6 +147,15 @@ struct AXTextRuns {
    }
    String substring(unsigned start, unsigned length = StringImpl::MaxLength) const;
    String toString() const { return substring(0); }
+
+    // Returns a "local" rect representing the range specified by |start| and |end|.
+    // "Local" means the rect is not relative to any coordinate space or object.


It's still relative to something, right? Could you say: in the coordinate system of the object's own bounding box, or something like that? Or, relative to the top-left of the object's bounds?

Or, relative to the top-left of the object's bounds?

This wouldn't be accurate I think, as this rect does not apply the containing object's position (that is done in a wrapping function: viewportRelativeFrameFromRuns).

I think something like "in the coordinate system of the object's own bounding box" could make sense, will change in follow-up commit!

Oh, I'm realizing when you said "Or, relative to the top-left of the object's bounds?", you probably meant the AXTextRuns object, not the accessibility object that contains the runs. I think your suggestion makes sense in that case.

Fixed in latest commit!

webkit-early-warning-system · 2025-02-11T04:56:05Z

EWS run on current version of this PR (hash fac87ae)

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

fleizach · 2025-02-11T23:25:46Z

Source/WebCore/accessibility/AXTextMarker.cpp

+
+static FloatRect viewportRelativeFrameFromRuns(Ref<AXIsolatedObject> object, unsigned offset)
+{
+    const auto* runs = object->textRuns();


any reason to put this into a variable?

Nope! Will fix in a follow-up PR.

…ccessibilityBoundsForTextMarkerRangeAttribute off the main-thread https://bugs.webkit.org/show_bug.cgi?id=287369 rdar://106041510 Reviewed by Chris Fleizach. This commits implements the ability to serve NSAccessibilityBoundsForRangeParameterizedAttribute and NSAccessibilityBoundsForTextMarkerRangeAttribute off the main-thread. We do this by measuring the size of each character in the text run and dividing the result by the number of characters. This means we can compute a resonable frame for any arbirtrary range of characters using this average character size. This patch also implements conversion of rects to screen-relative space by generalizing the existing logic for doing so in AXIsolatedObject::screenRelativePosition(). Both bounds-for-range APIs expect the returned rects to be in screen-relative space. In a future patch, we should implement a system that computes 100% accurate character sizes on-demand for text ranges that ATs are actually interacting with via these APIs. So if an AT requests NSAccessibilityBoundsForTextMarkerRangeAttribute for one range, we serve an estimate, and then compute the true per-character widths for that text and also nearby text under the assumption that an AT is likely going to move there next. An example usecase might be line-by-line navigation, where if one of these APIs is requested for one line, it's likely the user will continue moving down to the next lines. This commit leaves other problems unresolved, too — specifically: - AXTextRuns::rect() likely does not do the right thing for vertical writing modes. - We probably need to special-case certain types of text-run-producing objects, like those from br elements. We'll need to address these in later commits. * Source/WebCore/accessibility/AXTextMarker.cpp: (WebCore::viewportRelativeFrameFromRuns): (WebCore::AXTextMarkerRange::viewportRelativeFrame const): * Source/WebCore/accessibility/AXTextMarker.h: * Source/WebCore/accessibility/AXTextRun.cpp: (WebCore::AXTextRuns::rect const): * Source/WebCore/accessibility/AXTextRun.h: (WebCore::AXTextRuns::AXTextRuns): * Source/WebCore/accessibility/AccessibilityRenderObject.cpp: (WebCore::AccessibilityRenderObject::textRuns): * Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h: * Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm: (-[WebAccessibilityObjectWrapper accessibilityAttributeValue:forParameter:]): Canonical link: https://commits.webkit.org/290242@main

webkit-commit-queue · 2025-02-12T00:37:45Z

Committed 290242@main (c6bfb17): https://commits.webkit.org/290242@main

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

twilco self-assigned this Feb 9, 2025

twilco added the Accessibility For bugs related to accessibility. label Feb 9, 2025

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 10, 2025

minorninth approved these changes Feb 10, 2025

View reviewed changes

twilco force-pushed the eng/AX-Serve-NSAccessibilityBoundsForRangeParameterizedAttribute-and-NSAccessibilityBoundsForTextMarkerRangeAttribute-off-the-main-thread branch from 0a79216 to fac87ae Compare February 11, 2025 04:55

fleizach reviewed Feb 11, 2025

View reviewed changes

fleizach approved these changes Feb 11, 2025

View reviewed changes

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 Feb 12, 2025

webkit-commit-queue force-pushed the eng/AX-Serve-NSAccessibilityBoundsForRangeParameterizedAttribute-and-NSAccessibilityBoundsForTextMarkerRangeAttribute-off-the-main-thread branch from fac87ae to c6bfb17 Compare February 12, 2025 00:37

webkit-commit-queue merged commit c6bfb17 into WebKit:main Feb 12, 2025

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 12, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AX: Serve NSAccessibilityBoundsForRangeParameterizedAttribute and NSAccessibilityBoundsForTextMarkerRangeAttribute off the main-thread #40326

AX: Serve NSAccessibilityBoundsForRangeParameterizedAttribute and NSAccessibilityBoundsForTextMarkerRangeAttribute off the main-thread #40326

twilco commented Feb 9, 2025 •

edited by webkit-early-warning-system

Loading

webkit-early-warning-system commented Feb 9, 2025 •

edited

Loading

minorninth Feb 10, 2025

twilco Feb 11, 2025

minorninth Feb 10, 2025

twilco Feb 11, 2025

twilco Feb 11, 2025

minorninth Feb 10, 2025

minorninth Feb 10, 2025

twilco Feb 11, 2025 •

edited

Loading

twilco Feb 11, 2025

twilco Feb 11, 2025

webkit-early-warning-system commented Feb 11, 2025 •

edited

Loading

fleizach Feb 11, 2025

twilco Feb 12, 2025

webkit-commit-queue commented Feb 12, 2025

AX: Serve NSAccessibilityBoundsForRangeParameterizedAttribute and NSAccessibilityBoundsForTextMarkerRangeAttribute off the main-thread #40326

AX: Serve NSAccessibilityBoundsForRangeParameterizedAttribute and NSAccessibilityBoundsForTextMarkerRangeAttribute off the main-thread #40326

Conversation

twilco commented Feb 9, 2025 • edited by webkit-early-warning-system Loading

webkit-early-warning-system commented Feb 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

twilco Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webkit-commit-queue commented Feb 12, 2025

twilco commented Feb 9, 2025 •

edited by webkit-early-warning-system

Loading

webkit-early-warning-system commented Feb 9, 2025 •

edited

Loading

twilco Feb 11, 2025 •

edited

Loading

webkit-early-warning-system commented Feb 11, 2025 •

edited

Loading