Bug 1746336 - Treat nsEventStatus_eIgnore as INPUT_RESULT_UNHANDLED consistently. r=hiro,geckoview-reviewers,owlish

Differential Revision: https://phabricator.services.mozilla.com/D152351

gfx/layers/apz/public/APZInputBridge.h

@@ -7,11 +7,11 @@
 #ifndef mozilla_layers_APZInputBridge_h
 #define mozilla_layers_APZInputBridge_h
 
+#include "Units.h"                  // for LayoutDeviceIntPoint
 #include "mozilla/EventForwards.h"  // for WidgetInputEvent, nsEventStatus
 #include "mozilla/layers/APZPublicUtils.h"       // for APZWheelAction
 #include "mozilla/layers/LayersTypes.h"          // for ScrollDirections
 #include "mozilla/layers/ScrollableLayerGuid.h"  // for ScrollableLayerGuid
-#include "Units.h"                               // for LayoutDeviceIntPoint
 
 namespace mozilla {
 
@@ -127,7 +127,7 @@ struct APZEventResult {
   }
 
   bool WillHaveDelayedResult() const {
-    return GetStatus() != nsEventStatus_eConsumeNoDefault &&
+    return GetStatus() == nsEventStatus_eConsumeDoDefault &&
            !GetHandledResult();
   }
 

mobile/android/geckoview/src/androidTest/assets/www/scroll.html

@@ -17,18 +17,24 @@ body {
 #one {
    background-color: red;
    width: 200vw;
-   height: 50vh;
+   height: 33vh;
 }
 
 #two {
    background-color: green;
    width: 200vw;
-   height: 50vh;
+   height: 33vh;
 }
 
 #three {
    background-color: blue;
    width: 200vw;
+   height: 33vh;
+}
+
+#four {
+   background-color: purple;
+   width: 200vw;
    height: 200vh;
 }
     </style>
@@ -37,8 +43,14 @@ body {
     <div id="one"></div>
     <div id="two"></div>
     <div id="three"></div>
+    <div id="four"></div>
     <script>
       document.getElementById('two').addEventListener('touchstart', e => {
+        console.log('preventing default');
+        e.preventDefault();
+      });
+
+      document.getElementById('three').addEventListener('touchstart', e => {
         console.log('not preventing default');
       });
     </script>

mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/PanZoomControllerTest.kt

@@ -260,13 +260,15 @@ class PanZoomControllerTest : BaseSessionTest() {
     fun touchEventForResultWithStaticToolbar() {
         setupTouch()
 
-        // No touch handlers, without scrolling
+        // Non-scrollable page: value is always INPUT_RESULT_UNHANDLED
+        
+        // No touch handler
         var value = sessionRule.waitForResult(sendDownEvent(50f, 15f))
         assertThat("Value should match", value, equalTo(PanZoomController.INPUT_RESULT_UNHANDLED))
 
         // Touch handler with preventDefault
         value = sessionRule.waitForResult(sendDownEvent(50f, 45f))
-        assertThat("Value should match", value, equalTo(PanZoomController.INPUT_RESULT_HANDLED_CONTENT))
+        assertThat("Value should match", value, equalTo(PanZoomController.INPUT_RESULT_UNHANDLED))
 
         // Touch handler without preventDefault
         value = sessionRule.waitForResult(sendDownEvent(50f, 75f))
@@ -275,12 +277,18 @@ class PanZoomControllerTest : BaseSessionTest() {
         // move in response to the event.
         assertThat("Value should match", value, equalTo(PanZoomController.INPUT_RESULT_UNHANDLED))
 
-        // No touch handlers, with scrolling
+        // Scrollable page: value depends on the presence and type of touch handler
         setupScroll()
-        value = sessionRule.waitForResult(sendDownEvent(50f, 25f))
+
+        // No touch handler
+        value = sessionRule.waitForResult(sendDownEvent(50f, 15f))
         assertThat("Value should match", value, equalTo(PanZoomController.INPUT_RESULT_HANDLED))
 
-        // Touch handler with scrolling
+        // Touch handler with preventDefault
+        value = sessionRule.waitForResult(sendDownEvent(50f, 45f))
+        assertThat("Value should match", value, equalTo(PanZoomController.INPUT_RESULT_HANDLED_CONTENT))
+
+        // Touch handler without preventDefault
         value = sessionRule.waitForResult(sendDownEvent(50f, 75f))
         assertThat("Value should match", value, equalTo(PanZoomController.INPUT_RESULT_HANDLED))
     }
@@ -411,20 +419,28 @@ class PanZoomControllerTest : BaseSessionTest() {
     fun touchEventForResultWithPreventDefault() {
         sessionRule.display?.run { setDynamicToolbarMaxHeight(20) }
 
-        var files = arrayOf(
-            ROOT_100_PERCENT_HEIGHT_HTML_PATH,
-            ROOT_98VH_HTML_PATH,
-            ROOT_100VH_HTML_PATH,
-            IFRAME_100_PERCENT_HEIGHT_NO_SCROLLABLE_HTML_PATH,
-            IFRAME_100_PERCENT_HEIGHT_SCROLLABLE_HTML_PATH,
-            IFRAME_98VH_SCROLLABLE_HTML_PATH,
-            IFRAME_98VH_NO_SCROLLABLE_HTML_PATH)
-
-        for (file in files) {
+        // Entries are pairs of (filename, pageIsPannable)
+        // Note: "pageIsPannable" means "pannable" in the sense used in
+        // AsyncPanZoomController::ArePointerEventsConsumable().
+        // For example, in iframe_98vh_no_scrollable.html, even though
+        // the page does not have a scroll range, the page is "pannable"
+        // because the dynamic toolbar can be hidden.
+        var entries = arrayOf(
+            Pair(ROOT_100_PERCENT_HEIGHT_HTML_PATH, false),
+            Pair(ROOT_98VH_HTML_PATH, true),
+            Pair(ROOT_100VH_HTML_PATH, true),
+            Pair(IFRAME_100_PERCENT_HEIGHT_NO_SCROLLABLE_HTML_PATH, false),
+            Pair(IFRAME_100_PERCENT_HEIGHT_SCROLLABLE_HTML_PATH, true),
+            Pair(IFRAME_98VH_SCROLLABLE_HTML_PATH, true),
+            Pair(IFRAME_98VH_NO_SCROLLABLE_HTML_PATH, true))
+        for (entry in entries) {
+          var (file, pageIsPannable) = entry;
+          var expected = if (pageIsPannable) PanZoomController.INPUT_RESULT_HANDLED_CONTENT
+                         else PanZoomController.INPUT_RESULT_UNHANDLED;
           setupDocument(file + "?event-prevent")
           var value = sessionRule.waitForResult(sendDownEvent(50f, 50f))
-          assertThat("The input result should be HANDLED_CONTENT in " + file,
-                      value, equalTo(PanZoomController.INPUT_RESULT_HANDLED_CONTENT))
+          assertThat("The input result should be " + expected + " in " + file,
+                      value, equalTo(expected))
 
           // Scroll to the bottom edge if it's possible.
           mainSession.evaluateJS("""
@@ -440,8 +456,8 @@ class PanZoomControllerTest : BaseSessionTest() {
           mainSession.flushApzRepaints()
 
           value = sessionRule.waitForResult(sendDownEvent(50f, 50f))
-          assertThat("The input result should be HANDLED_CONTENT in " + file,
-                      value, equalTo(PanZoomController.INPUT_RESULT_HANDLED_CONTENT))
+          assertThat("The input result should be " + expected + " in " + file,
+                      value, equalTo(expected))
         }
     }
 

widget/android/nsWindow.cpp

@@ -38,6 +38,7 @@
 #include "WidgetUtils.h"
 #include "WindowRenderer.h"
 
+#include "mozilla/EventForwards.h"
 #include "nsAppShell.h"
 #include "nsContentUtils.h"
 #include "nsFocusManager.h"
@@ -851,30 +852,23 @@ class NPZCSupport final
       window->DispatchHitTest(touchEvent);
     });
 
+    if (result.GetStatus() == nsEventStatus_eIgnore) {
+      if (aReturnResult) {
+        aReturnResult->Complete(java::PanZoomController::InputResultDetail::New(
+            INPUT_RESULT_UNHANDLED,
+            java::PanZoomController::SCROLLABLE_FLAG_NONE,
+            java::PanZoomController::OVERSCROLL_FLAG_NONE));
+      }
+      return;
+    }
+
+    MOZ_ASSERT(result.GetStatus() == nsEventStatus_eConsumeDoDefault);
+
     if (aReturnResult && result.GetHandledResult() != Nothing()) {
       // We know conclusively that the root APZ handled this or not and
       // don't need to do any more work.
-      switch (result.GetStatus()) {
-        case nsEventStatus_eIgnore:
-          aReturnResult->Complete(
-              java::PanZoomController::InputResultDetail::New(
-                  INPUT_RESULT_UNHANDLED,
-                  java::PanZoomController::SCROLLABLE_FLAG_NONE,
-                  java::PanZoomController::OVERSCROLL_FLAG_NONE));
-          break;
-        case nsEventStatus_eConsumeDoDefault:
-          aReturnResult->Complete(
-              ConvertAPZHandledResult(result.GetHandledResult().value()));
-          break;
-        default:
-          MOZ_ASSERT_UNREACHABLE("Unexpected nsEventStatus");
-          aReturnResult->Complete(
-              java::PanZoomController::InputResultDetail::New(
-                  INPUT_RESULT_UNHANDLED,
-                  java::PanZoomController::SCROLLABLE_FLAG_NONE,
-                  java::PanZoomController::OVERSCROLL_FLAG_NONE));
-          break;
-      }
+      aReturnResult->Complete(
+          ConvertAPZHandledResult(result.GetHandledResult().value()));
     }
   }
 };