From 4eaf4d5834bede7665814a84b54bba6d23de4a81 Mon Sep 17 00:00:00 2001 From: Mugurell Date: Wed, 12 Jan 2022 17:04:35 +0200 Subject: [PATCH] For #11527 - Use deprecated APIs for immersive mode since they work better Opened https://issuetracker.google.com/u/2/issues/214012501 for what seems a bug with the new insets based APIs. In the meantime we are using the old OnSystemUiVisibilityChangeListener to know when to restore immersive mode. 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 | 53 ++++--------------- .../support/ktx/android/view/ActivityTest.kt | 28 ---------- docs/changelog.md | 2 + 3 files changed, 13 insertions(+), 70 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..3e7cd082d59 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 @@ -5,10 +5,7 @@ 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 import androidx.core.view.WindowInsetsCompat @@ -17,11 +14,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] - * + * This will automatically register and use a [View.OnSystemUiVisibilityChangeListener] * 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,21 +40,12 @@ 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 -> - if (insets.isVisible(statusBars())) { - setAsImmersive() - } - insets - } - } else { - @Suppress("DEPRECATION") // insets.isVisible(int) is available only starting with API 30 - window.decorView.setOnSystemUiVisibilityChangeListener { newFlags -> - if (newFlags and View.SYSTEM_UI_FLAG_FULLSCREEN == 0) { - setAsImmersive() - } + // Still using the now deprecated approach until a resolution of https://issuetracker.google.com/issues/214012501 + // Previous approach based on which the bug was discovered is available in history. + @Suppress("DEPRECATION") + window.decorView.setOnSystemUiVisibilityChangeListener { newFlags -> + if (newFlags and View.SYSTEM_UI_FLAG_FULLSCREEN == 0) { + setAsImmersive() } } } @@ -75,14 +59,10 @@ fun Activity.exitImmersiveModeIfNeeded() { return } - window.decorView.viewTreeObserver?.removeOnWindowFocusChangeListener(onWindowFocusChangeListener) - - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { - window.decorView.setOnApplyWindowInsetsListener(null) - } else { - @Suppress("DEPRECATION") - window.decorView.setOnSystemUiVisibilityChangeListener(null) - } + // Still using the now deprecated approach until a resolution of https://issuetracker.google.com/issues/214012501 + // Previous approach based on which the bug was discovered is available in history. + @Suppress("DEPRECATION") + window.decorView.setOnSystemUiVisibilityChangeListener(null) window.clearFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON) window.getWindowInsetsController().apply { @@ -90,17 +70,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..7659be888fc 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,7 +64,6 @@ 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()) } @@ -73,7 +71,6 @@ class ActivityTest { fun `check enableImmersiveModeRestore sets focus and 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,9 +113,7 @@ 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 @@ -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 0ad00cacacf..4d1242da9fd 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -11,6 +11,8 @@ 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) - Using now deprecated APIs to fix immersive mode not being applied all the time. https://issuetracker.google.com/u/2/issues/214012501 can be followed for what seems a framework issue. # 97.0.0 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v96.0.0...v97.0.0)