Closed Bug 1498352 Opened 3 years ago Closed 3 years ago
Content Utils::To Widget Point can return incorrect coordinates in content process
46 bytes, text/x-phabricator-request
|Details | Review|
`nsContentUtils::ToWidgetPoint` uses the APZ resolution from `nsLayoutUtils::GetCurrentAPZResolutionScale` when converting to widget coordinates. However, `nsLayoutUtils::GetCurrentAPZResolutionScale` doesn't take into account the resolution of the root pres shell. In a content process on mobile, the content document resolution _is_ the resolution of the root pres shell. Therefore `nsLayoutUtils::GetCurrentAPZResolutionScale` can return the wrong result if the document resolution is different than 1.
`nsContentUtils::ToWidgetPoint` uses the current APZ resolution to scale to layout device coordinates. However, that APZ resolution does not take into account the root pres-shell's resolution. In a content process, the root pres-shell's resolution represents the content document resolution. Therefore, `nsContentUtils::ToWidgetPoint` fails to work correctly in a content process when the content document has a resolution that's different than one. This patch changes the APZ resolution to include the root pres shell resolution to resolve the issue.
How was this manifesting? i.e. is there a user-observable issue that this fixes? And is it possible to make a test case for it? Off the top of my head I agree with Botond's comment on the review that this is probably intentional. Getting more understanding for the motivation behind this change will help us understand if this is the right fix.
We're migrating tests on mobile from non-e10s to e10s. Some existing tests (e.g. "dom/animation/test/mozilla/test_restyles.html") fail when running under e10s on mobile because 1) the tests call `nsContentUtils::ToWidgetPoint` indirectly by synthesizing mouse events from the content process, and 2) unlike desktop, the test documents have a different resolution than one on mobile. So I guess these existing, failing tests are the test cases for this issue.
I think that on non-e10s the root prescontext at  is the chrome prescontext, but in e10s in the content process it's the content prescontext. That might explain the difference in behaviour. But I'll need to spend more time thinking about this before I'm comfortable r+'ing it.
3 years ago
Priority: -- → P3
Because the root resolution is conceptually at the parent/child process boundary, when sending mouse events from child to the parent, we need to apply that resolution to the mouse coordinates.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/084f6e60778f Apply root resolution when sending mouse events from child to parent; r=kats
You need to log in before you can comment on or make changes to this bug.