From 6f60a8a44b9fa24b62c223130d39bc5ea8095ae6 Mon Sep 17 00:00:00 2001 From: Mugurell Date: Mon, 17 Jan 2022 19:24:36 +0200 Subject: [PATCH] For #11527 - Allow the decor view time to process the incoming insets There was a race between how insets are applied when entering immersive mode and enabling immersive mode restore by setting an insets listener which is now resolved by ensuring the decor view has the time needed to process the incoming insets, solution recommended on issuetracker https://issuetracker.google.com/u/2/issues/214012501 . Removed the onWindowFocusChangeListener extension property since by having to offer it through a getter the current implementation would always leak the old one. Fenix wasn't using it when this APIs allowed Fenix to pass such a listener and there was no issue observed so there should be no observable negative impact. --- .../support/ktx/android/view/Activity.kt | 24 +++----------- .../support/ktx/android/view/ActivityTest.kt | 32 ++----------------- docs/changelog.md | 3 ++ 3 files changed, 9 insertions(+), 50 deletions(-) diff --git a/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/view/Activity.kt b/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/view/Activity.kt index ded74e7fea5..dc5913aa1ef 100644 --- a/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/view/Activity.kt +++ b/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/view/Activity.kt @@ -7,7 +7,6 @@ package mozilla.components.support.ktx.android.view import android.app.Activity import android.os.Build import android.view.View -import android.view.ViewTreeObserver import android.view.WindowInsets.Type.statusBars import android.view.WindowManager import androidx.annotation.VisibleForTesting @@ -19,8 +18,7 @@ import mozilla.components.support.base.log.logger.Logger * Attempts to enter immersive mode - fullscreen with the status bar and navigation buttons hidden. * This will automatically register and use an * - an inset listener: [View.OnApplyWindowInsetsListener] on API 30+ or - * [View.OnSystemUiVisibilityChangeListener] for below APIs - * - a focus listener: [ViewTreeObserver.OnWindowFocusChangeListener] + * - a system visibility listener: [View.OnSystemUiVisibilityChangeListener] for below APIs. * * to restore immersive mode if interactions with various other widgets like the keyboard or dialogs * got the activity out of immersive mode without [exitImmersiveModeIfNeeded] being called. @@ -47,14 +45,13 @@ internal fun Activity.setAsImmersive() { */ @VisibleForTesting internal fun Activity.enableImmersiveModeRestore() { - window.decorView.viewTreeObserver?.addOnWindowFocusChangeListener(onWindowFocusChangeListener) - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { - window.decorView.setOnApplyWindowInsetsListener { _, insets -> + window.decorView.setOnApplyWindowInsetsListener { view, insets -> if (insets.isVisible(statusBars())) { setAsImmersive() } - insets + // Allow the decor view to have a chance to process the incoming WindowInsets. + view.onApplyWindowInsets(insets) } } else { @Suppress("DEPRECATION") // insets.isVisible(int) is available only starting with API 30 @@ -75,8 +72,6 @@ fun Activity.exitImmersiveModeIfNeeded() { return } - window.decorView.viewTreeObserver?.removeOnWindowFocusChangeListener(onWindowFocusChangeListener) - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { window.decorView.setOnApplyWindowInsetsListener(null) } else { @@ -90,17 +85,6 @@ fun Activity.exitImmersiveModeIfNeeded() { } } -/** - * OnWindowFocusChangeListener used to ensure immersive mode is not broken by other views interactions. - */ -@VisibleForTesting -internal val Activity.onWindowFocusChangeListener: ViewTreeObserver.OnWindowFocusChangeListener - get() = ViewTreeObserver.OnWindowFocusChangeListener { hasFocus -> - if (hasFocus) { - setAsImmersive() - } - } - /** * Calls [Activity.reportFullyDrawn] while also preventing crashes under some circumstances. * diff --git a/components/support/ktx/src/test/java/mozilla/components/support/ktx/android/view/ActivityTest.kt b/components/support/ktx/src/test/java/mozilla/components/support/ktx/android/view/ActivityTest.kt index ac2f466af04..45bbc3994c9 100644 --- a/components/support/ktx/src/test/java/mozilla/components/support/ktx/android/view/ActivityTest.kt +++ b/components/support/ktx/src/test/java/mozilla/components/support/ktx/android/view/ActivityTest.kt @@ -54,7 +54,6 @@ class ActivityTest { verify(decorView).systemUiVisibility = View.SYSTEM_UI_FLAG_FULLSCREEN verify(decorView).systemUiVisibility = View.SYSTEM_UI_FLAG_HIDE_NAVIGATION // verify that the immersive mode restoration is set as expected - verify(window.decorView.viewTreeObserver).addOnWindowFocusChangeListener(any()) verify(window.decorView).setOnSystemUiVisibilityChangeListener(any()) } @@ -65,15 +64,13 @@ class ActivityTest { verify(window).addFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON) verify(decorView).systemUiVisibility = View.SYSTEM_UI_FLAG_FULLSCREEN verify(decorView).systemUiVisibility = View.SYSTEM_UI_FLAG_HIDE_NAVIGATION - verify(window.decorView.viewTreeObserver, never()).addOnWindowFocusChangeListener(any()) verify(window.decorView, never()).setOnSystemUiVisibilityChangeListener(any()) } @Test - fun `check enableImmersiveModeRestore sets focus and insets listeners`() { + fun `check enableImmersiveModeRestore sets insets listeners`() { activity.enableImmersiveModeRestore() - verify(window.decorView.viewTreeObserver).addOnWindowFocusChangeListener(any()) verify(window.decorView).setOnSystemUiVisibilityChangeListener(any()) } @@ -97,26 +94,6 @@ class ActivityTest { verify(decorView).systemUiVisibility = View.SYSTEM_UI_FLAG_HIDE_NAVIGATION } - @Test - fun `check enableImmersiveModeRestore set focus listener has the correct behavior`() { - val focusListenerCaptor = argumentCaptor() - - activity.enableImmersiveModeRestore() - verify(window.decorView.viewTreeObserver).addOnWindowFocusChangeListener(focusListenerCaptor.capture()) - - focusListenerCaptor.value.onWindowFocusChanged(false) - // If the activity is not focused restoration is needed. - // Cannot test if "setAsImmersive()" was called it being an extension function but we can check the effect of that call. - verify(window, never()).addFlags(anyInt()) - verify(decorView, never()).systemUiVisibility = anyInt() - verify(decorView, never()).systemUiVisibility = anyInt() - - focusListenerCaptor.value.onWindowFocusChanged(true) - verify(window).addFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON) - verify(decorView).systemUiVisibility = View.SYSTEM_UI_FLAG_FULLSCREEN - verify(decorView).systemUiVisibility = View.SYSTEM_UI_FLAG_HIDE_NAVIGATION - } - @Test fun `check exitImmersiveModeIfNeeded sets the correct flags`() { val attributes = mock(WindowManager.LayoutParams::class.java) @@ -129,7 +106,6 @@ class ActivityTest { verify(decorView, never()).systemUiVisibility = View.SYSTEM_UI_FLAG_FULLSCREEN.inv() verify(decorView, never()).systemUiVisibility = View.SYSTEM_UI_FLAG_HIDE_NAVIGATION.inv() verify(decorView, never()).setOnSystemUiVisibilityChangeListener(null) - verify(window.decorView.viewTreeObserver, never()).removeOnWindowFocusChangeListener(any()) attributes.flags = WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON @@ -137,13 +113,11 @@ class ActivityTest { verify(window).clearFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON) verify(decorView, times(2)).systemUiVisibility = View.SYSTEM_UI_FLAG_VISIBLE - verify(window.decorView.viewTreeObserver).removeOnWindowFocusChangeListener(any()) verify(decorView).setOnSystemUiVisibilityChangeListener(null) - verify(window.decorView.viewTreeObserver).removeOnWindowFocusChangeListener(any()) } @Test - fun `check exitImmersiveModeIfNeeded correctly cleanups inset and focus listeners`() { + fun `check exitImmersiveModeIfNeeded correctly cleanups the insets listeners`() { val attributes = mock(WindowManager.LayoutParams::class.java) `when`(window.attributes).thenReturn(attributes) attributes.flags = 0 @@ -151,12 +125,10 @@ class ActivityTest { activity.exitImmersiveModeIfNeeded() verify(decorView, never()).setOnSystemUiVisibilityChangeListener(null) - verify(window.decorView.viewTreeObserver, never()).removeOnWindowFocusChangeListener(any()) attributes.flags = WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON activity.exitImmersiveModeIfNeeded() verify(decorView).setOnSystemUiVisibilityChangeListener(null) - verify(window.decorView.viewTreeObserver).removeOnWindowFocusChangeListener(any()) } } diff --git a/docs/changelog.md b/docs/changelog.md index e73931dfa78..426b354854f 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -11,6 +11,9 @@ permalink: /changelog/ * [Gecko](https://github.com/mozilla-mobile/android-components/blob/main/buildSrc/src/main/java/Gecko.kt) * [Configuration](https://github.com/mozilla-mobile/android-components/blob/main/.config.yml) +* **support-ktx** + * 🚒 Bug fixed [issue #11527](https://github.com/mozilla-mobile/android-components/issues/11527) - Fix some situations in which the immersive mode wasn't properly applied. + * **lib/publicsuffixlist** * ⚠️ **This is a breaking change**: Removed `String.urlToTrimmedHost` extension method.