Bug 1298908 - On Windows, fire the contextmenu and long-tap events on the long-tap-up user action. r?botond,jimm draft
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 30 Aug 2016 17:32:08 -0400
changeset 407724 8cc3a15748a06887f0d40ef9cf24f58091d88f9f
parent 407723 5e6bb095e129bbceaca13a30ac65cccac18bbebe
child 529932 fb30815183b08b959bd3aea839418faa7233c9f2
push id28026
push userkgupta@mozilla.com
push dateTue, 30 Aug 2016 21:32:39 +0000
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