Prevent sending click event for non primary button

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 102 matching lines @@
         // eventPath() can be empty if event path is shrinked by relataedTarget retargeting.
         return DispatchEventResult::NotCanceled;
     }
     m_event->eventPath().ensureWindowEventContext();
 
     m_event->setTarget(EventPath::eventTargetRespectingTargetRules(*m_node));
     ASSERT(!EventDispatchForbiddenScope::isEventDispatchForbidden());
     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();
         }
     }
     dispatchEventPostProcess(preDispatchEventHandlerResult);
 
     // Ensure that after event dispatch, the event's target object is the
     // outermost shadow DOM boundary.
     m_event->setTarget(m_event->eventPath().windowEventContext().target());
@@ skipping 98 matching lines @@
                 m_event->eventPath()[i].node()->defaultEventHandler(m_event.get());
                 ASSERT(!m_event->defaultPrevented());
                 if (m_event->defaultHandled())
                     return;
             }
         }
     }
 }
 
 } // namespace blink