74410 - context menus spawned from shift-F10 use mouse pos, not focussed element

Status: VERIFIED FIXED

(Core :: XUL, defect, P1)

Description

[Mike Pinkerton (not reading bugmail)]

From dean in bug 36665:

When I use VK_APPS to bring up a context menu, it works.  This is a good.  But 
the context menu applies to whatever's under the pointer, wherever that may be.

For example, tab to this link:  bug 16766  and move your pointer over this link:
 bug 45108 .  Press the application key (VK_APPS) and select Open Link In New
Window.  You'd expect a window with 16766 to open, but it's actually 45108.

This no less confusing if the pointer is over a blank area of the document,
because there's no link-related menu options at all.

Comment 1

[Mike Pinkerton (not reading bugmail)]

not sure if i'm the right one to own this bug.

Comment 2

[:deanis74]

Is there an API to get the rectangle of the currently focussed element on the 
active page?  If not, we should open a new bug for such a function and make 
this bug dependent on it.

Comment 3

[Jesse Ruderman]

Raising severity to major, since bug 36665 was major, and since the fix for 
36665 won't be very useful until this bug is also fixed.

Comment 4

[Mike Pinkerton (not reading bugmail)]

over to accessibility

Comment 5

[Peter Trudelle]

P1/Blocker.  Need ETA date in whiteboard summary.

Comment 6

[Mike Pinkerton (not reading bugmail)]

--
from bug 36665
I also looked into having the event indicate a frame that it applies to, rather 
than the current mouse coordinates. When windows fires a WM_CONTEXT_MENU event, 
the lParam is 0xffffffff if it was done via the keyboard. Normally the lParam 
has the mouse coordinates. How can we create a proper NS_CONTEXTMENU event for 
nsEventListenerManager.cpp that has the correct frame, rather than coordinates?
--

Aaron, what makes the most sense to me is that we create the NS_CONTEXTMENU event 
with no target frame. The ESM, which knows the focussed content, can then use 
that as a flag to fill it in before dispatching the event. How does that sound?

Comment 7

[Aaron Leventhal]

pink is working on this

Comment 8

[Mike Pinkerton (not reading bugmail)]

Attachment: [patch] first stab

Comment 9

[Mike Pinkerton (not reading bugmail)]

can someone give this a test on win32. my patch-fu is terrible.

Comment 10

[Mike Pinkerton (not reading bugmail)]

Attachment: [patch] same patch, dos line endings

Comment 11

[Mike Pinkerton (not reading bugmail)]

patch needs to include a small change in nsWindow.cpp to scope the context menu 
case. lame.

Comment 12

[Mike Pinkerton (not reading bugmail)]

if you tried the patch, you'll notice that it crashes. more work coming (danm 
gave me the right patch-fu).

Comment 13

[Mike Pinkerton (not reading bugmail)]

hrm. we still have a problem. the WM_RBUTTONDOWN that comes before WM_CONTEXTMENU 
clears the focused element so by the time we popup the context menu, the focus is 
long gone.

Comment 14

[Mike Pinkerton (not reading bugmail)]

Attachment: [patch #2] this patch actually works

Comment 15

[Mike Pinkerton (not reading bugmail)]

new patch attached, this one works on my win32 box. cc'ing roc to r= the small 
view changes.

Comment 16

[:deanis74]

Is it just me, or will "ret" not always have a value by the time it hits "if 
(NS_OK == ret)" in nsEventListenerManager.cpp?

Other than that, r=me on the non-view manager stuff.

Comment 17

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

r=roc+moz on the view changes, which just simplify the code a tiny bit (as well
as add the context menu key test)

Comment 18

[Mike Pinkerton (not reading bugmail)]

ret is initialized to ns_ok at the start, and changed only if the dom event 
wasn't created (the first time, after that it's reused). all events are like this 
and follow this pattern.

Comment 19

[:deanis74]

Whoops, sorry.  I didn't check into that.  More formally, then
r=[e-mail].

Comment 20

[David Hyatt]

sr=hyatt

Comment 21

[Mike Pinkerton (not reading bugmail)]

landed

Comment 22

[:deanis74]

Looks good on Windows using 2001051720.  A nice enhancement would be for the 
context menu to open close to the focused item instead of at 0,0.

Now for Linux and Mac... anyone?

Comment 23

[sairuh (rarely reading bugmail)]

using 2001.05.18.13 comm verif bits, i cannot bring up the context menu using
shift+F10 on linux or mac. the cursor just disappears momentarily. is there
another bug [other than bug 36665] which still needs to get fixed for this to work?

on winnt, i see what dean saw --the context menu appears at 0,0 in the content
area. but, wasn't this bug supposed to fix that? or is there another bug for that?

Comment 24

[Mike Pinkerton (not reading bugmail)]

oh, i didn't do any work for linux or mac. someone needs to tell me what the key
combos are for showing a context menu for the keyboard.

Comment 25

[:deanis74]

Whoops, that's right, this work was for Windows only, wasn't it?

This bug was to ensure that the context menu was activated for the focussed 
content when brought up by the keyboard.  It originally popped up for whatever 
was under the pointer.

Marking verified.  Filed bug 81723 for the positioning request.

Comment 26

[sairuh (rarely reading bugmail)]

hrm, confusion. /me looks at the platform/os... okay i've filed bug 81727 to
cover implementation in linux and mac. :)