Bug 15173 – Middle click should not fire onclick event

• Component: DOM
• Status: RESOLVED DUPLICATE

[Robert Blaut]

I've noticed that Safari is the only browser I've tested which fire onclick event after using middle click (wheel button) to load a link on a new tab. This is very frustrating behaviour, not visible in other popular browsers on Windows (Internet Explorer 7, Firefox 2 and Opera 9).

Steps to reproduce:

1. Go to a page: http://bugs.blaut.biz/webkit/onclick-issue.htm
2. Middle click on the only link on the page.

Expected result:

Safari should open a new tab and load a page http://webkit.org. The tab with the clicked link should not load webkit.org site.

Current result:

Safari loads on a new tab a page http://webkit.org and also load a page webkit.org on the tab with the clicked link.

[Robert Blaut]

Created attachment 16247 [details]
A minimal test case

[Mark Rowe (bdash)]

<rdar://problem/5473092>

[Robert Blaut]

(In reply to comment #2)
> <rdar://problem/5473092>
> 

I assume the bug is confirmed.

[Tony Chang]

It looks like in Firefox, click event handlers only fire for left click.

In IE7, click events fire for left click or middle click.  However, anchor tags seem to be a special case where if there's a href, the href is opened in a new tab.  Otherwise, the click event handler runs.

As far as I can tell, it seems like the click event should fire for middle clicks, but the spec seems ambiguous:
http://www.w3.org/TR/DOM-Level-3-Events/events.html#event-click

Here's a good test case:
http://www.w3.org/WAI/UA/TS/html401/cp0102/0102-ONCLICK-MULTIPLE-EVENTS.html

Also, this is what causes middle clicking a link in Gmail to open in a new window rather than as a background tab in Safari.

[Eric Seidel (no email)]

The "fix" to this would be a one line change in:
EventHandler.cpp:
bool EventHandler::handleMouseReleaseEvent(const PlatformMouseEvent& mouseEvent)

There is already code in that function to avoid ever dispatching click events for right-clicks, we'd just have to change it to not dispatch clicks for middle clicks as well (possibly only to dispatch click events for left-clicks in general).

[Peter Kasting]

"A pointing device button" on the spec seems unambiguous that any button should fire clicks.  It's not clear to me whether to claim the other browsers are wrong (certainly not firing clicks on middle/right clicks _seems_ bizarre, and makes life harder on JS authors who want to use those buttons to trigger actions), or to claim that the spec should change.

[Ian 'Hixie' Hickson]

It seems like the better behaviour would be to fire the click event normally, but then catch any attempt at navigating the page, and instead make another browsing context navigate, or something like that. Then it would work with all kinds of weird scripts, too. That's more work, though.

[Eric Seidel (no email)]

Created attachment 21957 [details]
Only dispatch clicks for left-button clicks

 WebCore/ChangeLog             |   13 +++++++++++++
 WebCore/page/EventHandler.cpp |    7 ++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

[Eric Seidel (no email)]

I see there are differing opinions above as to how this should be solved.  Regardless, I wrote a patch which solves this by making us not send these click events like other browsers.

I also noticed in my testing that we do not send a mouse-release event on right-clicks.  FF does (right as it's displaying the context menu).

[Darin Adler]

Comment on attachment 21957 [details]
Only dispatch clicks for left-button clicks

Lets add something to eventSender so we can do an automated tests. Should be simple.

[Ian 'Hixie' Hickson]

I think Eric's patch is fine for now, my earlier suggestion is what I think we should do on the long term though. It would work for far more cases, and would be really cool.

[Eric Seidel (no email)]

Created attachment 21976 [details]
First attempt at adding eventSender.mouseDown(1) support

 .../DumpRenderTree.xcodeproj/project.pbxproj       |    2 +-
 .../DumpRenderTree/mac/EventSendingController.h    |    2 +-
 .../DumpRenderTree/mac/EventSendingController.mm   |   95 ++++++++++++++++----
 3 files changed, 79 insertions(+), 20 deletions(-)

[Eric Seidel (no email)]

Created attachment 21977 [details]
Add multi-button mouseevent support to DRT and test case

 .../fast/events/mouse-click-events-expected.txt    |   32 +++++++
 LayoutTests/fast/events/mouse-click-events.html    |   13 +++
 .../fast/events/resources/mouse-click-events.js    |   45 ++++++++++
 .../DumpRenderTree.xcodeproj/project.pbxproj       |    2 +-
 .../DumpRenderTree/mac/EventSendingController.h    |    2 +-
 .../DumpRenderTree/mac/EventSendingController.mm   |   88 ++++++++++++++++----
 6 files changed, 165 insertions(+), 17 deletions(-)

[Eric Seidel (no email)]

Created attachment 21980 [details]
Add multi-button mouseevent support to DRT and test case

 LayoutTests/ChangeLog                              |   12 ++
 .../fast/events/mouse-click-events-expected.txt    |   32 ++++++
 LayoutTests/fast/events/mouse-click-events.html    |   13 +++
 .../fast/events/resources/mouse-click-events.js    |   45 ++++++++
 WebKitTools/ChangeLog                              |   22 ++++
 .../DumpRenderTree.xcodeproj/project.pbxproj       |    2 +-
 .../DumpRenderTree/mac/EventSendingController.h    |    2 +-
 .../DumpRenderTree/mac/EventSendingController.mm   |  111 ++++++++++++++++----
 8 files changed, 214 insertions(+), 25 deletions(-)

[Darin Adler]

Comment on attachment 21957 [details]
Only dispatch clicks for left-button clicks

r=me, assuming you land the test too

[Darin Adler]

Comment on attachment 21980 [details]
Add multi-button mouseevent support to DRT and test case

Looks good, but we need to add this test to the skipped list for the other platforms, since the new support in DumpRenderTree is Mac-only.

r=me

[Eric Seidel (no email)]

Actually.  My patch just breaks middle-click-to-open-links entirely.  I guess the "middle click to open links" behavior depends on there being a click event.   Sigh.

[Eric Seidel (no email)]

(In reply to comment #17)
> Actually.  My patch just breaks middle-click-to-open-links entirely.  I guess
> the "middle click to open links" behavior depends on there being a click event.
>   Sigh.

Yeah, of course it does.  We do navigation based on HTMLAnchorElement::defaultEventHandler.  I'm not sure why I ever thought this would work.  I guess the test case is still useful, but there is no easy way for us to replicate FF/IE behavior here.  I think we should scratch this patch and implement a better "window.open opens in new tab if called from a middle-click" behavior.  However I bet that's a Safari change more than a WebKit change.

[Eric Seidel (no email)]

Comment on attachment 21957 [details]
Only dispatch clicks for left-button clicks

Removing Review flag.  This patch breaks more than it helps.  I'll still land the test case patch though.

[Eric Seidel (no email)]

Ok.  I landed the test case and DRT enhancements as part of:
http://trac.webkit.org/changeset/34871

We've decided to continue to send these events but instead fix window.open to open in a new tab when the result of a command-click or middle-click.  The rest needs to be done in Safari (And is thus tracked internally by Apple).

Closing.

[Eric Seidel (no email)]

So unfortunately Gmail still isn't fixed (middle clicking on links, still opens in a new window).

Part of that is going to be a Safari fix, but part of it is going to need to be a WebKit fix.  I don't believe this could be fixed in Safari TOT, since WebKit TOT isn't passing any event context information to the WebViewLoadDelegate as part of the open new url callback.  the NavigationAction does not have an event() set on it when you middle-click a gmail link, so there is no way this information could be being passed up to Safari.

I don't think this is the right bug to track the brokeness of gmail-middle-clicking.  But I was asked by some avid Safari users internally to finally see if I couldn't get to the bottom of this Safari + Gmail bug.  As far as I can tell, there are still fixes which need to happen in WebKit (and possibly Safari as well).

Perhaps someone on the Apple side of things can give me some guidance as to any work you all have done to fix the "Safari + middle-click in Gmail opens a new window instead of a new-tab behind the current" bug.

[Adam Barth]

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