Context menus don't position right when content is zoomed

RESOLVED FIXED in Firefox 28

Status

Firefox for Metro
Browser
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jimm, Assigned: jimm)

Tracking

Trunk
Firefox 28
x86_64
Windows 8.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [block28])

Attachments

(2 attachments, 4 obsolete attachments)

(Assignee)

Description

4 years ago
str:

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

result: context menu is positioned incorrectly
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/
Created attachment 830024 [details]
aligned incorrectly when zoomed
(Assignee)

Comment 3

4 years ago
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.
Flags: needinfo?(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.
Flags: needinfo?(botond)
(Assignee)

Comment 5

4 years ago
Created attachment 8334044 [details] [diff] [review]
part 1 - remove some old zoom code
(Assignee)

Comment 6

4 years ago
Created attachment 8334088 [details] [diff] [review]
part 2 - update browser scale to return apzc resolution
Assignee: nobody → jmathies
(Assignee)

Updated

4 years ago
Attachment #8334044 - Flags: review?(mbrubeck)
(Assignee)

Comment 7

4 years ago
Created attachment 8334522 [details] [diff] [review]
update browser scale to return apzc resolution

review -> mbrubeck
feedback -> kats, is my assumption here about symmetrical resolution correct?
Attachment #8334088 - Attachment is obsolete: true
Attachment #8334522 - Flags: review?(mbrubeck)
Attachment #8334522 - Flags: feedback?(bugmail.mozilla)
(Assignee)

Comment 8

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

There's more here I can get rid of.
Attachment #8334044 - Flags: review?(mbrubeck)
(Assignee)

Comment 9

4 years ago
Created attachment 8334532 [details] [diff] [review]
remove some old zoom code
Attachment #8334044 - Attachment is obsolete: true
Attachment #8334532 - Flags: review?(mbrubeck)
(Assignee)

Updated

4 years ago
Attachment #8334522 - Attachment description: part 2 - update browser scale to return apzc resolution → update browser scale to return apzc resolution
(Assignee)

Comment 10

4 years ago
Created attachment 8334535 [details] [diff] [review]
remove some old zoom code
Attachment #8334532 - Attachment is obsolete: true
Attachment #8334532 - Flags: review?(mbrubeck)
Attachment #8334535 - Flags: review?(mbrubeck)
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.
Attachment #8334522 - Flags: feedback?(bugmail.mozilla) → feedback+
(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.
(Assignee)

Comment 13

4 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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.
(Assignee)

Updated

4 years ago
Attachment #8334535 - Flags: review?(mbrubeck)
(Assignee)

Updated

4 years ago
Attachment #8334535 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Depends on: 940632
(Assignee)

Updated

4 years ago
Blocks: 940952
Attachment #8334522 - Flags: review?(mbrubeck) → review+
https://hg.mozilla.org/mozilla-central/rev/bbda2959c7e3
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.