From 7255e3c65c2c0996a7e3aa70c8675de94d337e85 Mon Sep 17 00:00:00 2001 From: rahulsainani Date: Tue, 26 Mar 2024 22:02:38 +0000 Subject: [PATCH] Bug 1887829 - Use icon from content state in tab strip when available r=android-reviewers,007 We discussed this scenario in the [[ https://github.com/mozilla-mobile/firefox-android/pull/5218#discussion_r1499421059 | PR adding tab strip ]] and thought simply Favicon could be used, but turns out there are cases where `ContentState.icon` has the bitmap but `Favicon` doesn't return that from the cache. This still doesn't solve addons.mozilla.org case. Differential Revision: https://phabricator.services.mozilla.com/D205729 --- .../fenix/browser/tabstrip/TabStrip.kt | 31 +++++++++++++++---- .../fenix/browser/tabstrip/TabStripState.kt | 4 +++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/tabstrip/TabStrip.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/tabstrip/TabStrip.kt index 821ccaa710424..5cf4a1d9cd0a2 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/tabstrip/TabStrip.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/tabstrip/TabStrip.kt @@ -4,7 +4,9 @@ package org.mozilla.fenix.browser.tabstrip +import android.graphics.Bitmap import androidx.compose.foundation.ExperimentalFoundationApi +import androidx.compose.foundation.Image import androidx.compose.foundation.background import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement @@ -37,6 +39,7 @@ import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip +import androidx.compose.ui.graphics.asImageBitmap import androidx.compose.ui.res.dimensionResource import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource @@ -297,7 +300,10 @@ private fun TabItem( Spacer(modifier = Modifier.size(8.dp)) - TabStripIcon(state.url) + TabStripIcon( + url = state.url, + icon = state.icon, + ) Spacer(modifier = Modifier.size(8.dp)) @@ -332,17 +338,30 @@ private fun TabItem( } @Composable -private fun TabStripIcon(url: String) { +private fun TabStripIcon( + url: String, + icon: Bitmap?, +) { Box( modifier = Modifier .size(tabStripIconSize) .clip(CircleShape), contentAlignment = Alignment.Center, ) { - Favicon( - url = url, - size = tabStripIconSize, - ) + if (icon != null && !icon.isRecycled) { + Image( + bitmap = icon.asImageBitmap(), + contentDescription = null, + modifier = Modifier + .size(tabStripIconSize) + .clip(CircleShape), + ) + } else { + Favicon( + url = url, + size = tabStripIconSize, + ) + } } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/tabstrip/TabStripState.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/tabstrip/TabStripState.kt index f9bb1afb07da8..57a88d674c123 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/tabstrip/TabStripState.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/tabstrip/TabStripState.kt @@ -4,6 +4,7 @@ package org.mozilla.fenix.browser.tabstrip +import android.graphics.Bitmap import mozilla.components.browser.state.selector.getNormalOrPrivateTabs import mozilla.components.browser.state.selector.selectedTab import mozilla.components.browser.state.state.BrowserState @@ -27,6 +28,7 @@ data class TabStripState( * @property id The id of the tab. * @property title The title of the tab. * @property url The url of the tab. + * @property icon The icon of the tab. * @property isPrivate Whether or not the tab is private. * @property isSelected Whether or not the tab is selected. */ @@ -34,6 +36,7 @@ data class TabStripItem( val id: String, val title: String, val url: String, + val icon: Bitmap? = null, val isPrivate: Boolean, val isSelected: Boolean, ) @@ -56,6 +59,7 @@ internal fun BrowserState.toTabStripState( id = it.id, title = it.content.title.ifBlank { it.content.url }, url = it.content.url, + icon = it.content.icon, isPrivate = it.content.private, isSelected = !isSelectDisabled && it.id == selectedTabId, )