commit f2e5df085bd16703e6fba9ba3650dd9c68615148
parent 9ea64c60a894af58cd8ec143876a2b5f9c4dc8c4
Author: Julie De Lorenzo <jdelorenzo@mozilla.com>
Date: Fri, 7 Nov 2025 21:58:30 +0000
Bug 1998612: Fix unintended tint of WebExtensionListItem by separating Color params r=android-reviewers,007
Differential Revision: https://phabricator.services.mozilla.com/D271821
Diffstat:
4 files changed, 132 insertions(+), 8 deletions(-)
diff --git a/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/components/MenuItemTest.kt b/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/components/MenuItemTest.kt
@@ -0,0 +1,69 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+package org.mozilla.fenix.components
+
+import android.graphics.Bitmap
+import androidx.compose.foundation.background
+import androidx.compose.foundation.layout.Column
+import androidx.compose.foundation.layout.padding
+import androidx.compose.material3.MaterialTheme
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.graphics.Color
+import androidx.compose.ui.graphics.asAndroidBitmap
+import androidx.compose.ui.platform.testTag
+import androidx.compose.ui.res.painterResource
+import androidx.compose.ui.test.captureToImage
+import androidx.compose.ui.test.hasTestTag
+import androidx.compose.ui.test.junit4.createComposeRule
+import androidx.compose.ui.unit.dp
+import mozilla.components.compose.base.theme.surfaceDimVariant
+import org.junit.Assert.assertFalse
+import org.junit.Rule
+import org.junit.Test
+import org.mozilla.fenix.compose.list.IconListItem
+import mozilla.components.ui.icons.R as iconsR
+
+class MenuItemTest {
+ @get:Rule
+ val composeTestRule = createComposeRule()
+
+ @Test
+ fun testIconListItemIconUntintedArgumentRespected() {
+ with(composeTestRule) {
+ setContent {
+ Column(Modifier.background(color = MaterialTheme.colorScheme.surfaceDimVariant).padding(16.dp)) {
+ IconListItem(
+ label = "Test Label",
+ beforeIconPainter = painterResource(iconsR.drawable.mozac_ic_shield_slash_critical_24),
+ beforeIconTint = Color.Unspecified,
+ enabled = true,
+ modifier = Modifier.testTag("NoTint"),
+ )
+ IconListItem(
+ label = "Test Label",
+ beforeIconPainter = painterResource(iconsR.drawable.mozac_ic_shield_slash_critical_24),
+ enabled = true,
+ modifier = Modifier.testTag("DefaultTint"),
+ )
+ }
+ }
+ val untintedIconListItem = onNode(hasTestTag("NoTint"), useUnmergedTree = true).captureToImage().asAndroidBitmap()
+ val defaultTintedIconListItem = onNode(hasTestTag("DefaultTint"), useUnmergedTree = true).captureToImage().asAndroidBitmap()
+
+ assertFalse(untintedIconListItem.sameColorsAs(defaultTintedIconListItem))
+ }
+ }
+
+ fun Bitmap.sameColorsAs(other: Bitmap): Boolean {
+ for (x in 0 until this.width) {
+ for (y in 0 until this.height) {
+ if (this.getColor(x, y) != other.getColor(x, y)) {
+ return false
+ }
+ }
+ }
+ return true
+ }
+}
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarksScreen.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarksScreen.kt
@@ -990,9 +990,9 @@ private fun NewFolderListItem(onClick: () -> Unit) {
modifier = Modifier.width(FirefoxTheme.layout.size.containerMaxWidth),
colors = ListItemDefaults.colors(
headlineColor = MaterialTheme.colorScheme.tertiary,
- leadingIconColor = MaterialTheme.colorScheme.tertiary,
),
beforeIconPainter = painterResource(iconsR.drawable.mozac_ic_folder_add_24),
+ beforeIconTint = MaterialTheme.colorScheme.tertiary,
onClick = onClick,
)
}
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/menu/compose/MenuItem.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/menu/compose/MenuItem.kt
@@ -145,8 +145,6 @@ internal fun MenuItem(
colors = ListItemDefaults.colors(
headlineColor = labelTextColor,
supportingColor = descriptionTextColor,
- leadingIconColor = iconTint,
- trailingIconColor = iconTint,
),
maxLabelLines = 2,
description = description,
@@ -160,10 +158,12 @@ internal fun MenuItem(
onClick = onClick,
beforeIconPainter = beforeIconPainter,
beforeIconDescription = beforeIconDescription,
+ beforeIconTint = iconTint,
isBeforeIconHighlighted = isBeforeIconHighlighted,
showDivider = showDivider,
afterIconPainter = afterIconPainter,
afterIconDescription = afterIconDescription,
+ afterIconTint = iconTint,
onAfterIconClick = onAfterIconClick,
afterListAction = afterContent,
)
@@ -232,7 +232,7 @@ internal fun WebExtensionMenuItem(
IconListItem(
label = label,
enabled = enabled == true,
- colors = ListItemDefaults.colors(leadingIconColor = iconTint),
+ beforeIconTint = iconTint,
beforeIconPainter = iconPainter,
onClick = onClick,
modifier = Modifier
@@ -449,6 +449,16 @@ private fun WebExtensionMenuItemPreview() {
onClick = {},
onSettingsClick = {},
)
+ // Web extensions may have multi-colored assets with no tint.
+ WebExtensionMenuItem(
+ label = "colorful icon",
+ iconPainter = painterResource(iconsR.drawable.mozac_ic_shield_slash_critical_24),
+ iconTint = Color.Unspecified,
+ enabled = true,
+ badgeText = "17",
+ onClick = {},
+ onSettingsClick = {},
+ )
}
}
}
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/compose/list/ListItem.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/compose/list/ListItem.kt
@@ -2,6 +2,8 @@
* 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("TooManyFunctions")
+
package org.mozilla.fenix.compose.list
import android.content.res.Configuration
@@ -63,6 +65,7 @@ import androidx.compose.ui.tooling.preview.PreviewParameterProvider
import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.dp
import mozilla.components.compose.base.modifier.thenConditional
+import mozilla.components.compose.base.theme.surfaceDimVariant
import org.mozilla.fenix.compose.Favicon
import org.mozilla.fenix.compose.button.RadioButton
import org.mozilla.fenix.theme.FirefoxTheme
@@ -256,11 +259,17 @@ fun FaviconListItem(
* @param minHeight An optional minimum height for the list item.
* @param onClick Called when the user clicks on the item.
* @param onLongClick Called when the user long clicks on the item.
+ * @param beforeIconTint [Color] used to tint the icon. Note: Color.Unspecified is used when you
+ * wish to preserve the original colors of the icon. This color should NOT be combined with
+ * ListItemColors because ListItemDefaults will not allow you to specify Color.Unspecified.
* @param beforeIconPainter [Painter] used to display an [Icon] before the list item.
* @param beforeIconDescription Content description of the icon.
* @param isBeforeIconHighlighted Whether or not the item should be highlighted with a notification icon.
* @param showDivider Whether or not to display a vertical divider line before the [IconButton]
* at the end.
+ * @param afterIconTint [Color] used to tint the icon. Note: Color.Unspecified is used when you
+ * wish to preserve the original colors of the icon. This color should NOT be combined with
+ * ListItemColors because ListItemDefaults will not allow you to specify Color.Unspecified.
* @param afterIconPainter [Painter] used to display an icon after the list item.
* @param afterIconDescription Content description of the icon.
* @param onAfterIconClick Called when the user clicks on the icon. An [IconButton] will be
@@ -281,10 +290,12 @@ fun IconListItem(
minHeight: Dp = LIST_ITEM_HEIGHT,
onClick: (() -> Unit)? = null,
onLongClick: (() -> Unit)? = null,
+ beforeIconTint: Color = ListItemDefaults.colors().leadingIconColor,
beforeIconPainter: Painter,
beforeIconDescription: String? = null,
isBeforeIconHighlighted: Boolean = false,
showDivider: Boolean = false,
+ afterIconTint: Color = ListItemDefaults.colors().leadingIconColor,
afterIconPainter: Painter? = null,
afterIconDescription: String? = null,
onAfterIconClick: (() -> Unit)? = null,
@@ -308,7 +319,7 @@ fun IconListItem(
isHighlighted = isBeforeIconHighlighted,
painter = beforeIconPainter,
description = beforeIconDescription,
- tint = if (enabled) colors.leadingIconColor else colors.disabledLeadingIconColor,
+ tint = if (enabled) beforeIconTint else colors.disabledLeadingIconColor,
)
},
afterListItemAction = {
@@ -316,7 +327,7 @@ fun IconListItem(
enabled = enabled,
painter = afterIconPainter,
description = afterIconDescription,
- tint = if (enabled) colors.trailingIconColor else colors.disabledTrailingIconColor,
+ tint = if (enabled) afterIconTint else colors.disabledTrailingIconColor,
onClick = onAfterIconClick,
listAction = afterListAction,
showDivider = showDivider,
@@ -326,6 +337,21 @@ fun IconListItem(
}
@Composable
+@PreviewLightDark
+private fun IconListItemBeforeIconPreview() {
+ FirefoxTheme {
+ Box(Modifier.background(MaterialTheme.colorScheme.surfaceDimVariant)) {
+ IconListItemBeforeIcon(
+ isHighlighted = false,
+ painter = painterResource(iconsR.drawable.mozac_ic_shield_slash_critical_24),
+ description = "",
+ tint = Color.Unspecified,
+ )
+ }
+ }
+}
+
+@Composable
private fun IconListItemBeforeIcon(
isHighlighted: Boolean,
painter: Painter,
@@ -618,10 +644,16 @@ fun SelectableFaviconListItem(
* @param minHeight An optional minimum height for the list item.
* @param onClick Called when the user clicks on the item.
* @param onLongClick Called when the user long clicks on the item.
+ * @param beforeIconTint [Color] used to tint the icon. Note: Color.Unspecified is used when you
+ * wish to preserve the original colors of the icon. This color should NOT be combined with
+ * ListItemColors because ListItemDefaults will not allow you to specify Color.Unspecified.
* @param beforeIconPainter [Painter] used to display an [Icon] before the list item.
* @param beforeIconDescription Content description of the icon.
* @param showDivider Whether or not to display a vertical divider line before the [IconButton]
* at the end.
+ * @param afterIconTint [Color] used to tint the icon. Note: Color.Unspecified is used when you
+ * wish to preserve the original colors of the icon. This color should NOT be combined with
+ * ListItemColors because ListItemDefaults will not allow you to specify Color.Unspecified.
* @param afterIconPainter [Painter] used to display an icon after the list item.
* @param afterIconDescription Content description of the icon.
* @param onAfterIconClick Called when the user clicks on the icon. An [IconButton] will be
@@ -643,9 +675,11 @@ fun SelectableIconListItem(
minHeight: Dp = LIST_ITEM_HEIGHT,
onClick: (() -> Unit)? = null,
onLongClick: (() -> Unit)? = null,
+ beforeIconTint: Color = ListItemDefaults.colors().leadingIconColor,
beforeIconPainter: Painter,
beforeIconDescription: String? = null,
showDivider: Boolean = false,
+ afterIconTint: Color = ListItemDefaults.colors().trailingIconColor,
afterIconPainter: Painter? = null,
afterIconDescription: String? = null,
onAfterIconClick: (() -> Unit)? = null,
@@ -670,7 +704,7 @@ fun SelectableIconListItem(
Icon(
painter = beforeIconPainter,
contentDescription = beforeIconDescription,
- tint = if (enabled) colors.leadingIconColor else colors.disabledLeadingIconColor,
+ tint = if (enabled) beforeIconTint else colors.disabledLeadingIconColor,
)
},
)
@@ -680,7 +714,7 @@ fun SelectableIconListItem(
return@ListItem
}
- val tint = if (enabled) colors.trailingIconColor else colors.disabledTrailingIconColor
+ val tint = if (enabled) afterIconTint else colors.disabledTrailingIconColor
if (showDivider) {
VerticalDivider()
@@ -1064,6 +1098,17 @@ private fun IconListItemPreview() {
afterIconPainter = painterResource(iconsR.drawable.mozac_ic_chevron_right_24),
afterIconDescription = null,
)
+
+ IconListItem(
+ label = "Colorful icon list item",
+ enabled = true,
+ onClick = {},
+ beforeIconTint = Color.Unspecified,
+ beforeIconPainter = painterResource(iconsR.drawable.mozac_ic_shield_slash_critical_24),
+ beforeIconDescription = "click me",
+ afterIconPainter = painterResource(iconsR.drawable.mozac_ic_chevron_right_24),
+ afterIconDescription = null,
+ )
}
}
}
