2
0
mirror of https://github.com/fork-maintainers/iceraven-browser synced 2024-11-19 09:25:34 +00:00

[fenix] Bug 1804107 - Fix for BaseBrowserFragment toolbar getter null crash and invalidating actions when modifying toolbar

This commit is contained in:
Zac McKenney 2022-12-06 10:42:08 -08:00 committed by mergify[bot]
parent 663b50a29a
commit aed86b9d30
3 changed files with 43 additions and 10 deletions

View File

@ -33,7 +33,6 @@ import com.google.android.material.snackbar.Snackbar
import kotlinx.coroutines.Dispatchers.IO
import kotlinx.coroutines.Dispatchers.Main
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.mapNotNull
import kotlinx.coroutines.launch
@ -1400,6 +1399,11 @@ abstract class BaseBrowserFragment :
binding.swipeRefresh.isEnabled = shouldPullToRefreshBeEnabled(inFullScreen)
}
@CallSuper
internal open fun onUpdateToolbarForConfigurationChange(toolbar: BrowserToolbarView) {
toolbar.dismissMenu()
}
/*
* Dereference these views when the fragment view is destroyed to prevent memory leaks
*/
@ -1472,7 +1476,9 @@ abstract class BaseBrowserFragment :
override fun onConfigurationChanged(newConfig: Configuration) {
super.onConfigurationChanged(newConfig)
_browserToolbarView?.dismissMenu()
_browserToolbarView?.let {
onUpdateToolbarForConfigurationChange(it)
}
}
// This method is called in response to native web extension messages from

View File

@ -5,7 +5,6 @@
package org.mozilla.fenix.browser
import android.content.Context
import android.content.res.Configuration
import android.os.StrictMode
import android.view.View
import android.view.ViewGroup
@ -32,6 +31,7 @@ import org.mozilla.fenix.GleanMetrics.ReaderMode
import org.mozilla.fenix.R
import org.mozilla.fenix.components.FenixSnackbar
import org.mozilla.fenix.components.TabCollectionStorage
import org.mozilla.fenix.components.toolbar.BrowserToolbarView
import org.mozilla.fenix.components.toolbar.ToolbarMenu
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.nav
@ -90,7 +90,7 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
browserToolbarView.view.addNavigationAction(homeAction)
setScreenSize(isTablet = resources.getBoolean(R.bool.tablet))
updateToolbarActions(isTablet = resources.getBoolean(R.bool.tablet))
val readerModeAction =
BrowserToolbar.ToggleButton(
@ -170,7 +170,14 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
}
}
private fun setScreenSize(isTablet: Boolean) {
override fun onUpdateToolbarForConfigurationChange(toolbar: BrowserToolbarView) {
super.onUpdateToolbarForConfigurationChange(toolbar)
updateToolbarActions(isTablet = resources.getBoolean(R.bool.tablet))
}
@VisibleForTesting
internal fun updateToolbarActions(isTablet: Boolean) {
if (isTablet == this.isTablet) return
if (isTablet) {
@ -280,6 +287,8 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
refreshAction?.let {
browserToolbarView.view.addNavigationAction(it)
}
browserToolbarView.view.invalidateActions()
}
private fun removeTabletActions() {
@ -292,6 +301,8 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
refreshAction?.let {
browserToolbarView.view.removeNavigationAction(it)
}
browserToolbarView.view.invalidateActions()
}
override fun onStart() {
@ -447,9 +458,4 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
internal fun updateLastBrowseActivity() {
requireContext().settings().lastBrowseActivity = System.currentTimeMillis()
}
override fun onConfigurationChanged(newConfig: Configuration) {
super.onConfigurationChanged(newConfig)
setScreenSize(isTablet = resources.getBoolean(R.bool.tablet))
}
}

View File

@ -312,6 +312,21 @@ class BrowserFragmentTest {
verify(exactly = 1) { toolbarIntegration.invalidateMenu() }
}
@Test
fun `WHEN toolbar is initialized THEN onConfigurationChanged sets toolbar actions for size in fragment`() {
val browserToolbarView: BrowserToolbarView = mockk(relaxed = true)
browserFragment._browserToolbarView = null
browserFragment.onConfigurationChanged(mockk(relaxed = true))
verify(exactly = 0) { browserFragment.onUpdateToolbarForConfigurationChange(any()) }
verify(exactly = 0) { browserFragment.updateToolbarActions(any()) }
browserFragment._browserToolbarView = browserToolbarView
browserFragment.onConfigurationChanged(mockk(relaxed = true))
verify(exactly = 1) { browserFragment.onUpdateToolbarForConfigurationChange(any()) }
verify(exactly = 1) { browserFragment.updateToolbarActions(any()) }
}
@Test
fun `WHEN fragment configuration changed THEN menu is dismissed`() {
val browserToolbarView: BrowserToolbarView = mockk(relaxed = true)
@ -325,7 +340,9 @@ class BrowserFragmentTest {
@Test
fun `WHEN fragment configuration screen size changes between tablet and mobile size THEN tablet action items added and removed`() {
val browserToolbarView: BrowserToolbarView = mockk(relaxed = true)
val browserToolbar: BrowserToolbar = mockk(relaxed = true)
browserFragment._browserToolbarView = browserToolbarView
every { browserFragment.browserToolbarView.view } returns browserToolbar
mockkObject(ThemeManager.Companion)
@ -348,7 +365,9 @@ class BrowserFragmentTest {
@Test
fun `WHEN fragment configuration change enables tablet size twice THEN tablet action items are only added once`() {
val browserToolbarView: BrowserToolbarView = mockk(relaxed = true)
val browserToolbar: BrowserToolbar = mockk(relaxed = true)
browserFragment._browserToolbarView = browserToolbarView
every { browserFragment.browserToolbarView.view } returns browserToolbar
mockkObject(ThemeManager.Companion)
@ -370,7 +389,9 @@ class BrowserFragmentTest {
@Test
fun `WHEN fragment configuration change sets mobile size twice THEN tablet action items are not added or removed`() {
val browserToolbarView: BrowserToolbarView = mockk(relaxed = true)
val browserToolbar: BrowserToolbar = mockk(relaxed = true)
browserFragment._browserToolbarView = browserToolbarView
every { browserFragment.browserToolbarView.view } returns browserToolbar
mockkObject(ThemeManager.Companion)