936567 - Context menus don't position right when content is zoomed

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Browser, defect)

Description

[Jim Mathies [:jimm]]

str:

1) zoom content with a drop down in it
2) tab the drop down

result: context menu is positioned incorrectly

Comment 1

[Kamil Jozwiak [:kjozwiak]]

This is also happening with the context menu's when accessing text boxes.

- Attached a screenshot to make it easier for QA to see what the original issue was when testing

Was using the following build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-11-10-03-02-05-mozilla-central/

Comment 2

[Kamil Jozwiak [:kjozwiak]]

Attachment: aligned incorrectly when zoomed

Comment 3

[Jim Mathies [:jimm]]

What we'll need here is the reverse of TransformCoordinateToGecko, a method content can call to transform a dom point to a screen point so that we can position front end chrome (which isn't affected by zoom) at the proper location. Maybe we can create a new apzc utils interface down in widget for this.

http://mxr.mozilla.org/mozilla-central/source/gfx/layers/composite/APZCTreeManager.h#133

Botond, any chance you could help us out with this? I'm not an expert on matrix math. You recently reworked all of this code so curious if you might be able to help us out.

Comment 4

[Botond Ballo [:botond]]

(In reply to Jim Mathies [:jimm] from comment #3)
> What we'll need here is the reverse of TransformCoordinateToGecko, a method
> content can call to transform a dom point to a screen point so that we can
> position front end chrome (which isn't affected by zoom) at the proper
> location. Maybe we can create a new apzc utils interface down in widget for
> this.
> 
> http://mxr.mozilla.org/mozilla-central/source/gfx/layers/composite/
> APZCTreeManager.h#133
> 
> Botond, any chance you could help us out with this? I'm not an expert on
> matrix math. You recently reworked all of this code so curious if you might
> be able to help us out.

Kats and I discussed this on IRC and we think adding a new interface to the APZ may not be necessary - instead, just multiplying the coordinates by the pres shell resolution should yield the desired transformation. The pres shell resolution can be obtained via nsIDOMWindowUtils (http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/base/nsIDOMWindowUtils.idl#188).

Perhaps we can try that first? If that doesn't work, we can revisit the idea of adding an interface to the APZ.

Comment 5

[Jim Mathies [:jimm]]

Attachment: part 1 - remove some old zoom code

Comment 6

[Jim Mathies [:jimm]]

Attachment: part 2 - update browser scale to return apzc resolution

Comment 7

[Jim Mathies [:jimm]]

Attachment: update browser scale to return apzc resolution

review -> mbrubeck
feedback -> kats, is my assumption here about symmetrical resolution correct?

Comment 8

[Jim Mathies [:jimm]]

Comment on attachment 8334044 [details] [diff] [review]
part 1 - remove some old zoom code

There's more here I can get rid of.

Comment 9

[Jim Mathies [:jimm]]

Attachment: remove some old zoom code

Comment 10

[Jim Mathies [:jimm]]

Attachment: remove some old zoom code

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8334522 [details] [diff] [review]
update browser scale to return apzc resolution

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

So the getResolution will give you the scale between LayoutDevicePixels and LayerPixels. I'm unsure of the context browser.scale is used in and so I'm not sure if that's always the right thing you want to be using. In particular this scale doesn't include the widget scale factor for hi-dpi screens, so I'm not sure if you want to be multiplying that in as well.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

(In reply to Jim Mathies [:jimm] from comment #7)
> feedback -> kats, is my assumption here about symmetrical resolution correct?

But yes, the assumption is correct.

Comment 13

[Jim Mathies [:jimm]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #11)
> Comment on attachment 8334522 [details] [diff] [review]
> update browser scale to return apzc resolution
> 
> Review of attachment 8334522 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> So the getResolution will give you the scale between LayoutDevicePixels and
> LayerPixels. I'm unsure of the context browser.scale is used in and so I'm
> not sure if that's always the right thing you want to be using. In
> particular this scale doesn't include the widget scale factor for hi-dpi
> screens, so I'm not sure if you want to be multiplying that in as well.

From testing with context menus this is giving us the coordinates we want. For example context menus for selection or links properly position around the text.

Comment 14

[Jim Mathies [:jimm]]

https://hg.mozilla.org/integration/fx-team/rev/bbda2959c7e3

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bbda2959c7e3