Bug 1298908 - On Windows, fire the contextmenu and long-tap events on the long-tap-up user action. r=botond,jimm
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 30 Aug 2016 17:32:08 -0400
changeset 312360 fd96f3fee9754562392a3877464b10aa90f82b71
parent 312359 01b5da819a7c9281b00d3f2cb0959713b3078580
child 312361 42ada9dc0ab3c9e872f56201a0a1c4f7eb685acc
push id20447
push userkwierso@gmail.com
push dateFri, 02 Sep 2016 20:36:44 +0000
treeherderfx-team@969397f22187 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond, jimm
bugs1298908
milestone51.0a1
Bug 1298908 - On Windows, fire the contextmenu and long-tap events on the long-tap-up user action. r=botond,jimm This patch prevents the Windows widget code from dispatching the contextmenu event if APZ is handling touch input. Instead, the APZ code processes the raw touch input, and will fire a contextmenu event when the user lifts their finger after a long-press action, in keeping with the Windows platform convention. Doing it this way also allows us to respect web conventions where the web content can prevent the contextmenu event from firing by calling preventDefault on the touchstart event; this was not possible when dispatching the contextmenu event directly from the widget code. This also makes long-pressing on browser chrome components work properly, as it just shifts the point in time that the contextmenu event is fired without changing any of the code that triggers the XUL popup. However, some changes were needed to have the widget code ignore the synthetic mouse events that the Windows platform sends us, because those would otherwise immediately dismiss the contextmenu popup after it appeared. MozReview-Commit-ID: 9HFZLC6xUAi
dom/ipc/TabChild.cpp
gfx/layers/apz/util/APZEventState.cpp
gfx/layers/apz/util/APZEventState.h
gfx/layers/apz/util/ChromeProcessController.cpp
widget/windows/nsWindow.cpp
widget/windows/nsWindow.h
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -1744,17 +1744,17 @@ TabChild::HandleTap(GeckoContentControll
   case GeckoContentController::TapType::eLongTap:
     if (mGlobal && mTabChildGlobal) {
       mAPZEventState->ProcessLongTap(presShell, point, scale, aModifiers, aGuid,
           aInputBlockId);
     }
     break;
   case GeckoContentController::TapType::eLongTapUp:
     if (mGlobal && mTabChildGlobal) {
-      mAPZEventState->ProcessLongTapUp();
+      mAPZEventState->ProcessLongTapUp(presShell, point, scale, aModifiers);
     }
     break;
   case GeckoContentController::TapType::eSentinel:
     // Should never happen, but we need to handle this case to make the compiler
     // happy.
     MOZ_ASSERT(false);
     break;
   }
--- a/gfx/layers/apz/util/APZEventState.cpp
+++ b/gfx/layers/apz/util/APZEventState.cpp
@@ -253,37 +253,51 @@ APZEventState::ProcessLongTap(const nsCO
 
   nsCOMPtr<nsIWidget> widget = GetWidget();
   if (!widget) {
     return;
   }
 
   SendPendingTouchPreventedResponse(false);
 
+#ifdef XP_WIN
+  // On Windows, we fire the contextmenu events when the user lifts their
+  // finger, in keeping with the platform convention. This happens in the
+  // ProcessLongTapUp function.
+  bool eventHandled = false;
+#else
   bool eventHandled = FireContextmenuEvents(aPresShell, aPoint, aScale,
         aModifiers, widget);
+#endif
   mContentReceivedInputBlockCallback(aGuid, aInputBlockId, eventHandled);
 
   if (eventHandled) {
     // Also send a touchcancel to content, so that listeners that might be
     // waiting for a touchend don't trigger.
     WidgetTouchEvent cancelTouchEvent(true, eTouchCancel, widget.get());
     cancelTouchEvent.mModifiers = aModifiers;
     auto ldPoint = LayoutDeviceIntPoint::Round(aPoint * aScale);
     cancelTouchEvent.mTouches.AppendElement(new mozilla::dom::Touch(mLastTouchIdentifier,
         ldPoint, LayoutDeviceIntPoint(), 0, 0));
     APZCCallbackHelper::DispatchWidgetEvent(cancelTouchEvent);
   }
 }
 
 void
-APZEventState::ProcessLongTapUp()
+APZEventState::ProcessLongTapUp(const nsCOMPtr<nsIPresShell>& aPresShell,
+                                const CSSPoint& aPoint,
+                                const CSSToLayoutDeviceScale& aScale,
+                                Modifiers aModifiers)
 {
-  nsCOMPtr<nsIObserverService> observerService = mozilla::services::GetObserverService();
-  observerService->NotifyObservers(nullptr, "APZ:LongTapUp", nullptr);
+#ifdef XP_WIN
+  nsCOMPtr<nsIWidget> widget = GetWidget();
+  if (widget) {
+    FireContextmenuEvents(aPresShell, aPoint, aScale, aModifiers, widget);
+  }
+#endif
 }
 
 void
 APZEventState::ProcessTouchEvent(const WidgetTouchEvent& aEvent,
                                  const ScrollableLayerGuid& aGuid,
                                  uint64_t aInputBlockId,
                                  nsEventStatus aApzResponse,
                                  nsEventStatus aContentResponse)
--- a/gfx/layers/apz/util/APZEventState.h
+++ b/gfx/layers/apz/util/APZEventState.h
@@ -51,17 +51,20 @@ public:
                         Modifiers aModifiers,
                         const ScrollableLayerGuid& aGuid);
   void ProcessLongTap(const nsCOMPtr<nsIPresShell>& aUtils,
                       const CSSPoint& aPoint,
                       const CSSToLayoutDeviceScale& aScale,
                       Modifiers aModifiers,
                       const ScrollableLayerGuid& aGuid,
                       uint64_t aInputBlockId);
-  void ProcessLongTapUp();
+  void ProcessLongTapUp(const nsCOMPtr<nsIPresShell>& aPresShell,
+                        const CSSPoint& aPoint,
+                        const CSSToLayoutDeviceScale& aScale,
+                        Modifiers aModifiers);
   void ProcessTouchEvent(const WidgetTouchEvent& aEvent,
                          const ScrollableLayerGuid& aGuid,
                          uint64_t aInputBlockId,
                          nsEventStatus aApzResponse,
                          nsEventStatus aContentResponse);
   void ProcessWheelEvent(const WidgetWheelEvent& aEvent,
                          const ScrollableLayerGuid& aGuid,
                          uint64_t aInputBlockId);
--- a/gfx/layers/apz/util/ChromeProcessController.cpp
+++ b/gfx/layers/apz/util/ChromeProcessController.cpp
@@ -192,17 +192,17 @@ ChromeProcessController::HandleTap(TapTy
   case TapType::eDoubleTap:
     HandleDoubleTap(point, aModifiers, aGuid);
     break;
   case TapType::eLongTap:
     mAPZEventState->ProcessLongTap(presShell, point, scale, aModifiers, aGuid,
         aInputBlockId);
     break;
   case TapType::eLongTapUp:
-    mAPZEventState->ProcessLongTapUp();
+    mAPZEventState->ProcessLongTapUp(presShell, point, scale, aModifiers);
     break;
   case TapType::eSentinel:
     // Should never happen, but we need to handle this case branch for the
     // compiler to be happy.
     MOZ_ASSERT(false);
     break;
   }
 }
--- a/widget/windows/nsWindow.cpp
+++ b/widget/windows/nsWindow.cpp
@@ -5200,16 +5200,25 @@ nsWindow::ProcessMessage(UINT msg, WPARA
                          WidgetMouseEvent::eLeftButton,
                          nsIDOMMouseEvent::MOZ_SOURCE_PEN);
       InkCollector::sInkCollector->ClearTarget();
     }
     break;
 
     case WM_CONTEXTMENU:
     {
+      // If the context menu is brought up by a touch long-press, then
+      // the APZ code is responsible for dealing with this, so we don't
+      // need to do anything.
+      if (mTouchWindow && MOUSE_INPUT_SOURCE() == nsIDOMMouseEvent::MOZ_SOURCE_TOUCH) {
+        MOZ_ASSERT(mAPZC); // since mTouchWindow is true, APZ must be enabled
+        result = true;
+        break;
+      }
+
       // if the context menu is brought up from the keyboard, |lParam|
       // will be -1.
       LPARAM pos;
       bool contextMenukey = false;
       if (lParam == -1)
       {
         contextMenukey = true;
         pos = lParamToClient(GetMessagePos());
@@ -7405,16 +7414,23 @@ nsWindow::NeedsToHandleNCActivateDelayed
   // wndproc shouldn't handle it as deactivating. Instead, at receiving
   // WM_ACTIVIATE after that, WM_NCACTIVATE should be sent again manually.
   // This returns true if the window needs to handle WM_NCACTIVATE later.
 
   nsWindow* window = WinUtils::GetNSWindowPtr(aWnd);
   return window && !window->IsPopup();
 }
 
+static bool
+IsTouchSupportEnabled(HWND aWnd)
+{
+  nsWindow* topWindow = WinUtils::GetNSWindowPtr(WinUtils::GetTopLevelHWND(aWnd, true));
+  return topWindow ? topWindow->IsTouchWindow() : false;
+}
+
 // static
 bool
 nsWindow::DealWithPopups(HWND aWnd, UINT aMessage,
                          WPARAM aWParam, LPARAM aLParam, LRESULT* aResult)
 {
   NS_ASSERTION(aResult, "Bad outResult");
 
   // XXX Why do we use the return value of WM_MOUSEACTIVATE for all messages?
@@ -7436,22 +7452,42 @@ nsWindow::DealWithPopups(HWND aWnd, UINT
   static bool sPendingNCACTIVATE = false;
   uint32_t popupsToRollup = UINT32_MAX;
 
   bool consumeRollupEvent = false;
 
   nsWindow* popupWindow = static_cast<nsWindow*>(popup.get());
   UINT nativeMessage = WinUtils::GetNativeMessage(aMessage);
   switch (nativeMessage) {
+    case WM_TOUCH:
+      if (!IsTouchSupportEnabled(aWnd)) {
+        // If APZ is disabled, don't allow touch inputs to dismiss popups. The
+        // compatibility mouse events will do it instead.
+        return false;
+      }
+      MOZ_FALLTHROUGH;
     case WM_LBUTTONDOWN:
     case WM_RBUTTONDOWN:
     case WM_MBUTTONDOWN:
     case WM_NCLBUTTONDOWN:
     case WM_NCRBUTTONDOWN:
     case WM_NCMBUTTONDOWN:
+      if (nativeMessage != WM_TOUCH &&
+          IsTouchSupportEnabled(aWnd) &&
+          MOUSE_INPUT_SOURCE() == nsIDOMMouseEvent::MOZ_SOURCE_TOUCH) {
+        // If any of these mouse events are really compatibility events that
+        // Windows is sending for touch inputs, then don't allow them to dismiss
+        // popups when APZ is enabled (instead we do the dismissing as part of
+        // WM_TOUCH handling which is more correct).
+        // If we don't do this, then when the user lifts their finger after a
+        // long-press, the WM_RBUTTONDOWN compatibility event that Windows sends
+        // us will dismiss the contextmenu popup that we displayed as part of
+        // handling the long-tap-up.
+        return false;
+      }
       if (!EventIsInsideWindow(popupWindow) &&
           GetPopupsToRollup(rollupListener, &popupsToRollup)) {
         break;
       }
       return false;
 
     case WM_MOUSEWHEEL:
     case WM_MOUSEHWHEEL:
--- a/widget/windows/nsWindow.h
+++ b/widget/windows/nsWindow.h
@@ -309,16 +309,17 @@ public:
                    aPosition) override;
   virtual void DefaultProcOfPluginEvent(
                  const mozilla::WidgetPluginEvent& aEvent) override;
   virtual nsresult OnWindowedPluginKeyEvent(
                      const mozilla::NativeEventData& aKeyEventData,
                      nsIKeyEventInPluginCallback* aCallback) override;
 
   void GetCompositorWidgetInitData(mozilla::widget::CompositorWidgetInitData* aInitData) override;
+  bool IsTouchWindow() const { return mTouchWindow; }
 
 protected:
   virtual ~nsWindow();
 
   virtual void WindowUsesOMTC() override;
   virtual void RegisterTouchWindow() override;
 
   // A magic number to identify the FAKETRACKPOINTSCROLLABLE window created