Skip to content

AX: AXFrameGeometry should work for scrolling inside iframes#62325

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
hoffmanjoshua:eng/AX-FrameGeometry-should-work-for-scrolling-inside-iframes
Apr 10, 2026
Merged

AX: AXFrameGeometry should work for scrolling inside iframes#62325
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
hoffmanjoshua:eng/AX-FrameGeometry-should-work-for-scrolling-inside-iframes

Conversation

@hoffmanjoshua
Copy link
Copy Markdown
Contributor

@hoffmanjoshua hoffmanjoshua commented Apr 9, 2026

5a72101

AX: AXFrameGeometry should work for scrolling inside iframes
https://bugs.webkit.org/show_bug.cgi?id=311763
rdar://174356334

Reviewed by Tyler Wilcock.

This PR fixes AXFrameGeometry to be more accurate in more cases. Prior, we were not accurately
computing the screen position for elements that were within a scrolled iframe. This was due to
double-applying the scroll position, via using contentsToView in AXFrameGeometry + the relative
frame, both of which applied the scroll position.

Now, we have much simpler math when computing screen position:
- AXFrameGeometry.screenPosition: the is the position of that frame *including* scroll (which
means, that the position is not necessarily the top left corner of the frame, since it may
be scrolled, but rather the position of the top left of that document).
- ElementRect: this is just the position of an element (without sccroll applied)
relative to the top left corner of it's document.

By adding these together, we can correctly compute the screen position, including when it is
nested. I've added two tests to verify this behavior.

Tests: accessibility/mac/iframe-content-position-after-inner-scroll.html
       accessibility/mac/iframe-nested-scroll-position.html

* LayoutTests/accessibility/mac/iframe-content-position-after-inner-scroll-expected.txt: Added.
* LayoutTests/accessibility/mac/iframe-content-position-after-inner-scroll.html: Added.
* LayoutTests/accessibility/mac/iframe-nested-scroll-position-expected.txt: Added.
* LayoutTests/accessibility/mac/iframe-nested-scroll-position.html: Added.
* LayoutTests/accessibility/mac/iframe-position-after-div-move.html:
* LayoutTests/accessibility/mac/iframe-position-after-scroll.html:
* LayoutTests/resources/accessibility-helper.js:
* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::setFrameGeometry):
(WebCore::AXObjectCache::getAndUpdateFrameGeometry):
* Source/WebCore/accessibility/AXObjectCache.h:
* Source/WebCore/accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::convertFrameToSpace const):
* Source/WebCore/accessibility/AccessibilityScrollView.cpp:
(WebCore::AccessibilityScrollView::frameGeometry const):
* Source/WebCore/accessibility/AccessibilityScrollView.h:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::convertFrameToSpace const):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::create):
(WebCore::AXIsolatedTree::setFrameGeometry):
(WebCore::AXIsolatedTree::updateRootScreenRelativePosition):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:
(WebCore::AXIsolatedTree::frameGeometry const):
* Source/WebCore/rendering/AccessibilityRegionContext.cpp:
(WebCore::AccessibilityRegionContext::takeBoundsInternal):
(WebCore::AccessibilityRegionContext::takeBounds):
(WebCore::AccessibilityRegionContext::onPaint):
* Source/WebKit/Scripts/webkit/messages.py:
(headers_for_type):
* Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::requestFrameScreenPosition):
(WebKit::WebPageProxy::applyAccessibilityFrameScreenPosition):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::updateRemotePageAccessibilityScreenPosition):
(WebKit::WebPage::didChangeScrollOffsetForFrame):
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/WebPage.messages.in:

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

4d2831c

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe 🛠 win loading 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 🧪 win-tests ❌ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe ⏳ 🛠 vision-apple
✅ 🧪 webkitpy ✅ 🧪 ios-wk2-wpt ✅ 🧪 api-mac-debug ✅ 🛠 gtk3-libwebrtc
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
🛠 ios-safer-cpp ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress 🛠 playstation
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@hoffmanjoshua hoffmanjoshua self-assigned this Apr 9, 2026
@hoffmanjoshua hoffmanjoshua added the Accessibility For bugs related to accessibility. label Apr 9, 2026
@hoffmanjoshua hoffmanjoshua force-pushed the eng/AX-FrameGeometry-should-work-for-scrolling-inside-iframes branch from 681610b to 5b5ddbc Compare April 9, 2026 02:10
@hoffmanjoshua hoffmanjoshua marked this pull request as ready for review April 9, 2026 14:56
Copy link
Copy Markdown
Contributor

@minorninth minorninth left a comment

Choose a reason for hiding this comment

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

Could you add/update comments to the definition of FrameGeometry in AXObjectCache.h? Things to maybe define could include:

  • Origin (top-left vs bottom-left on Mac?)
  • Units (display pixels, CSS pixels)
  • Relative to?
  • Takes scroll into account?

I also wonder if that should also be named AXFrameGeometry, since it's referenced in a lot of non-AX code and I wouldn't want people confusing it with other general-purpose structs.

}
}

void WebPageProxy::applyFrameScreenPosition(FrameIdentifier frameID, const FloatRect& rootViewRect)
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.

This is an accessibility-specific function, it should maybe have accessibility in the name

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! Addressed this, and your other comment, in my latest update

@hoffmanjoshua hoffmanjoshua force-pushed the eng/AX-FrameGeometry-should-work-for-scrolling-inside-iframes branch from 5b5ddbc to 28902a6 Compare April 9, 2026 21:56
@hoffmanjoshua hoffmanjoshua changed the title AX: FrameGeometry should work for scrolling inside iframes AX: AXFrameGeometry should work for scrolling inside iframes Apr 9, 2026
@hoffmanjoshua hoffmanjoshua force-pushed the eng/AX-FrameGeometry-should-work-for-scrolling-inside-iframes branch from 28902a6 to 4d2831c Compare April 9, 2026 21:59
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 10, 2026
@hoffmanjoshua hoffmanjoshua 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 Apr 10, 2026
https://bugs.webkit.org/show_bug.cgi?id=311763
rdar://174356334

Reviewed by Tyler Wilcock.

This PR fixes AXFrameGeometry to be more accurate in more cases. Prior, we were not accurately
computing the screen position for elements that were within a scrolled iframe. This was due to
double-applying the scroll position, via using contentsToView in AXFrameGeometry + the relative
frame, both of which applied the scroll position.

Now, we have much simpler math when computing screen position:
- AXFrameGeometry.screenPosition: the is the position of that frame *including* scroll (which
means, that the position is not necessarily the top left corner of the frame, since it may
be scrolled, but rather the position of the top left of that document).
- ElementRect: this is just the position of an element (without sccroll applied)
relative to the top left corner of it's document.

By adding these together, we can correctly compute the screen position, including when it is
nested. I've added two tests to verify this behavior.

Tests: accessibility/mac/iframe-content-position-after-inner-scroll.html
       accessibility/mac/iframe-nested-scroll-position.html

* LayoutTests/accessibility/mac/iframe-content-position-after-inner-scroll-expected.txt: Added.
* LayoutTests/accessibility/mac/iframe-content-position-after-inner-scroll.html: Added.
* LayoutTests/accessibility/mac/iframe-nested-scroll-position-expected.txt: Added.
* LayoutTests/accessibility/mac/iframe-nested-scroll-position.html: Added.
* LayoutTests/accessibility/mac/iframe-position-after-div-move.html:
* LayoutTests/accessibility/mac/iframe-position-after-scroll.html:
* LayoutTests/resources/accessibility-helper.js:
* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::setFrameGeometry):
(WebCore::AXObjectCache::getAndUpdateFrameGeometry):
* Source/WebCore/accessibility/AXObjectCache.h:
* Source/WebCore/accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::convertFrameToSpace const):
* Source/WebCore/accessibility/AccessibilityScrollView.cpp:
(WebCore::AccessibilityScrollView::frameGeometry const):
* Source/WebCore/accessibility/AccessibilityScrollView.h:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::convertFrameToSpace const):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::create):
(WebCore::AXIsolatedTree::setFrameGeometry):
(WebCore::AXIsolatedTree::updateRootScreenRelativePosition):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:
(WebCore::AXIsolatedTree::frameGeometry const):
* Source/WebCore/rendering/AccessibilityRegionContext.cpp:
(WebCore::AccessibilityRegionContext::takeBoundsInternal):
(WebCore::AccessibilityRegionContext::takeBounds):
(WebCore::AccessibilityRegionContext::onPaint):
* Source/WebKit/Scripts/webkit/messages.py:
(headers_for_type):
* Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::requestFrameScreenPosition):
(WebKit::WebPageProxy::applyAccessibilityFrameScreenPosition):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::updateRemotePageAccessibilityScreenPosition):
(WebKit::WebPage::didChangeScrollOffsetForFrame):
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/WebPage.messages.in:

Canonical link: https://commits.webkit.org/310935@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/AX-FrameGeometry-should-work-for-scrolling-inside-iframes branch from 4d2831c to 5a72101 Compare April 10, 2026 16:12
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 310935@main (5a72101): https://commits.webkit.org/310935@main

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

@webkit-commit-queue webkit-commit-queue merged commit 5a72101 into WebKit:main Apr 10, 2026
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Apr 10, 2026
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