Prevent sending click event for non primary button

third_party/WebKit/Source/core/events/EventDispatcher.cpp

Index: third_party/WebKit/Source/core/events/EventDispatcher.cpp
diff --git a/third_party/WebKit/Source/core/events/EventDispatcher.cpp b/third_party/WebKit/Source/core/events/EventDispatcher.cpp
index c97cc0e40d012f1906676a19a9db2df0f1ace10a..230e8309f875af6f6d38bde3794ea65c5becd183 100644
--- a/third_party/WebKit/Source/core/events/EventDispatcher.cpp
+++ b/third_party/WebKit/Source/core/events/EventDispatcher.cpp
@@ -120,7 +120,13 @@ DispatchEventResult EventDispatcher::dispatch()
     ASSERT(m_event->target());
     TRACE_EVENT1("devtools.timeline", "EventDispatch", "data", InspectorEventDispatchEvent::data(*m_event));
     EventDispatchHandlingState* preDispatchEventHandlerResult = nullptr;
-    if (dispatchEventPreProcess(preDispatchEventHandlerResult) == ContinueDispatching) {

[dtapuska, 2016/04/22 18:32:30]

Adding this here seems incorrect and a number of layering violations.

I imagine other browsers will allow:
node.dispatchEvent( new MouseEvent("click", {button: 1}); to be dispatched no?

Conceptually I'd expect us to just avoid generating the click event here:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

But I presume I am missing something.

[Navid Zolghadr, 2016/04/22 19:09:58]

I realized this file shouldn't be for mouse specific checks. But there was
already some specific checks for mouse in this file. So I thought it shouldn't
be that bad. But FireFox does let that fake event to be fired which I didn't
know it would pass this part of the code and as a result we do not let them fire
that after this CL which obviously is incorrect.

I wanted to have this suppressing logic in EventHandler in the first place. But
the problem is calling the default behavior of click in EventHandler.  Basically
if this event is suppressed in EventHandler layer the new tab will not open.
What do you suggest about that? Here it traverses up in the tree and calls the
default behavior on all the parents. I assume it does more logic anyway around
that code. 

Do you think kind of copying that logic (say only the default behavior calling
on all the parents) to EventHandler or somewhat having it exposed to
EventHandler with some functions is better?

[dtapuska, 2016/04/22 19:16:32]

What about removing this and adding:

    if (nativeEvent.button() != LeftButton)
      event->stopPropagation();

In Node.cpp dispatchMouseEvent...

Would that get the best of both worlds?

[Navid Zolghadr, 2016/04/25 18:37:48]

Done.

+
+    bool isClickForNonPrimaryButtons = m_event->isMouseEvent()
+        && toMouseEvent(*m_event).type() == EventTypeNames::click
+        && toMouseEvent(*m_event).button() != MouseButton::LeftButton;
+
+    if (!isClickForNonPrimaryButtons
+        && dispatchEventPreProcess(preDispatchEventHandlerResult) == ContinueDispatching) {
         if (dispatchEventAtCapturing() == ContinueDispatching) {
             if (dispatchEventAtTarget() == ContinueDispatching)
                 dispatchEventAtBubbling();