1753836 - MouseEvent.screenX/Y coordinate space is weird.

Status: RESOLVED FIXED

(Core :: DOM: Events, defect, P3)

Description

[Emilio Cobos Álvarez (:emilio)]

It converts device pixels to CSS-but-without-zoom pixels. This is a rather odd coordinate space, which causes issues like bug 1741830.

We should probably use CSS pixels consistently (in terms of stuff being web observable), but that probably involves fixing callers that pass screenX/screenY from the child to the parent process to either convert to global screen coordinates, or at least to undo the full zoom conversion.

Then nsIScreenManager could be a bit more explicit about stuff being on global coordinates, to avoid stuff like bug 1741830, but not sure how that would work...

I've seen some related bugs (see the see also field).

Comment 1

[Masatoshi Kimura [:emk]]

How do other browsers behave? We can't change them just for our own convenience.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Sure, I agree. But at least we should be internally consistent. screen.height / screen.width / etc account for zoom, while screenX doesn't.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Testcase

The current behavior of not accounting for zoom clearly makes no sense. On a zoomed page I can trivially get screenX/screenY values outside of screen dimensions.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Also printing dpr.

So stuff here is a mess. Chrome changes DPR on zoom like we do but doesn't adjust screen coordinates. But unzoomed does expose CSS pixels.

Safari doesn't change DPR not screen coords on zoom, but does change outerWidth and inner window coords.

So I think making event.screenX/Y return CSS coords consistently is better (but then we need to expose a way of converting to desktop pixels to chrome JS somehow). That said, that's clearly a pre-existing issue, see bug 1741830 comment 21.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: WIP: Bug 1753836 - Make Event.screenX/screenY return page CSS coordinates.

Depends on D138021

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1753836 - Fix AutoScroll code to deal with Event.screen* returning page CSS coordinates. r=Gijs,mconley

Depends on D138024

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1753836 - Make Event.screenX/screenY return page CSS coordinates. r=smaug!,jfkthame!

We report window.screen coordinates in CSS space, so it makes sense to do
the same for screen-relative offsets. This requires some front-end fixes
incoming in following patches.

Depends on D138021

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1753836 - Fix AutoScroll code to deal with Event.screen* returning page CSS coordinates. r=Gijs,mconley

If the page is zoomed, its devicePixelRatio will differ from the browser
chrome's. Account for this by converting to device pixels before starting to
scroll.

Depends on D138035

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1753836 - Fix context menus to deal with Event.screen* returning page CSS coordinates. r=Gijs,mconley

If the page is zoomed, its devicePixelRatio will differ from the browser
chrome's. Account for this by converting to device pixels before starting to
scroll.

Depends on D138036

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1753836 - Document why ContentDelegateChild doesn't need changes due to the change above. r=agi

Android has no full-page zoom, so we are not meaningfully changing behavior.

However, these values are exposed to the GeckoView JS api, surprisingly, yet
viewport scaling and so on can change the CSS to device pixel ratio...

Shouldn't we expose device pixels there instead? Or do the api consumers assume
that those CSS pixels are scaled to the device scale factor somehow (and that's
not working)?

Depends on D138037

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1753836 - Fix callers of screenForRect to pass desktop pixels. r=emk,jfkthame,Gijs

Device pixels and desktop pixels are not the same on macOS and Win7.
Expose the desktop-to-device scale to JS and use it appropriately.

Depends on D138038

Comment 12

[Masatoshi Kimura [:emk]]

Attachment: Test file to show screen coordinates

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1753836 - Make window screen and event screen coordinates consistent. r=jfkthame,emk

This basically undoes bug 1246346. The current behavior is pretty bizarre,
the screenX origin / position doesn't match the mouse event coordinates,
because on windows we return device pixels rather than CSS pixels for the
window coordinates.

This makes behavior consistent with how other browsers report these coordinates
at least on Windows in non-mixed DPI mode, and I think is fine.

In mixed DPI mode, there might indeed be overlapping coordinates, but again I
think that's fine, because the CSS coordinate space of the different monitors
is different. You need to multiply by the devicePixelRatio if you want
coordinates not to overlap.

Depends on D138039

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/db443ce0406d
Make Event.screenX/screenY return page CSS coordinates. r=smaug,jfkthame
https://hg.mozilla.org/integration/autoland/rev/9209832ecffe
Fix AutoScroll code to deal with Event.screen* returning page CSS coordinates. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/faa8a8f2d40f
Fix context menus to deal with Event.screen* returning page CSS coordinates. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/774af76be463
Document why ContentDelegateChild doesn't need changes due to the change above. r=agi
https://hg.mozilla.org/integration/autoland/rev/371a495ff728
Fix callers of screenForRect to pass desktop pixels. r=jfkthame,Gijs
https://hg.mozilla.org/integration/autoland/rev/a572cbc0fac2
Make window screen and event screen coordinates consistent. r=jfkthame

Comment 15

[Sandor Molnar[:smolnar]]

Backed out 6 changesets (bug 1753836) for causing mochitest failures in test_event_screenXY_with_zoom

Backout link: https://hg.mozilla.org/integration/autoland/rev/70b17d77834692747e860c50dad2d9dd0d8c1ff2

Push with failures

Failure log

WARNING -  TEST-UNEXPECTED-FAIL | dom/events/test/test_event_screenXY_with_zoom.html | Test timed out. -
[task 2022-02-15T23:06:49.099Z] 23:06:49  WARNING -  TEST-UNEXPECTED-FAIL | dom/events/test/test_event_screenXY_with_zoom.html | [SimpleTest.finish()] No checks actually run. (You need to call ok(), is(), or similar functions at least once.  Make sure you use SimpleTest.waitForExplicitFinish() if you need it.)
[task 2022-02-15T23:06:49.100Z] 23:06:49     INFO -      SimpleTest.ok@SimpleTest/SimpleTest.js:417:16
[task 2022-02-15T23:06:49.100Z] 23:06:49     INFO -      afterCleanup@SimpleTest/SimpleTest.js:1416:18
[task 2022-02-15T23:06:49.100Z] 23:06:49     INFO -      executeCleanupFunction@SimpleTest/SimpleTest.js:1481:7
[task 2022-02-15T23:06:49.100Z] 23:06:49     INFO -      SimpleTest.finish@SimpleTest/SimpleTest.js:1501:3
[task 2022-02-15T23:06:49.100Z] 23:06:49     INFO -      killTest@SimpleTest/TestRunner.js:194:22

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c6094f3a20bd
Make Event.screenX/screenY return page CSS coordinates. r=smaug,jfkthame
https://hg.mozilla.org/integration/autoland/rev/893602e4d374
Fix AutoScroll code to deal with Event.screen* returning page CSS coordinates. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/b5a79abda144
Fix context menus to deal with Event.screen* returning page CSS coordinates. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/52840794d1ab
Document why ContentDelegateChild doesn't need changes due to the change above. r=agi
https://hg.mozilla.org/integration/autoland/rev/d1efe8bdd1af
Fix callers of screenForRect to pass desktop pixels. r=jfkthame,Gijs
https://hg.mozilla.org/integration/autoland/rev/808f848af710
Make window screen and event screen coordinates consistent. r=jfkthame

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d0345eaae543
Apparently we access layout.css.dpi a bit less.

Comment 18

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/c6094f3a20bd
https://hg.mozilla.org/mozilla-central/rev/893602e4d374
https://hg.mozilla.org/mozilla-central/rev/b5a79abda144
https://hg.mozilla.org/mozilla-central/rev/52840794d1ab
https://hg.mozilla.org/mozilla-central/rev/d1efe8bdd1af
https://hg.mozilla.org/mozilla-central/rev/808f848af710
https://hg.mozilla.org/mozilla-central/rev/d0345eaae543