Fix #9614 - Deprecate EngineView#InputResult

This will be replaced by the new InputResultDetail.
mozilla-mobile · Mar 30, 2021 · 71d76ed · 71d76ed
1 parent b4f0f19
commit 71d76ed
Show file tree

Hide file tree

Showing 13 changed files with 45 additions and 3 deletions.
diff --git a/...ngine-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineView.kt b/...ngine-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineView.kt
@@ -93,6 +93,7 @@ class GeckoEngineView @JvmOverloads constructor(
     /**
      * Cache of the last valid input result we got from GeckoView.
      */
+    @Suppress("Deprecation")
     @VisibleForTesting
     internal var lastInputResult: EngineView.InputResult = EngineView.InputResult.INPUT_RESULT_UNHANDLED
 
@@ -174,7 +175,7 @@ class GeckoEngineView @JvmOverloads constructor(
     override fun canScrollVerticallyDown() =
         true // waiting for this issue https://bugzilla.mozilla.org/show_bug.cgi?id=1507569
 
-    @Suppress("MagicNumber")
+    @Suppress("MagicNumber", "Deprecation")
     override fun getInputResult(): EngineView.InputResult {
         // Direct mapping of GeckoView's returned values.
         // If not fail fast to allow for a quick fix.

diff --git a/...e-gecko-beta/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineViewTest.kt b/...e-gecko-beta/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineViewTest.kt
@@ -278,13 +278,15 @@ class GeckoEngineViewTest {
         assertFalse(engineView.canClearSelection())
     }
 
+    @Suppress("Deprecation")
     @Test
     fun `currentInputResult should default to EngineView#InputResult#INPUT_RESULT_UNHANDLED`() {
         val engineView = GeckoEngineView(context)
 
         assertEquals(EngineView.InputResult.INPUT_RESULT_UNHANDLED, engineView.lastInputResult)
     }
 
+    @Suppress("Deprecation")
     @Test
     fun `getInputResult should do a 1-1 mapping of the values received from GeckoView and cache the result`() {
         val engineView = GeckoEngineView(context)
@@ -303,6 +305,7 @@ class GeckoEngineViewTest {
         assertEquals(EngineView.InputResult.INPUT_RESULT_HANDLED_CONTENT, engineView.lastInputResult)
     }
 
+    @Suppress("Deprecation")
     @Test
     fun `INPUT_RESULD_IGNORED should be ignored`() {
         val engineView = GeckoEngineView(context)
@@ -317,6 +320,7 @@ class GeckoEngineViewTest {
         assertEquals(EngineView.InputResult.INPUT_RESULT_HANDLED, engineView.lastInputResult)
     }
 
+    @Suppress("Deprecation")
     @Test(expected = IllegalArgumentException::class)
     fun `Values other than 0, 1, 2, 3 received as input results from GeckoView should throw`() {
         val engineView = GeckoEngineView(context)

diff --git a/...ne-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineView.kt b/...ne-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineView.kt
@@ -93,6 +93,7 @@ class GeckoEngineView @JvmOverloads constructor(
     /**
      * Cache of the last valid input result we got from GeckoView.
      */
+    @Suppress("Deprecation")
     @VisibleForTesting
     internal var lastInputResult: EngineView.InputResult = EngineView.InputResult.INPUT_RESULT_UNHANDLED
 
@@ -174,7 +175,7 @@ class GeckoEngineView @JvmOverloads constructor(
     override fun canScrollVerticallyDown() =
         true // waiting for this issue https://bugzilla.mozilla.org/show_bug.cgi?id=1507569
 
-    @Suppress("MagicNumber")
+    @Suppress("MagicNumber", "Deprecation")
     override fun getInputResult(): EngineView.InputResult {
         // Direct mapping of GeckoView's returned values.
         // If not fail fast to allow for a quick fix.

diff --git a/...ecko-nightly/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineViewTest.kt b/...ecko-nightly/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineViewTest.kt
@@ -278,13 +278,15 @@ class GeckoEngineViewTest {
         assertFalse(engineView.canClearSelection())
     }
 
+    @Suppress("Deprecation")
     @Test
     fun `currentInputResult should default to EngineView#InputResult#INPUT_RESULT_UNHANDLED`() {
         val engineView = GeckoEngineView(context)
 
         assertEquals(EngineView.InputResult.INPUT_RESULT_UNHANDLED, engineView.lastInputResult)
     }
 
+    @Suppress("Deprecation")
     @Test
     fun `getInputResult should do a 1-1 mapping of the values received from GeckoView and cache the result`() {
         val engineView = GeckoEngineView(context)
@@ -303,6 +305,7 @@ class GeckoEngineViewTest {
         assertEquals(EngineView.InputResult.INPUT_RESULT_HANDLED_CONTENT, engineView.lastInputResult)
     }
 
+    @Suppress("Deprecation")
     @Test
     fun `INPUT_RESULD_IGNORED should be ignored`() {
         val engineView = GeckoEngineView(context)
@@ -317,6 +320,7 @@ class GeckoEngineViewTest {
         assertEquals(EngineView.InputResult.INPUT_RESULT_HANDLED, engineView.lastInputResult)
     }
 
+    @Suppress("Deprecation")
     @Test(expected = IllegalArgumentException::class)
     fun `Values other than 0, 1, 2, 3 received as input results from GeckoView should throw`() {
         val engineView = GeckoEngineView(context)

diff --git a/...ser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineView.kt b/...ser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineView.kt
@@ -93,6 +93,7 @@ class GeckoEngineView @JvmOverloads constructor(
     /**
      * Cache of the last valid input result we got from GeckoView.
      */
+    @Suppress("Deprecation")
     @VisibleForTesting
     internal var lastInputResult: EngineView.InputResult = EngineView.InputResult.INPUT_RESULT_UNHANDLED
 
@@ -174,7 +175,7 @@ class GeckoEngineView @JvmOverloads constructor(
     override fun canScrollVerticallyDown() =
         true // waiting for this issue https://bugzilla.mozilla.org/show_bug.cgi?id=1507569
 
-    @Suppress("MagicNumber")
+    @Suppress("MagicNumber", "Deprecation")
     override fun getInputResult(): EngineView.InputResult {
         // Direct mapping of GeckoView's returned values.
         // If not fail fast to allow for a quick fix.

diff --git a/...engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineViewTest.kt b/...engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineViewTest.kt
@@ -278,13 +278,15 @@ class GeckoEngineViewTest {
         assertFalse(engineView.canClearSelection())
     }
 
+    @Suppress("Deprecation")
     @Test
     fun `currentInputResult should default to EngineView#InputResult#INPUT_RESULT_UNHANDLED`() {
         val engineView = GeckoEngineView(context)
 
         assertEquals(EngineView.InputResult.INPUT_RESULT_UNHANDLED, engineView.lastInputResult)
     }
 
+    @Suppress("Deprecation")
     @Test
     fun `getInputResult should do a 1-1 mapping of the values received from GeckoView and cache the result`() {
         val engineView = GeckoEngineView(context)
@@ -303,6 +305,7 @@ class GeckoEngineViewTest {
         assertEquals(EngineView.InputResult.INPUT_RESULT_HANDLED_CONTENT, engineView.lastInputResult)
     }
 
+    @Suppress("Deprecation")
     @Test
     fun `INPUT_RESULD_IGNORED should be ignored`() {
         val engineView = GeckoEngineView(context)
@@ -317,6 +320,7 @@ class GeckoEngineViewTest {
         assertEquals(EngineView.InputResult.INPUT_RESULT_HANDLED, engineView.lastInputResult)
     }
 
+    @Suppress("Deprecation")
     @Test(expected = IllegalArgumentException::class)
     fun `Values other than 0, 1, 2, 3 received as input results from GeckoView should throw`() {
         val engineView = GeckoEngineView(context)

diff --git a/...ser/engine-system/src/main/java/mozilla/components/browser/engine/system/NestedWebView.kt b/...ser/engine-system/src/main/java/mozilla/components/browser/engine/system/NestedWebView.kt
@@ -50,6 +50,7 @@ class NestedWebView(context: Context) : WebView(context), NestedScrollingChild {
      *
      * There must be a 1-1 relation between this values and [EngineView.InputResult]'s.
      */
+    @Suppress("Deprecation")
     internal val inputResult: Int
         get() = getInputResult(eventHandled)
 
@@ -155,6 +156,7 @@ class NestedWebView(context: Context) : WebView(context), NestedScrollingChild {
         return childHelper.dispatchNestedPreFling(velocityX, velocityY)
     }
 
+    @Suppress("Deprecation")
     private fun getInputResult(eventHandled: Boolean): Int {
         return if (eventHandled) {
             EngineView.InputResult.INPUT_RESULT_HANDLED.value

diff --git a/.../engine-system/src/main/java/mozilla/components/browser/engine/system/SystemEngineView.kt b/.../engine-system/src/main/java/mozilla/components/browser/engine/system/SystemEngineView.kt
@@ -694,6 +694,7 @@ class SystemEngineView @JvmOverloads constructor(
 
     override fun canScrollVerticallyDown() = session?.webView?.canScrollVertically(1) ?: false
 
+    @Suppress("Deprecation")
     override fun getInputResult(): EngineView.InputResult {
         (session?.webView as? NestedWebView)?.let { nestedWebView ->
             // Direct mapping of WebView's returned values.

diff --git a/...olbar/src/main/java/mozilla/components/browser/toolbar/behavior/BrowserToolbarBehavior.kt b/...olbar/src/main/java/mozilla/components/browser/toolbar/behavior/BrowserToolbarBehavior.kt
@@ -77,6 +77,7 @@ class BrowserToolbarBehavior(
      * - the website is not scrollable
      * - the website handles the touch events itself through it's own touch event listeners.
      */
+    @Suppress("Deprecation")
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     internal val shouldScroll: Boolean
         get() = engineView?.getInputResult() == EngineView.InputResult.INPUT_RESULT_HANDLED && isScrollEnabled
@@ -232,6 +233,7 @@ class BrowserToolbarBehavior(
             }
         ))
 
+    @Suppress("Deprecation")
     @VisibleForTesting
     internal fun startNestedScroll(axes: Int, type: Int, toolbar: BrowserToolbar): Boolean {
         return if (shouldScroll && axes == ViewCompat.SCROLL_AXIS_VERTICAL) {

diff --git a/...r/src/test/java/mozilla/components/browser/toolbar/behavior/BrowserToolbarBehaviorTest.kt b/...r/src/test/java/mozilla/components/browser/toolbar/behavior/BrowserToolbarBehaviorTest.kt
@@ -302,6 +302,7 @@ class BrowserToolbarBehaviorTest {
         verify(behavior).tryToScrollVertically(-100f)
     }
 
+    @Suppress("Deprecation")
     @Test
     fun `Behaviour shouldScroll if EngineView handled the MotionEvent and scrolling is enabled`() {
         val behavior = BrowserToolbarBehavior(testContext, null, ToolbarPosition.BOTTOM)
@@ -313,6 +314,7 @@ class BrowserToolbarBehaviorTest {
         assertTrue(behavior.shouldScroll)
     }
 
+    @Suppress("Deprecation")
     @Test
     fun `Behaviour !shouldScroll if EngineView handled the MotionEvent and scrolling is !enabled`() {
         val behavior = BrowserToolbarBehavior(testContext, null, ToolbarPosition.BOTTOM)
@@ -324,6 +326,7 @@ class BrowserToolbarBehaviorTest {
         assertFalse(behavior.shouldScroll)
     }
 
+    @Suppress("Deprecation")
     @Test
     fun `Behaviour !shouldScroll if EngineView didn't handle the MotionEvent and scrolling is enabled`() {
         val behavior = BrowserToolbarBehavior(testContext, null, ToolbarPosition.BOTTOM)
@@ -335,6 +338,7 @@ class BrowserToolbarBehaviorTest {
         assertFalse(behavior.shouldScroll)
     }
 
+    @Suppress("Deprecation")
     @Test
     fun `Behaviour !shouldScroll if EngineView didn't handle the MotionEvent and scrolling is !enabled`() {
         val behavior = BrowserToolbarBehavior(testContext, null, ToolbarPosition.BOTTOM)
@@ -346,6 +350,7 @@ class BrowserToolbarBehaviorTest {
         assertFalse(behavior.shouldScroll)
     }
 
+    @Suppress("Deprecation")
     @Test
     fun `Behaviour !shouldScroll if EngineView didn't handle the MotionEvent but the website and scrolling is enabled`() {
         val behavior = BrowserToolbarBehavior(testContext, null, ToolbarPosition.BOTTOM)
@@ -357,6 +362,7 @@ class BrowserToolbarBehaviorTest {
         assertFalse(behavior.shouldScroll)
     }
 
+    @Suppress("Deprecation")
     @Test
     fun `Behaviour !shouldScroll if EngineView didn't handle the MotionEvent but the website and scrolling is !enabled`() {
         val behavior = BrowserToolbarBehavior(testContext, null, ToolbarPosition.BOTTOM)

diff --git a/components/concept/engine/src/main/java/mozilla/components/concept/engine/EngineView.kt b/components/concept/engine/src/main/java/mozilla/components/concept/engine/EngineView.kt
@@ -2,12 +2,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+@file:Suppress("Deprecation")
+
 package mozilla.components.concept.engine
 
 import android.graphics.Bitmap
 import android.view.View
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.OnLifecycleEvent
+import mozilla.components.concept.engine.EngineView.InputResult.INPUT_RESULT_HANDLED
+import mozilla.components.concept.engine.EngineView.InputResult.INPUT_RESULT_HANDLED_CONTENT
+import mozilla.components.concept.engine.EngineView.InputResult.INPUT_RESULT_UNHANDLED
 import mozilla.components.concept.engine.selection.SelectionActionDelegate
 
 /**
@@ -91,8 +96,14 @@ interface EngineView {
     /**
      * @return [InputResult] indicating how user's last [android.view.MotionEvent] was handled.
      */
+    @Deprecated("Not enough data about how the touch was handled", ReplaceWith("getInputResultDetail()"))
     fun getInputResult(): InputResult = InputResult.INPUT_RESULT_UNHANDLED
 
+    /**
+     * @return [InputResultDetail] indicating how user's last [android.view.MotionEvent] was handled.
+     */
+    fun getInputResultDetail(): InputResultDetail = InputResultDetail()
+
     /**
      * Request a screenshot of the visible portion of the web page currently being rendered.
      * @param onFinish A callback to inform that process of capturing a
@@ -134,6 +145,7 @@ interface EngineView {
      * @see [INPUT_RESULT_HANDLED]
      * @see [INPUT_RESULT_HANDLED_CONTENT]
      */
+    @Deprecated("Not enough data about how the touch was handled", ReplaceWith("InputResultDetail"))
     enum class InputResult(val value: Int) {
         /**
          * Last [android.view.MotionEvent] was not handled by neither us nor the webpage.

diff --git a/...s/feature/session/src/main/java/mozilla/components/feature/session/SwipeRefreshFeature.kt b/...s/feature/session/src/main/java/mozilla/components/feature/session/SwipeRefreshFeature.kt
@@ -72,6 +72,7 @@ class SwipeRefreshFeature(
      * If the child view cannot scroll up and the scroll event is not handled by the webpage
      * it means we need to trigger the pull down to refresh functionality.
      */
+    @Suppress("Deprecation")
     override fun canChildScrollUp(parent: SwipeRefreshLayout, child: View?) =
         if (child is EngineView) {
             child.canScrollVerticallyUp() ||

diff --git a/docs/changelog.md b/docs/changelog.md
@@ -12,6 +12,9 @@ permalink: /changelog/
 * [Gecko](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Gecko.kt)
 * [Configuration](https://github.com/mozilla-mobile/android-components/blob/master/.config.yml)
 
+* **concept-engine**
+  * ⚠️ **EngineView#InputResult is deprecated in favor of InputResultDetail which offers more details about how a touch event will be handled.
+
 * **feature-downloads**:
   * 🚒 Bug fixed [issue #9964](https://github.com/mozilla-mobile/android-components/issues/9964) - Downloads notification strings are not localized.