1602987 - Fix Accessible::BoundsInAppUnits for OOP iframes

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect, P2)

Description

[James Teh [:Jamie]]

Accessibility tools such as screen readers and magnifiers need screen coordinates for mouse routing, visual highlighting, etc. In OOP iframes, these coordinates are currently relative to the document, not absolute screen coordinates.

OS specific a11y code calls Accessible::Bounds, which calls Accessible::BoundsInAppUnits, which finally calls nsFrame::GetScreenRectInAppUnits. As noted in bug 1550800, nsFrame::GetScreenRectInAppUnits doesn't return screen coordinates for OOP iframes. As per bug 1560627 comment 2, there won't be a layout API to do this from the content process; it is expected that this coordinate mapping should be done in the parent process.

For platforms other than Windows, we should be able to do this in the parent process, since clients always talk to the parent process there. We'll need to do this somewhere in ProxyAccessible.

On Windows, things are much more complicated, since clients talk directly to the content process. We do have AccessibleHandler which intercepts calls for in-process clients which inject into our parent process, but we have no way of intercepting calls for out-of-process clients. We can't make sync IPC calls from content to parent for each Bounds query because this would be too slow and would cause deadlocks. So, we need some way to push required info down to content processes. We need to consider both coordinates and transforms. While the former might be possible, I have no idea how to do the latter.

We can send this info down via PDocAccessible. We already have a framework for scrolling events implemented for Android. So, we can probably update this info whenever a scrollingEnd event is fired. That means there will be a short period where the data is stale after scrolling, but I honestly don't know if there's a better option.

Comment 1

[James Teh [:Jamie]]

:jwatt, can you point me in the right direction here?

1. In the parent process, how do I map screen coords to coords suitable for an OOP iframe document?
2. For Windows, what can I send down from parent to content to allow content to figure out screen coords itself?
   • I imagine I'll need some sort of offset.
   • Is it even possible to handle transforms like this? For example, can I send down some sort of matrix I can apply on the content side? And if so, where might I get that from?

Comment 2

[Jonathan Watt [:jwatt]]

Ryan, did you implement something that would be useful for this? I thought you did, but looking at BrowsingContext.webidl and other files I'm not seeing anything.

Comment 3

[Ryan Hunt [:rhunt]]

If what you need is purely visible, then you may be able to hack it onto EffectsInfo. This struct is computed during painting and contains the visible rect and scaling information for a frame. [1] It currently doesn't contain the transform to the screen however. Everything is relative to the root document in the OOP browser, so you'd need to extend it. It might be easier to just add a new mechanism to BrowsingContext.

[1] https://searchfox.org/mozilla-central/rev/e878e5b81bb319c141900ce9cfcde732df5c8449/dom/ipc/EffectsInfo.h#40

Comment 4

[Hiroyuki Ikezoe (:hiro)]

FWIW, you can convert nsRect in OOP iframe to the rect in browser window coordinate system with BrowserChild::GetChildToParentConversionMatrix(). See here. So to me a rest of works is to obtain the offset of the browser window on screen.

Comment 5

[James Teh [:Jamie]]

To clarify, for any coords we get using methods like nsFrame::GetScreenRectInAppUnits, we would transform them to window coords using the matrix from BrowserChild::GetChildToParentConversionMatrix, and then add the window offset to make them absolute screen coords? That makes sense.

I've dug into this a bit further and realised a few more things:

1. We use nsFrame::GetScreenRectInAppUnits in quite a few other places. We'll need to tweak these as well.
2. We have nsCoreUtils::GetScreenCoordsForWindow, which doesn't use nsFrame::GetScreenRectInAppUnits but does need to work (and probably doesn't work now?). This uses nsIBaseWindow::GetPosition. I'm guessing that'll be the same as the window screen offset discussed above.
3. Accessible::ChildAtPoint needs to convert from absolute screen coords to OOP iframe coords (the reverse). I'm guessing we can't use the child to parent matrix to do this; we'd need a parent to child matrix? Or can we use the same matrix for both?

Comment 6

[Hiroyuki Ikezoe (:hiro)]

(In reply to James Teh [:Jamie] from comment #5)

To clarify, for any coords we get using methods like nsFrame::GetScreenRectInAppUnits, we would transform them to window coords using the matrix from BrowserChild::GetChildToParentConversionMatrix, and then add the window offset to make them absolute screen coords? That makes sense.

To clarify this clarification. for any coords in OOP iframe. Just in case. :)

1. We use nsFrame::GetScreenRectInAppUnits in quite a few other places. We'll need to tweak these as well.

I think we can make GetScreenRectInAppUnits work for nsIFrame in OOP iframe (which is bug 1550800 I assume)

2. We have nsCoreUtils::GetScreenCoordsForWindow, which doesn't use nsFrame::GetScreenRectInAppUnits but does need to work (and probably doesn't work now?). This uses nsIBaseWindow::GetPosition. I'm guessing that'll be the same as the window screen offset discussed above.

Looks like nsCoreUtils::GetScreenCoordsForWindow doesn't work as of now in OOP iframes. That's said, as per the comment for nsIBaseWindow::GetWindow, the comment says "This is relatie to the parent window" (I guess relatie means relative), so we don't need any tweaks there?

3. Accessible::ChildAtPoint needs to convert from absolute screen coords to OOP iframe coords (the reverse). I'm guessing we can't use the child to parent matrix to do this; we'd need a parent to child matrix? Or can we use the same matrix for both?

You can probably use the inverse of the matrix. A patch I am currently working can be a reference. See BrowserChild::GetRemoteDocumentRectRelativeToSelf() in https://phabricator.services.mozilla.com/D61938. (The function name might be changed, it's still in review)

Comment 7

[James Teh [:Jamie]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #6)

(In reply to James Teh [:Jamie] from comment #5)

To clarify, for any coords we get using methods like nsFrame::GetScreenRectInAppUnits, we would transform them to window coords using the matrix from BrowserChild::GetChildToParentConversionMatrix, and then add the window offset to make them absolute screen coords? That makes sense.

To clarify this clarification. for any coords in OOP iframe. Just in case. :)

Right... which might be interesting because we can't just use XRE_IsContentProcess, since that will also be true for tab documents.

I think we can make GetScreenRectInAppUnits work for nsIFrame in OOP iframe (which is bug 1550800 I assume)

See also bug 1560627. It's a bit confusing. :)

Looks like nsCoreUtils::GetScreenCoordsForWindow doesn't work as of now in OOP iframes. That's said, as per the comment for nsIBaseWindow::GetWindow, the comment says "This is relatie to the parent window" (I guess relatie means relative), so we don't need any tweaks there?

I guess that depends how you define parent window in an OOP context. For tab documents, this currently works, even though the parent window is actually in the parent process in that case. That said, it might just be easier to fix this in the a11y function, since we always explicitly want the screen coords there.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

FWIW, now nsFrame::GetScreenRectInAppUnits works properly in OOP iframes.

Comment 9

[James Teh [:Jamie]]

Manual testing suggests that both bounds and hit testing are now working as expected. I wasn't expecting hit testing to work, but I guess because bug 1550800 fixed PuppetWidget instead of just nsFrame::GetScreenRectInAppUnits, the other calls we're making are using the correct coordinate space too. That's totally awesome; thanks! :)

I suspect we'll still need to fix nsCoreUtils::GetScreenCoordsForWindow, since it uses nsIBaseWindow::GetPosition. I'll look into that and file a separate bug as needed.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

\o/