User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

1. Set apz.allow_zooming=true
2. Open any page with text and pinch to zoom in with touchpad
3. Try to use "Look Up" on a word by moving the mouse cursor to that word and force-clicking on touchpad or pressing the Cmd-Ctrl-D shortcut

Actual results:

The "Look Up" window does not appear at all or opens with a different word, which seems to be the one on that position before zooming.

Expected results:

The "Look Up" window should show up for the word that's under the mouse cursor when the page at the current zoom level.

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

We should re-test this once bug 1556556 lands, as that may fix it.

Kats, would you be able to re-test this on a Mac with bug 1556556 in the tree?

Yeah, it's still happening.

Actually, I should clarify: the word that gets selected is the right one - it's the one under the cursor post-zoom. However the popup and word highlight appears in the wrong place. So this might actually get fixed by one of the other bugs about popup windows spawning in the wrong place.

This code may or may not be relevant: https://searchfox.org/mozilla-central/rev/3d39d3b7dd1b2be30692d4541ea681614e34c786/toolkit/actors/ControllersParent.jsm#69-85 (I just came across it during unrelated investigations)

That code seems okay, it's dealing in visual coords in the parent process. This code seems closer to the problem

https://searchfox.org/mozilla-central/rev/3d39d3b7dd1b2be30692d4541ea681614e34c786/dom/base/nsGlobalWindowCommands.cpp#1009-1029

This is a minimal patch that fixes this bug for me. There is probably a lot of other coord issues like this in this file (dom/events/ContentEventHandler.cpp), those issues might affect things like IME or whatever else functions in this file are used for.

If anyone else wants this feel free to take it.

(In reply to Timothy Nikkel (:tnikkel) from comment #8)

Created attachment 9158388 [details] [diff] [review]
fixlookup

This is a minimal patch that fixes this bug for me. There is probably a lot of other coord issues like this in this file (dom/events/ContentEventHandler.cpp), those issues might affect things like IME or whatever else functions in this file are used for.

Yeah, the coordinates are used to specify position of IME own windows. In other words, if current ContentEventHandler is not APZ-aware, IME's window is shown at wrong position too.

This might cause something wrong on Android if same zooming code is used.

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #11)

This might cause something wrong on Android if same zooming code is used.

Yes. https://bugzilla.mozilla.org/show_bug.cgi?id=1597897

Masayuki sent me some instructions on how to exercise different codepaths in ContentEventHandler.cpp and indeed there other things that will need updating for APZ zoom. i.e. the patch on this bug doesn't fix all the problems. I'll spin off separate bugs with STRs.

No functional changes, just some cleanup.

Depends on D86982

After the patches in this bug and the one on bug 1658927, there's still a conversion in QueryContentRect that looks like it should have this LayoutToVisual conversion, but it's not clear to me what user-visible effect that might have. :masayuki if you know, please let me know so I can test that.

https://hg.mozilla.org/mozilla-central/rev/ff0cfe6047ed
https://hg.mozilla.org/mozilla-central/rev/b6c0086812a8