340870 - Control-click doesn't bring up contextual menu in Cocoa widgets

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Mark Mentovai]

Steps to reproduce:
1. Command-click on something.

Expected behavior:
See a contentless contextual menu popup window (in its present broken state)

Observed behavior:
Nothing.  Actually, the contextual menu occasionally flickers open but disappears immediately.

Comment 1

[Josh Aas]

Attachment: fix v1.0

just threw this together, not requesting review until I've had more time to think about it

Comment 2

[Josh Aas]

Attachment: stack trace #1

I figured something fishy was going on here. A context menu is being shown on control-click, its just being immediately closed. To figure out what was closing it, I started up the browser in gdb and set a breakpoint at orderOut:, which is what would be called on a context menu to close it. Then I control-clicked. This is the stack I got, posting it here for my own reference if nothing else.

Fix v1.0 is almost certainly not what we want. We need to figure out why the context menu is being closed and stop that from happening.

Comment 3

[Josh Aas]

Attachment: fix v2.0

This is the right way to deal with the problem. We always want to send gecko a mouse down event for any mouse down (right or left), and in some cases we want to send a context menu event afterwards. The problem with sending context menu events from menuForEvent: is that the mouseDown: message comes after menuForEvent:, but rightMouseDown: comes before menuForEvent: (Cocoa oddity). Because of this, menuForEvent would put up a context menu on control-click, but mouseDown: would immediately close it since it unaware that some other native event just put the context menu up.

This patch does away with menuForEvent: altogether and handles context menu events in the mouse button handlers. I used the GTK2 impl as a reference.

Comment 4

[Håkan Waara]

I think menuForEvent: is used to invoke context menus by other means that clicking (for example, by VoiceOver). If that's the case, we need to use it.

How about inventing a BOOL that will make sure the useless click event after the menuForEvent: is disregarded instead?  Overall, I'd find that cleaner than relying on stuff happening a mousedown event.

Comment 5

[Josh Aas]

Attachment: fix v3.0

Comment 6

[Josh Aas]

Attachment: fix v3.1

Comment 7

[Håkan Waara]

Comment on attachment 225312 [details] [diff] [review]
fix v3.1

Tiny nit, this leaks the stored event, if there is one, when the child view goes away, right?

Comment 8

[Mark Mentovai]

Comment on attachment 225312 [details] [diff] [review]
fix v3.1

Release mLastMenuForEventEvent (nice name) when deallocing the childview.

Comment 9

[Josh Aas]

yep, fixed on checkin. this is landed on trunk.

Comment 10

[Hank Mills]

While control-click does now bring up a context menu, right click still does not. Not sure if I should comment here or open a new bug. FWIW, right click does create a context menu in the location bar.