976772 - [e10s] Widget can appear at the wrong place with iframes

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: coordinates

For bug 938359, I was messing around with positioning and I discovered that our existing code for the context menu and autocomplete doesn't handle frames correctly.

This first patch addresses the issue with the context menu. The main problem is that event.client{X,Y} returns coordinates relative to the innermost frame. I decided to use event.screen{X,Y} instead and to pass those values to openPopupAtScreen. Doing it this way seems more compatible with our API than trying to anchor everything to the <browser> element. It also deals correctly with chrome zooming.

I also discovered that event.screen{X,Y} gives different values depending on whether we're in-process or out-of-process, so I added a new method to browser.xml to normalize out the difference. The comment explains the issue.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: autocomplete

There's a related problem with autocomplete. It uses getBoundingClientRect(), which also returns coordinates relative to the innermost frame. These coordinates also aren't adjusted to account for zooming.

This patch just does a little arithmetic to adjust everything. I've tested it with both chrome and content zooming as well as frames and it seems to work.

Comment 2

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8381706 [details] [diff] [review]
autocomplete

Review of attachment 8381706 [details] [diff] [review]:
-----------------------------------------------------------------

I had done something similar for the <select> popups code, in a slightly different manner (based on some Metro code IIRC). 
https://hg.mozilla.org/mozilla-central/rev/56c09de8d0f6#l8.49
But it still doesn't account for zoom level, and your implementation is better (so with window.mozInnerScreenX we don't need to iterate through nested frames?)
It'd be nice to unify the two and have your impl somewhere that the <select> code can access, and presumably other features too which will need it.

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: autocomplete v2

This version is more comprehensive. It moves the getBoundingClientRect stuff to a JSM. I also switched a few things to use openPopupAtScreen, since I now remember from the tooltip bug that that function works with RTL text and openPopup doesn't.

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e4139eb421ff
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a822b2b8810

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e4139eb421ff
https://hg.mozilla.org/mozilla-central/rev/9a822b2b8810

Comment 6

[Catalin Varga [QA][:VarCat]]

Hi, do you think you can provide us with a test page in order to manually verify this bug?

Comment 7

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

That doesn't seem worth the effort.