Bug 22505 – Add a test to validate the event order for context clicks.

• Component: DOM
• Status: RESOLVED FIXED

[David Levin]

A key thing to verify is that "mouseup" shouldn't happen after "contextmenu".

[David Levin]

Created attachment 25515 [details]
Adds the test mentioned in the bug.

[Eric Seidel (no email)]

Comment on attachment 25515 [details]
Adds the test mentioned in the bug.

The test looks OK.  I'm just not sure why our results are correct?  Why would we want to send these events in different orders on different platforms?

We generally prefer tests which include the expected results over ones which just dump them.

If we could decide what the exact order for all platforms should be, then we could test for that explicitly.

[David Levin]

I thought the difference either in events sent for the different platforms was odd but thought I must not know something due to my newbie status.  Here's the survey that I did when looking at this test.

For Windows:
  Safari, Chromium, and Firefox all send a mousedown, then a mouseup, and then a contextmenu event all with event.button = 2 for all three events.

  IE sends the mousedown, mouseup with event.button =2, and then a contextmenu event with event.button = 0.

For OSX:

  Safari sends a mousedown and a contextmenu event all with event.button = 2.
  Firefox sends a mousedown, a contextmenu and then a mouseup event all with event.button = 2.

It seems to me that the correct behavior is to 
  "send a mousedown, then a mouseup, and then a contextmenu event all with event.button = 2 for all three events."

I'd be willing to change Safari on OSX to do this.  I'd modify 
    - (NSMenu *)menuForEvent:(NSEvent *)event in WebKit/mac/WebView/WebHTMLView.mm
by adding
        coreframe->eventHandler()->mouseUp(event);
right after the coreframe->eventHandler()->mouseDown(event); line.

I'll check if the contextmenu event is sent, it the mouseup is handled and mirror that behavior too (as well as add a regression test).

How does that sound?

PS fwiw, the mouseDown change was added by http://trac.webkit.org/changeset/28585 to mirror other browsers.)

[David Levin]

I had a discussion with Hixie on irc and he helped me to understand the platform difference.

mouseup shouldn't occur before contextmenu on osx because the contextmenu occurs on mousedown on OSX, so there is an expected difference for the mouseup event.

The main issue is for this test is that mouseup shouldn't occur after the contextmenu (on Windows).

[David Levin]

Comment on attachment 25515 [details]
Adds the test mentioned in the bug.

I think the patch should change to modify context-onmousedown-event-expected.txt, so I'll do that before having this reviewed further.

[David Levin]

Created attachment 25859 [details]
New patch.

On Windows, it is important for some web sites (like Google Spreadsheets) that the onmouseup event occurs before the oncontextmenu event or else the context menu disappears.

This test verifies that this event order for Windows.

[Eric Seidel (no email)]

Comment on attachment 25859 [details]
New patch.

Looks good.

[Pam Greene (IRC:pamg)]

Landed r39158.

The changes to fast/canvas/* from make-js-wrappers were already done in r39114. Removed those from the patch and ChangeLog.