918225 - Defect - Right-click drop down options are offset

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: General, defect, P2)

Description

[Manuela Muntean [Away]]

Follow up defect from bug 898833

Reproducible with the latest Nightly (build ID: 20130918030202), both on Win 8 32-bit and Win 8.1 32-bit

STR

1. Enter Metro mode and load https://pkiwidgets.quovadisglobal.com/pkiwidgets/generateCSR.aspx
2. Using a mouse, right-click on a hyperlink and you get a set of options

Actual results: the options are offset with a wide 'white space' from the end margin to where the name of the option starts (please see https://bug898833.bugzilla.mozilla.org/attachment.cgi?id=796640 for details)

Expected results: the option names should be closer to the margin

Comment 1

[juan becerra [:juanb]]

Tested on 2013-09-30 on latest nightly on Dell XPS 12. I was not able to reproduce this problem as described in comment #0 on the latest nightly. I tried on other sites as well as using the touch gesture to bring up the context menu on links, pages, and selected text.

Comment 2

[Kamil Jozwiak [:kjozwiak]]

Attachment: Example of issue described in comment #0

I reproduced the problem using my desktop machine with a simple right click of the mouse. Simply right clicked on a menu and noticed the offset described in comment #0. I am receiving on every single site, including:
- image context menu's
- link context menu's
- HTML5 video context menu's

- Attached a screenshot to illustrate the issue that was described in comment #0

I couldn't reproduce this on my Lenovo X1 Carbon touch notebook with the mousepad or touch gestures

Comment 3

[Rodrigo Silveira [:rsilveira] [:rodms]]

Attachment: 918225.patch

As Marina mentioned in the project meeting, it was related to the left hand option. I forced it to true and was able to see the gap.

Comment 4

[Matt Brubeck (:mbrubeck)]

Comment on attachment 817510 [details] [diff] [review]
918225.patch

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

::: browser/metro/base/content/helperui/MenuUI.js
@@ -379,5 @@
>      let screenHeight = ContentAreaObserver.height;
>  
> -    // Add padding on the side of the menu per the user's hand preference
> -    let leftHand =
> -          Services.metro.handPreference == Ci.nsIWinMetroUtils.handPreferenceLeft;

This nsIWinMetroUtils property is now unused; maybe we should remove the widget code too?

Comment 5

[Rodrigo Silveira [:rsilveira] [:rodms]]

Attachment: Remove widget code

It's not being used anywhere and there are no plans to use it.

Comment 6

[Matt Brubeck (:mbrubeck)]

Comment on attachment 820616 [details] [diff] [review]
Remove widget code

Passing the review to a Windows widget peer.

Comment 7

[Matt Brubeck (:mbrubeck)]

Comment on attachment 820616 [details] [diff] [review]
Remove widget code

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

::: widget/nsIWinMetroUtils.idl
@@ -12,5 @@
>   * implementation of this interface for non-Windows systems, for testing and
>   * development purposes only.
>   */
>  [scriptable, uuid(25524bde-8b30-4b49-8d67-7070c790aada)]
>  interface nsIWinMetroUtils : nsISupports

...but one drive-by comment: I think you need to generate a new UUID here.

Comment 8

[Rodrigo Silveira [:rsilveira] [:rodms]]

Attachment: Remove widget handPreference code

Oh yeah, thanks! (mach update-uuids is handy!)

Comment 9

[Jim Mathies [:jimm]]

Comment on attachment 820632 [details] [diff] [review]
Remove widget handPreference code

Do we still have a fake nsIWinMetroUtils for non-windows platforms that needs updating?

r+ I guess. Are we sure we want to ditch this? It would be nice to support good left and right hand customization in our ui, but maybe we don't have any elements that need it?

Comment 10

[Matt Brubeck (:mbrubeck)]

(In reply to Jim Mathies [:jimm] from comment #9)
> Do we still have a fake nsIWinMetroUtils for non-windows platforms that
> needs updating?

Nope.

> r+ I guess. Are we sure we want to ditch this? It would be nice to support
> good left and right hand customization in our ui, but maybe we don't have
> any elements that need it?

Windows seems to use this setting in desktop apps rather than in Metro apps.  (Makes sense since it's found in the desktop control panel.)  If we're going to expose it anywhere, it would probably make more sense in the "win32" widget code.

Comment 11

[Rodrigo Silveira [:rsilveira] [:rodms]]

OK, I'll fold and land to make sure they're in sync. Resurfacing it would be trivial anyway.

Comment 12

[Rodrigo Silveira [:rsilveira] [:rodms]]

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

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/13132285f1c9

Comment 14

[Cornel Ionce [:noni] [Hubs QA]]

Mozilla/5.0 (Windows NT 6.2; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Windows NT 6.2; rv:27.0) Gecko/20100101 Firefox/27.0

Verified as fixed on latest Nightly (build ID: 20131028030205) by right clicking on links, images and html5 video context menus; the options are not offset anymore.