From 65a722e6e1b279e926aecb7dcb5c5d05865d1bb6 Mon Sep 17 00:00:00 2001 From: Botond Ballo Date: Thu, 18 Aug 2022 02:07:14 +0000 Subject: [PATCH] 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 | 4 +- .../src/androidTest/assets/www/scroll.html | 16 +++++- .../geckoview/test/PanZoomControllerTest.kt | 54 ++++++++++++------- widget/android/nsWindow.cpp | 36 ++++++------- 4 files changed, 66 insertions(+), 44 deletions(-) diff --git a/gfx/layers/apz/public/APZInputBridge.h b/gfx/layers/apz/public/APZInputBridge.h index 16ab53e6356f5..b719984b6c047 100644 --- a/gfx/layers/apz/public/APZInputBridge.h +++ b/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(); } diff --git a/mobile/android/geckoview/src/androidTest/assets/www/scroll.html b/mobile/android/geckoview/src/androidTest/assets/www/scroll.html index 9c92eae8393a1..6d98574f9c3bf 100644 --- a/mobile/android/geckoview/src/androidTest/assets/www/scroll.html +++ b/mobile/android/geckoview/src/androidTest/assets/www/scroll.html @@ -17,18 +17,24 @@ #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; } @@ -37,8 +43,14 @@
+
diff --git a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/PanZoomControllerTest.kt b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/PanZoomControllerTest.kt index 57110374676d7..be10dc577a1c9 100644 --- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/PanZoomControllerTest.kt +++ b/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)) } } diff --git a/widget/android/nsWindow.cpp b/widget/android/nsWindow.cpp index c2248280e3de1..c550e2dc5d4a5 100644 --- a/widget/android/nsWindow.cpp +++ b/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())); } } };