Bug 12399 - REGRESSION: Unable to prevent default context menu from appearing.
Summary: REGRESSION: Unable to prevent default context menu from appearing.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Beth Dakin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2007-01-24 14:46 PST by Tom Brown
Modified: 2007-02-22 16:06 PST (History)
1 user (show)

See Also:


Attachments
Test case reduction. (1.60 KB, text/html)
2007-01-24 14:56 PST, Tom Brown
no flags Details
Clear the controller's context menu before sending the event through the DOM (2.29 KB, patch)
2007-02-22 15:02 PST, Beth Dakin
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Brown 2007-01-24 14:46:58 PST
It is desirable to allow a developer to perform an action when the context menu ("Back, Reload, etc") is invoked. The "oncontextmenu" event exists for this purpose. The event can captured and canceled using standard methods. However, if the context menu is allowed to display even once, it can no longer be canceled until the window is closed. Firefox and IE both allow the event to be canceled repeatedly, even if the menu has been shown before.
Comment 1 Tom Brown 2007-01-24 14:56:54 PST
Created attachment 12651 [details]
Test case reduction.

1) Open test case.
2) Mark the checkbox labeled "prevent default context menu".
3) Attempt to open the default context menu by right-clicking or ctrl-clicking. Event data will be captured. The context menu will NOT be displayed. This is correct.
4) Unmark the checkbox labeled "prevent default context menu".
5) Right-click or ctrl-click to open the context menu. Event data will be captured. The context menu will be displayed. This is correct.
6) Clear the context menu.
7) Mark the checkbox labeled "prevent default context menu".
8) Attempt to open the default context menu by right-clicking or ctrl-clicking. Event data will be captured. The context menu WILL be displayed. This is NOT correct.

After this, any right-click (or ctrl-click) will cause the context menu to be displayed, regardless of whether or not the event is canceled. This will occur until the window is closed.
Comment 2 Tom Brown 2007-02-19 19:06:25 PST
This bug was introduced sometime between revisions 18554 and 18597, with 18592 being suspect ("turned on webcore context menus").
Comment 3 Beth Dakin 2007-02-20 14:22:28 PST
Confirmed. I think I know how to fix this . . .
Comment 4 Beth Dakin 2007-02-22 15:02:41 PST
Created attachment 13329 [details]
Clear the controller's context menu before sending the event through the DOM

This problem appeared because of two facets of the current context menu design. First, all context menu events are now considered to be "swallowed" since we take care of building up the regular context menu through the defaultEventHandler(). Second, the context menu controller holds onto it's context menu until a new one is created. There would be logistical problems changing this since AppKit relies on the menu being around for as long as it is visible on the screen and we don't get any notification once the menu is popped-down. 

There are a few different ways to fix this problem, but I think this is simplest. This solution does not change either of the two thing described above. It just clears out the controller's context menu before a new event is propagated through the DOM.
Comment 5 Adam Roben (:aroben) 2007-02-22 15:09:13 PST
Comment on attachment 13329 [details]
Clear the controller's context menu before sending the event through the DOM

+    page->contextMenuController()->clearContextMenu();

   It's probably worth a comment here saying why we have to do this.

   I wish we had a way to test this under DRT.

   r=me
Comment 6 Adam Roben (:aroben) 2007-02-22 15:19:54 PST
Comment on attachment 13329 [details]
Clear the controller's context menu before sending the event through the DOM

You should also add the testcase as a manual test.
Comment 7 Beth Dakin 2007-02-22 15:36:43 PST
rdar://5017416
Comment 8 Beth Dakin 2007-02-22 16:06:14 PST
Fixed with r19810.