Set the correct buttons for long press action

third_party/WebKit/Source/core/input/EventHandler.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2006 Alexey Proskuryakov (ap@webkit.org)
  * Copyright (C) 2012 Digia Plc. and/or its subsidiary(-ies)
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
@@ skipping 2994 matching lines @@
     doc->updateHoverActiveState(request, result.innerElement());
 
     // The contextmenu event is a mouse event even when invoked using the keyboard.
     // This is required for web compatibility.
     PlatformEvent::Type eventType = PlatformEvent::MousePressed;
     if (m_frame->settings() && m_frame->settings()->showContextMenuOnMouseUp())
         eventType = PlatformEvent::MouseReleased;
 
     PlatformMouseEvent mouseEvent(locationInRootFrame, globalPosition,
         RightButton, eventType, 1,
         PlatformEvent::NoModifiers, PlatformMouseEvent::RealOrIndistinguishable,

[mustaq, 2016/01/19 19:33:41]

Please use the same logic here too.

[Navid Zolghadr, 2016/01/19 20:41:40]

No. I don't think that is correct. This is different. We have not sent any mouse
down before and this just sends the context menu event having right click mouse
being the cause of it (for compatibility reason). Or do you mean to also send
the mouse up/down in this case as well?

[mustaq, 2016/01/19 21:50:13]

I didn't mean sending extra events here. With button=RightButton, we are still
pretending that the right button is down if sending MousePressed, right?
PlatformEvent::NoModifiers is therefore inconsistent here. Should really depend
on  eventType: none if mouseRelease, rightButton if mousePressed.

The spec is a bit cryptic here:
https://html.spec.whatwg.org/multipage/forms.html#context-menus
but the MDN page is clear:
https://developer.mozilla.org/en-US/docs/Web/Events/contextmenu

[Navid Zolghadr, 2016/01/19 21:57:28]

I can definitely do that. The only point is that this is not what FF itself
does. It always sets the buttons field to 0 if I press menu key on FF/Linux
(which shows context menu on mouse down).

         WTF::monotonicallyIncreasingTime(), WebPointerProperties::PointerType::Mouse);
 
     return sendContextMenuEvent(mouseEvent, overrideTargetElement);
 }
 
 WebInputEventResult EventHandler::sendContextMenuEventForGesture(const GestureEventWithHitTestResults& targetedEvent)
 {
     const PlatformGestureEvent& gestureEvent = targetedEvent.event();
     unsigned modifiers = gestureEvent.modifiers();
 
     // Send MouseMoved event prior to handling (https://crbug.com/485290).
     PlatformMouseEvent fakeMouseMove(gestureEvent.position(), gestureEvent.globalPosition(),
         NoButton, PlatformEvent::MouseMoved, /* clickCount */ 0,
         static_cast<PlatformEvent::Modifiers>(modifiers),
         PlatformMouseEvent::FromTouch, gestureEvent.timestamp(), WebPointerProperties::PointerType::Mouse);
     dispatchMouseEvent(EventTypeNames::mousemove, targetedEvent.hitTestResult().innerNode(), 0, fakeMouseMove);
 
     PlatformEvent::Type eventType = PlatformEvent::MousePressed;
 
     if (m_frame->settings() && m_frame->settings()->showContextMenuOnMouseUp())
         eventType = PlatformEvent::MouseReleased;

[mustaq, 2016/01/19 19:33:41]

This seems to be firing mouseup when showContextMenuOnMouseUp is true but the
page is not seeing this because I believe the context menu code hides it. Please
add a comment here to explain this, and make the "else" part below
unconditional.

[Navid Zolghadr, 2016/01/19 20:41:40]

That is not the case. Although we set the eventType to MouseReleased but since
we call handleMousePressEvent it eventually sends a mousedown event. Should we
send a mouse up and mouse down if we show context on mouse up in this case? If
we send them both I can understand to have buttons=0 in contextmenu event as
well. Any thoughts?

[mustaq, 2016/01/19 21:50:13]

The down vs up mix up here seems non-trivial for sure. Calling
handleMousePresEvent() on mouseReleased seems bad here, but I agree it's not
easy to resolve. The method seems to have hit-testing related branching on
event-type. Let's leave it as is.

But the "else" part is effectively unconditional because you are ORing with
RBDown anyway, right? If I am not missing anything, please move it out of "else"
with a comment that we are sending mousedown, and remove the ORing below.

[Navid Zolghadr, 2016/01/19 21:57:28]

Sure. I will remove the else.

     else
         modifiers |= PlatformEvent::RightButtonDown;
 
     PlatformMouseEvent mouseEvent(targetedEvent.event().position(), targetedEvent.event().globalPosition(), RightButton, eventType, 1,
-        static_cast<PlatformEvent::Modifiers>(modifiers),
+        static_cast<PlatformEvent::Modifiers>(modifiers | PlatformEvent::RightButtonDown),
         PlatformMouseEvent::FromTouch, WTF::monotonicallyIncreasingTime(), WebPointerProperties::PointerType::Mouse);
     // To simulate right-click behavior, we send a right mouse down and then
     // context menu event.
     // FIXME: Send HitTestResults to avoid redundant hit tests.
     handleMousePressEvent(mouseEvent);
     return sendContextMenuEvent(mouseEvent);
     // We do not need to send a corresponding mouse release because in case of
     // right-click, the context menu takes capture and consumes all events.
 }
 
@@ skipping 1092 matching lines @@
 PlatformEvent::Modifiers EventHandler::accessKeyModifiers()
 {
 #if OS(MACOSX)
     return static_cast<PlatformEvent::Modifiers>(PlatformEvent::CtrlKey | PlatformEvent::AltKey);
 #else
     return PlatformEvent::AltKey;
 #endif
 }
 
 } // namespace blink