271498 - Context menu appears top left of page when using keyboard

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Matti]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041121
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041121

When right clicking with the mouse, the context menu appears next to the cursor.
When using the keyboard button it appears at the top left of the page under the
menu bars.

Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Comment 1

[Troodon]

Dupe of bug 175568.

Comment 2

[Mats Palmgren (inactive)]

Please reopen the bug if it still occurs in a recent nightly build...

*** This bug has been marked as a duplicate of 175568 ***

Comment 3

[Matti]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050129

Still happening.

Comment 4

[Gervase Markham [:gerv]]

This is an automated message, with ID "auto-resolve01".

This bug has had no comments for a long time. Statistically, we have found that
bug reports that have not been confirmed by a second user after three months are
highly unlikely to be the source of a fix to the code.

While your input is very important to us, our resources are limited and so we
are asking for your help in focussing our efforts. If you can still reproduce
this problem in the latest version of the product (see below for how to obtain a
copy) or, for feature requests, if it's not present in the latest version and
you still believe we should implement it, please visit the URL of this bug
(given at the top of this mail) and add a comment to that effect, giving more
reproduction information if you have it.

If it is not a problem any longer, you need take no action. If this bug is not
changed in any way in the next two weeks, it will be automatically resolved.
Thank you for your help in this matter.

The latest beta releases can be obtained from:
Firefox:     http://www.mozilla.org/projects/firefox/
Thunderbird: http://www.mozilla.org/products/thunderbird/releases/1.5beta1.html
Seamonkey:   http://www.mozilla.org/projects/seamonkey/

Comment 5

[Gervase Markham [:gerv]]

This bug has been automatically resolved after a period of inactivity (see above
comment). If anyone thinks this is incorrect, they should feel free to reopen it.

Comment 6

[Sander]

Still happening with SeaMonkey 1.0a - this used to work correctly in the mozilla
1.0 days. Firefox does the same

Comment 7

[Brett Wilson]

Here's the behavior (hashed out in bug 175568):

If nothing's focused, the context menu appears at the top left of the window.
This seems like fine behavior, it has to appear somewhere.

If an element is focused, the top-left of the element is made visible and the
menu appears slightly below and to the right of the top-left of the element.
This works great for links.

The problem is if you are in a textarea. I expect the context menu to appear by
the cursor, since this is where you are looking when you are typing, and this is
what Word does. This will become more obvious when the inline spell checker is
landed (bug 302050) and people are more likely to use the context menu key to
access spell suggestions. The worst is when the textarea is large. The screen
will scroll so the top is visible, even if this scrolls the cursor off the
bottom of the screen.

I think the menu should appear at the cursor if you are in caret browsing mode
as well. So: current behavior is fine when there is no cursor. If there is a
cursor, the menu should appear there.

Comment 8

[Brett Wilson]

Attachment: Patch to fix the context menu

This patch implements my above criteria. If you have a caret, either because
you're caret browsing or because you're in a text edit box, the context menu
will appear under the caret. Otherwise, the behavior is the same as before (the
menu will pop up near the corner of the focused element/window).

Note: If you don't have a context menu key on your keyboard (next to the
right-hand Windows key) or it doesn't work (Linux), you can also use Shift+F10.

Comment 9

[Brian Ryner (not reading)]

Comment on attachment 199958 [details] [diff] [review]
Patch to fix the context menu

>--- content/events/src/nsEventListenerManager.cpp	26 Sep 2005 14:37:19 -0000	1.212
>+++ content/events/src/nsEventListenerManager.cpp	18 Oct 2005 17:51:41 -0000
>@@ -1725,17 +1728,16 @@ nsEventListenerManager::HandleEvent(nsPr
>   if (aEvent->message == NS_CONTEXTMENU || aEvent->message == NS_CONTEXTMENU_KEY) {
>     ret = FixContextMenuEvent(aPresContext, aCurrentTarget, aEvent, aDOMEvent);
>     if (NS_FAILED(ret)) {
>       NS_WARNING("failed to fix context menu event target");
>       ret = NS_OK;
>     }
>   }
> 
>-

Try to avoid unrelated whitespace changes like this, if possible.

>@@ -2205,16 +2207,18 @@ nsEventListenerManager::GetSystemEventGr
> }
> 
> nsresult
> nsEventListenerManager::FixContextMenuEvent(nsPresContext* aPresContext,
>                                             nsIDOMEventTarget* aCurrentTarget,
>                                             nsEvent* aEvent,
>                                             nsIDOMEvent** aDOMEvent)
> {
>+  nsresult ret = NS_OK;
>+

Same here; I don't see a real advantage to moving this around.

>+PRBool
>+nsEventListenerManager::PrepareToUseCaretPosition(nsIPresShell* aShell,
>+                                                  nsPoint& aTargetPt)
>+{
...
>+  nsCOMPtr<nsIContent> content(do_QueryInterface(node));
>+  for ( ; content; content = content->GetParent()) {
>+    if (!content->IsNativeAnonymous()) {

I'd like to better understand this check.  This basically means that for
various types of automatically generated content, we'll target the context menu
using the parent node.	Not sure if you copied this part from the find code,
but I'm not sure it makes sense for this, since all we want to do is scroll the
frame into view.

>+  // an edit box below the current view, you'll get the edit box aligned with
>+  // the top of the window. This is arguably better behavior anyway.
>+  if (frame) {
>+    rv = aShell->ScrollFrameIntoView(frame, NS_PRESSHELL_SCROLL_IF_NOT_VISIBLE,
>+      NS_PRESSHELL_SCROLL_IF_NOT_VISIBLE);

Indent this to be aligned with "frame".

>+  nsCOMPtr<nsISelectionController> selCon;
>+  if (tcFrame)
>+    tcFrame->GetSelectionContr(getter_AddRefs(selCon));
>+  else
>+    selCon = do_QueryInterface(aShell);
>+  if (selCon) {
>+    NS_ENSURE_SUCCESS(rv, PR_FALSE);

rv hasn't changed since the last time you checked it.

Looks good otherwise; r=me with those fixed, and/or a comment about the
IsNativeAnonymous check.

Comment 10

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

Comment on attachment 199958 [details] [diff] [review]
Patch to fix the context menu

This seems to assume that the view returned by GetCaretCoordinates has the
widget referenced by the event. That's probably currently true for textboxes
but it's a fragile assumption. You need to translate the coordinates from the
returned view into the widget's event's view (by calling nsIView::GetOffsetTo
and nsIView::GetNearestWidget to get any offset between the event's widget and
its view).

Comment 11

[Brett Wilson]

Attachment: Revised context menu patch

This addresses bryner's suggestions.

roc: I added a conversion to the event widget's view. If you could, please
check this very carefully. I don't fully understand the details here, and the
previous code worked in all cases I could find. Any change that works out to 0
would be indistinguishable from correct as far as I am concerned.

Comment 12

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

Comment on attachment 200015 [details] [diff] [review]
Revised context menu patch

+  nsIView* widgetView = view->GetViewFor(aEventWidget);
+  nsPoint viewDelta = view->GetOffsetTo(widgetView);

you want
nsIView* widgetView = nsIView::GetViewFor(aEventWidget);
nsPoint viewToWidget
widgetView->GetNearestWidget(&viewToWidget);
nsPoint viewDelta = view->GetOffsetTo(widgetView) + viewToWidget;

Comment 13

[Brett Wilson]

Attachment: roc's view suggestions incorporated

This just incorporates roc's above change.

Comment 14

[Jesse Ruderman]

There was a 2005-11-14 18:00 trunk checkin for this bug, with the comment "bug 271498: context menus should appear at cursor when using the keyboard r=bryner sr=roc".  Does that mean this bug is fixed?

Comment 15

[Adam Guthrie]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20051125 Firefox/1.6a1

Still seeing this on Linux, FWIW.

Comment 16

[Brett Wilson]

(In reply to comment #14)
> ...Does that mean this bug is fixed?

Yes. Sorry, I haven't been very good about marking things fixed.

Adam: I just tested the 2005.11.28 Linux nightly and it works fine. Are you sure you're using it correctly? If you just press Shift+F10 in normal browsing, the context menu will still open at the top left (it's got to appear somewhere). If you are in caret browsing mode or have a caret in a textbox, then it should appear at the cursor. Please reopen if you still see the problem.

Comment 17

[Adam Guthrie]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20051125 Firefox/1.6a1

Yep, this works just as Brett described.

Comment 18

[Brett Wilson]

Attachment: Branch patch

This branch patch is the trunk patch above plus the patch to bug 317145, a crasher bug that the trunk patch introduced. The only thing changed in these two patches is the call to GetPrimaryFrameFor which changed slightly on trunk.

Comment 19

[Mike Cowperthwaite]

Comment on attachment 217576 [details] [diff] [review]
Branch patch

I don't have rights to approve anything.

Comment 20

[Brett Wilson]

(In reply to comment #19)
> (From update of attachment 217576 [details] [diff] [review] [edit])
> I don't have rights to approve anything.

Sorry, I mistakenly thought I put you in for approval before, so I did that again when I put up the new patch.

Comment 21

[Boris Zbarsky [:bzbarsky]]

I'm not going to be able to look at this until at least Sunday, and most probably later.  Might be better to have roc approve, since he did the reviews and knows this code...

Comment 22

[Brett Wilson]

OK, thanks Boris.

Comment 23

[Brett Wilson]

Fixed on 1.8 branch.

Comment 24

[Bryan]

I've noticed this case:

1) Launch FF
2) Navigate to this page http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?treeid=default&module=AviarySuiteBranchTinderbox&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&date=hours&hours=24&mindate=&maxdate=&cvsroot=%2Fcvsroot
3) Click once in the URL field to highlight the URL
4) Click once on the "context-menu key" (between windows and control keys on the right) on your keyboard if you have one
5) Observe the "Cut, Copy, Paste and Delete" context menu is shown on the far right of the screen.  Not exactly where users might expect it to appear.

Bryan

Comment 25

[Brett Wilson]

Please open a new bug for this. I think it only happens on Windows.

Comment 26

[Mike Cowperthwaite]

*** Bug 332080 has been marked as a duplicate of this bug. ***