312225 - When right-clicking for context menu in bottom-right corner of screen, last menu option is immediately selected

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[gabe]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.7.12) Gecko/20051004 Firefox/1.0.7
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.7.12) Gecko/20051004 Firefox/1.0.7

If you right click in a browser window a context menu normally appears. 
However, if you right click in the bottom-right area of the screen(not just
browser), so that the context menu appears above, and not fully to the right of
the curser, the context menu appears breifly and the bottom-most menu item is
selected.  This seems to be because when you release the right-click, the curser
is actually over the last menu item, and the context menu registers this as a
mouse click release.

Reproducible: Always

Steps to Reproduce:
1. Move mouse to bottom-right corner of screen (must be screen, not just browser
window)
2. Right-click and release.
3.

Actual Results:  
Context menu briefly appeard and disapeared.  Last menu option was activated, in
this case the "Page Info" window was displayed.

Expected Results:  
Context menu should have remained until an option was selected or I clicked
outside of context menu.

Comment 1

[Brett Wilson]

I couldn't reproduce this on Windows (2005 Oct 13 trunk and 1.5b2) or on my
version of Linux with similar versions. If I click on the exact corner, the menu
appears slightly to the left of the mouse. If I click near the corner, the menu
is aligned with the bottom of the screen (and hence, the mouse), but the border
of the menu doesn't count as the menu.

Could this be theme related on your Linux system?

Comment 2

[Wladimir Palant]

This should be filed under Core - XP Toolkit/Widgets: Menus
Tweaking product and component...

Comment 3

[Wladimir Palant]

I got a report (http://bugzilla.mozdev.org/show_bug.cgi?id=13196) that this happens in Firefox 1.5.0.1 on Linux with the default theme. I can't confirm this however, for me the context menu works fine (Firefox 1.5.0.1 on KDE).

Comment 4

[nickers]

I can reproduce this bug on Gnome 2.12.2 with default settings (Metacity + Clearlooks). The bug seems to be caused by the fact, that the edge of context menu is always positioned a few pixels to the bottom-right from the mouse pointer. I suggest this to be changed, so the pointer is a few pixels _away_ from the menu.

The bug is also present in the current nightly [Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9a1) Gecko/20060213 Firefox/1.6a1].

Comment 5

[Boris Zbarsky [:bzbarsky]]

So... I can't seem to reproduce this.  roc, can you reproduce?

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Nope. The cursor is in the menu, but I have to move it a pixel or three upwards before an item is selected.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Hmm.. For me it's not even in the menu; it really shouldn't be in the menu....

Comment 9

[Florian Quèze [:florian]]

I can reproduce this on Ubuntu 6.10 with a current trunk build.
(Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a3pre) Gecko/20070222 Minefield/3.0a3pre)

When I open the context menu, the cursor is over the last item and it opens Page Info when I release the button.
I need to move the cursor one pixel upwards to get the highlight on the last menu item.
If I move the cursor at least one pixel downwards before releasing the button, the menu stays open.

Comment 10

[u294409]

It has not been fixed until three years?! Nice...
CC'ing myself.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Here's my fix. The problem is that the context menu code tries to move the menu two pixels down and to the right so that the menu does not appear under the cursor. But sometimes --- when you're near the bottom right of the screen --- nsMenuPopupFrame decides to place the context menu above the designated position; in that case, moving the menu down two pixels was wrong, we actually need to move the menu up two pixels.

So this patch passes the 2-pixel offset down as a parameter to nsMenuPopupFrame, which can use that information to place the popup correctly if it decides to make it appear above the mouse position.

Comment 12

[Neil Deakin]

Seems like it would be easier to just add the 2 pixels in nsMenuPopupFrame::SetPopupPosition as needed when mIsContextMenu is true instead.

  

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

That would change the behaviour of nsPopupBoxObject::OpenPopupAtScreen. If that's OK with you, I'll do that.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix v2

Okay, fixed as described.

Comment 15

[Neil Deakin]

Comment on attachment 316915 [details] [diff] [review]
fix v2

>-nsMenuPopupFrame::InitializePopupAtScreen(PRInt32 aXPos, PRInt32 aYPos)
>+nsMenuPopupFrame::InitializePopupAtScreen(PRInt32 aXPos, PRInt32 aYPos,
>+                                          PRBool aIsContextMenu)
> {
>   EnsureWidget();
> 
>   mPopupState = ePopupShowing;
>   mAnchorContent = nsnull;
>   mScreenXPos = aXPos;
>   mScreenYPos = aYPos;
>   mPopupAnchor = POPUPALIGNMENT_NONE;
>   mPopupAlignment = POPUPALIGNMENT_NONE;
>+  mIsContextMenu = aIsContextMenu;
>+  mAdjustOffsetForContextMenu = aIsContextMenu;

mAdjustOffsetForContextMenu should be set to false for the other two InitializeX methods.

>     screenViewLocY = nsPresContext::CSSPixelsToAppUnits(mScreenYPos) / factor;
>+    PRInt32 offsetForContextMenuCSS = mAdjustOffsetForContextMenu ? 2 : 0;
>+    PRInt32 offsetForContextMenuDev =
>+      nsPresContext::CSSPixelsToAppUnits(offsetForContextMenu) / factor;

You meant 'offsetForContextMenuCSS' here right?

I assume with this patch that the test for contextmenus (test_contextmenu_list.xul) works ok?

I'm not a superreviewer, so let's ask the other Neil.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix v3

Updated to comments.

test_contextmenu_list.xul passes on my Mac.

Comment 17

[[e-mail]]

Comment on attachment 317116 [details] [diff] [review]
fix v3

>+  nscoord offsetForContextMenu = 0;

>+    PRInt32 offsetForContextMenuCSS = mAdjustOffsetForContextMenu ? 2 : 0;
>+    PRInt32 offsetForContextMenuDev =
>+      nsPresContext::CSSPixelsToAppUnits(offsetForContextMenuCSS) / factor;
>+    offsetForContextMenu = presContext->DevPixelsToAppUnits(offsetForContextMenuDev);
Any reason not to do this in an if rather than possibly calculating with zero?

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I guess it's about as simple to put the whole thing in an "if", sure, I'll do that.

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 317116 [details] [diff] [review]
fix v3

Fixes a pretty bad Linux usability issue where we fire random context menu commands in some situations (I think it's a regression). May also fix or at least narrow down blocker bug 404314. Rather safe fix too, I think.

Comment 20

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 317116 [details] [diff] [review]
fix v3

a1.9=beltzner

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Checked in.

Comment 23

[r0polach]

I sometimes still see this problem in 3.5 (windows)
although in much less frequency than in 3.0.9...