1885106 - Enhance "LayoutUtils.sys.mjs" to allow the transformation of local coordinates to the browser window coordinates in CSS units

Status: RESOLVED FIXED

(Core :: Layout, enhancement)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

With the currently existing methods in LayoutUtils.sys.mjs any local rect or an elements bounding rect can be translated to screen coordinates for being used with native events, it misses methods to do the same but for window coordinates. Those would be important to have for widget level events, as what we would need for our WebDriver implementation (bug 1773393).

In our case events will not be dispatched eg. via nsIDOMWindowUtils.sendNativeMouseEvent but nsIDOMWindowUtils.sendMouseEvent and the latter needs coordinates relative to the window.

Comment 1

[Cieara Meador [:cmkm]]

Moving this to Core::Layout, please feel free to move if this is misplaced.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

I am assuming window means the browser window.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Yes, thanks for the correction!

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1885106 - Implement LayoutUtils.rectToTopLevelWidgetRect. r?#layout-reviewers

Comment 5

[Hiroyuki Ikezoe (:hiro)]

I haven't tested D211131 at all, I hope it works. A key point is that to convert into the browser window coods we don't need to this screen offset, so D211131 just introduced another variant of the coordinates conversion function without the offset.

My memory about nsIWidget/PuppetWidget metrics is quite stale, so there's probably something I am missing.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Henrik, I did a quick test on a new API named nsIDOMWindowUtils.toBrowserCoords in D211131, the result looks reasonable. So I wonder whether D211131 is sufficient for your purpose?

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hiro, I could certainly try out the patch. Would you mind creating at least a build for MacOS on try that I can use and won't have to build locally? Thanks.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Here it is; https://treeherder.mozilla.org/jobs?repo=try&revision=2cd5cd46b0c4b5bacaaaa832b840a6d969328d9d . Thanks!

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I had a first look after rebasing my patch and fixing the merge conflicts. Basically the method seems to work fine for different device pixel ratios.

Here something I noticed:

• It does not calculate the values based on CSS pixels but raw pixels. When I check for example window.screenX I get CSS pixels instead. Given that in WebDriver we basically operate on CSS pixels, should we have a method like toBrowserRectInCSSUnits()? If yes, would that work on all platforms?

• It already takes the browser UX into account which is great so that we can be sure to not leave the restricted section of the browser window. When Marionette will operate in chrome scope I assume that we can directly use the client rect for elements (not tested yet)?

Thanks!

Comment 10

[Hiroyuki Ikezoe (:hiro)]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #9)

I had a first look after rebasing my patch and fixing the merge conflicts. Basically the method seems to work fine for different device pixel ratios.

Here something I noticed:

• It does not calculate the values based on CSS pixels but raw pixels. When I check for example window.screenX I get CSS pixels instead. Given that in WebDriver we basically operate on CSS pixels, should we have a method like toBrowserRectInCSSUnits()? If yes, would that work on all platforms?

I'd agree with this comment. The conversion to CSS units can be done in the parent process (Though I am not sure, we don't need the conversion at all? Given that this TransformChildToParent which is used to convert native event coords into the browser one doesn't return CSS units)

• It already takes the browser UX into account which is great so that we can be sure to not leave the restricted section of the browser window. When Marionette will operate in chrome scope I assume that we can directly use the client rect for elements (not tested yet)?

You mean that if the function, LayoutUtils.rectToBrowserRect gets called in a document in the parent process? It should work since the function uses this _rectToClientRect which looks trying to traverse ancestor documents.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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

I'd agree with this comment. The conversion to CSS units can be done in the parent process (Though I am not sure, we don't need the conversion at all? Given that this TransformChildToParent which is used to convert native event coords into the browser one doesn't return CSS units)

The question is more about EventUtils here. Is it supporting non CSS coordinates in some way? So far we only used CSS pixels for synthesizing our events in the content process. If not should we transition away from that extra module and use DOMWindowUtils directly?

• It already takes the browser UX into account which is great so that we can be sure to not leave the restricted section of the browser window. When Marionette will operate in chrome scope I assume that we can directly use the client rect for elements (not tested yet)?

You mean that if the function, LayoutUtils.rectToBrowserRect gets called in a document in the parent process? It should work since the function uses this _rectToClientRect which looks trying to traverse ancestor documents.

Then there are actually two questions:

1. When I search for lazy.LayoutUtils. I can only see usage from child actors. So I assume it cannot be used from the parent process. I would have to test it but should be most likely independent from this bug.

2. When Marionette is in chrome scope mode it should be possible for tests to interact with elements (like the location bar) from the Firefox UI.

Comment 12

[Hiroyuki Ikezoe (:hiro)]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #11)

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

I'd agree with this comment. The conversion to CSS units can be done in the parent process (Though I am not sure, we don't need the conversion at all? Given that this TransformChildToParent which is used to convert native event coords into the browser one doesn't return CSS units)

The question is more about EventUtils here. Is it supporting non CSS coordinates in some way? So far we only used CSS pixels for synthesizing our events in the content process. If not should we transition away from that extra module and use DOMWindowUtils directly?

Even if EventUtils functions doesn't accept values in CSS units directly, you can just convert values into CSS units in browser coords, then invoke EventUtils functions with the converted value.

• It already takes the browser UX into account which is great so that we can be sure to not leave the restricted section of the browser window. When Marionette will operate in chrome scope I assume that we can directly use the client rect for elements (not tested yet)?

You mean that if the function, LayoutUtils.rectToBrowserRect gets called in a document in the parent process? It should work since the function uses this _rectToClientRect which looks trying to traverse ancestor documents.

Then there are actually two questions:

1. When I search for lazy.LayoutUtils. I can only see usage from child actors. So I assume it cannot be used from the parent process. I would have to test it but should be most likely independent from this bug.

If you want to use the new LayoutUtils.rectToBrowserRect directly in the parent process, that's not the use case of the function unfortunately. I supposed that "Enhance LayoutUtils.sys.mjs" means you want to use it in a child process.

If you want to do something in the parent process, what you'd probably need is that

1. Convert values with LayoutUtils._rectToClientRect so that the converted value becomes in each content process root coords (e.g. top level document coords, or each out-of-process root document)
2. Pass the converted values to the parent process
3. Convert the passed values into browser coords.

That's what I can tell right now.

2. When Marionette is in chrome scope mode it should be possible for tests to interact with elements (like the location bar) from the Firefox UI.

I think so, yes. If you use the new LayoutUtils.rectToBrowserRect for documents in the parent process (including the browser document), it should work.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Hiro. I think that you can continue with the patch. Anything else that might come up during the implementation in Marionette I can file as a follow-up bug. Having this method included will help a lot so that we don't have to do a full build.

Comment 14

[Hiroyuki Ikezoe (:hiro)]

Attachment: WIP: Bug 1885106 - [WebDriver] Dispatch wheel events in the parent process.

Comment 15

[Hiroyuki Ikezoe (:hiro)]

I uploaded D213182 which is a very rough and hacky implementation to make wheel event dispatching in the browser parent process. (Though I just confirmed it by running a single test, wheel-scrolling.html)

Anyways, as you can see in the patch, I think the work we need here is something in DOM event handling and WebDriver itself. So I think it would be better that someone in DOM or WebDriver team works on this bug.

Some notes I realized;

My understandings how dispatching a wheel event works currently is;

1. A wheel scroll event is initiated in a content process
2. It is propagated to the browser parent process
3. It is propagated back into the content process
4. It synthesizes a wheel event in the content process

And what I did in D213182 is at 4) just converting the given coordinates in CSS units into LayoutDevicePixel units (i.e. factoring into full zoom ratio and DPI), then return the converted coordinates back to the browser parent process, then synthesizing a wheel event with the converted coordinates in the browser parent process.

BUT, from what I can tell, I think we should do the things in between 1) and 2) and 3), thus we don't need to propagate information back into the content process. But I couldn't find a way to do it there.

ALSO, this is the most confusing thing, the wheel-scrolling.html just invokes scroll() just once, but it also involves a Pause action. I am not surprised by the involved Pause action, I am surprised by what the Pause action tries to do. It does try to kinda setTimeout. So if there's an action sequence such as pause: 16ms and wheel scroll, then it needs to be interpreted to await 16ms; synthesize a wheel event in the content process. I wonder how we can do this kind of things with synthesizing events in the parent process. I have no idea.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hiro, thanks for your work. Neverless I want to mention that this bug is just about the LayoutUtils module and the feature to calculate the browser window coordinates. It's not about emitting events via Marionette through the parent process. As such all what we need for now is your initial WIP with a basic test (if that exists for the other helpers in this module as well) without any event handling. Everything related to WebDriver (Marionette) will take place in dependent bugs of our meta bug 1773393, and follow-up bugs will clearly be filed when we hit road blocks.

So could we just get the feature that I tested recently finished and landed? That would be fantastic.

Comment 17

[Hiroyuki Ikezoe (:hiro)]

Yeah, I understand the situation, thus I concluded it's not something in Layout, rather it's in DOM.

So you are going to use LayoutUtils.rectToBrowserRect in D211131 in content processes, or in the parent process? Anyways, what I can tell is converting the given coordinates in the root document in a content process into the browser coordinates can be done both in the parent process and the content process. The way in D211131 does the conversion in the content process. The way in D213182 does the conversion in the browser parent process (it's done behind the RecvDispatchWheelEvent call in CanonicalBrowsingContext::SendWheelEvent). The latter is much much clearer than the former I believe.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I would use it where-ever it is best. Note that I still have to call into the content process when we need element coordinates. So in most situations it shouldn't make a difference. But when working with viewport offsets working directly from the parent process would be good! I would appreciate saving extra time in not having to do IPC on our own. But note that it would be also helpful to not bind it too tight to DOM events given that in some situations we might be interested in just the coordinates.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3bf8ce9974ed
Implement LayoutUtils.rectToTopLevelWidgetRect. r=layout-reviewers,emilio

Comment 20

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/3bf8ce9974ed