From 2bdf50d1ff6f43d7639d4df2185b2c2118e2a25d Mon Sep 17 00:00:00 2001 From: Mike Penz Date: Thu, 20 Aug 2026 18:29:29 +0200 Subject: [PATCH 1/2] fix(core): don't overwrite existing icons when no ico_ attribute is set IconicsAttrsApplier.getIconicsDrawable() always returned a drawable, even for an AttributeSet carrying no `ico_*` attribute at all, because the extractor falls back to creating an empty IconicsDrawable. Every caller treats a non-null result as "there is iconics data here" and assigns it, so a menu item's `android:icon` or an ImageView's `android:src` got replaced by an empty drawable. Return null when the styled attributes are empty, which is what all three call sites (IconicsMenuInflaterUtil, and the ActionMenuItemView / ImageView branches of IconicsFactory) already handle. Adds regression tests over `menu_playground`, which covers an item with a plain `android:icon`, an item defined via `ico_*` attributes and an item with no icon at all. Fixes #666 Supersedes #667 Co-authored-by: PrOF-kk --- app/build.gradle | 8 ++ app/src/main/res/menu/menu_playground.xml | 6 ++ .../sample/IconicsMenuInflaterUtilTest.kt | 76 +++++++++++++++++++ .../iconics/context/IconicsAttrsApplier.kt | 5 ++ 4 files changed, 95 insertions(+) create mode 100644 app/src/test/java/com/mikepenz/iconics/sample/IconicsMenuInflaterUtilTest.kt diff --git a/app/build.gradle b/app/build.gradle index 6293e3ee..016a0801 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -69,6 +69,12 @@ android { abortOnError false } + testOptions { + unitTests { + includeAndroidResources = true + } + } + packagingOptions { exclude 'META-INF/library-core_release.kotlin_module' exclude 'META-INF/library_release.kotlin_module' @@ -139,4 +145,6 @@ dependencies { implementation project(':weather-icons-typeface-library') testImplementation 'junit:junit:4.13.2' + testImplementation 'org.robolectric:robolectric:4.16' + testImplementation 'androidx.test:core:1.7.0' } diff --git a/app/src/main/res/menu/menu_playground.xml b/app/src/main/res/menu/menu_playground.xml index 7ad01d40..8c0b7af5 100644 --- a/app/src/main/res/menu/menu_playground.xml +++ b/app/src/main/res/menu/menu_playground.xml @@ -31,4 +31,10 @@ app:ico_color="@android:color/holo_blue_bright" app:ico_icon="gmd_star" app:ico_size="24dp" /> + + + diff --git a/app/src/test/java/com/mikepenz/iconics/sample/IconicsMenuInflaterUtilTest.kt b/app/src/test/java/com/mikepenz/iconics/sample/IconicsMenuInflaterUtilTest.kt new file mode 100644 index 00000000..a8abf767 --- /dev/null +++ b/app/src/test/java/com/mikepenz/iconics/sample/IconicsMenuInflaterUtilTest.kt @@ -0,0 +1,76 @@ +/* + * Copyright (c) 2026 Mike Penz + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.mikepenz.iconics.sample + +import android.app.Activity +import android.view.Menu +import androidx.appcompat.view.menu.MenuBuilder +import com.mikepenz.iconics.IconicsDrawable +import com.mikepenz.iconics.utils.IconicsMenuInflaterUtil +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.Robolectric +import org.robolectric.RobolectricTestRunner + +/** + * `menu_playground` covers the three relevant cases: + * - `menu_item_1` has a plain `android:icon` and no `ico_*` attributes + * - `menu_item_2` is defined via `ico_*` attributes + * - `menu_item_3` has no icon at all + */ +@RunWith(RobolectricTestRunner::class) +class IconicsMenuInflaterUtilTest { + + private lateinit var activity: Activity + private lateinit var menu: Menu + + @Before fun setUp() { + activity = Robolectric.buildActivity(PlaygroundActivity::class.java).get() + menu = MenuBuilder(activity) + } + + @Test fun `iconics item gets an IconicsDrawable`() { + IconicsMenuInflaterUtil.inflate(activity.menuInflater, activity, R.menu.menu_playground, menu) + + assertTrue(menu.findItem(R.id.menu_item_2).icon is IconicsDrawable) + } + + @Test fun `non-iconics item keeps its android icon`() { + activity.menuInflater.inflate(R.menu.menu_playground, menu) + val original = menu.findItem(R.id.menu_item_1).icon + assertNotNull("menu_item_1 is expected to declare an android:icon", original) + + IconicsMenuInflaterUtil.parseXmlAndSetIconicsDrawables(activity, R.menu.menu_playground, menu) + + assertSame( + "an item without ico_* attributes must keep the drawable of its android:icon", + original, + menu.findItem(R.id.menu_item_1).icon + ) + } + + @Test fun `item without any icon stays without one`() { + IconicsMenuInflaterUtil.inflate(activity.menuInflater, activity, R.menu.menu_playground, menu) + + assertNull(menu.findItem(R.id.menu_item_3).icon) + } +} diff --git a/iconics-core/src/main/java/com/mikepenz/iconics/context/IconicsAttrsApplier.kt b/iconics-core/src/main/java/com/mikepenz/iconics/context/IconicsAttrsApplier.kt index b337a9fd..b51316ae 100644 --- a/iconics-core/src/main/java/com/mikepenz/iconics/context/IconicsAttrsApplier.kt +++ b/iconics-core/src/main/java/com/mikepenz/iconics/context/IconicsAttrsApplier.kt @@ -45,6 +45,11 @@ object IconicsAttrsApplier { @JvmStatic fun getIconicsDrawable(res: Resources, theme: Theme?, attrs: AttributeSet?): IconicsDrawable? { return theme?.obtainStyledAttributes(attrs, R.styleable.Iconics, 0, 0)?.use { + // no `ico_*` attribute at all -> there is nothing to build an icon from. Returning an + // empty drawable here would overwrite whatever icon the target already has, e.g. the + // `android:icon` of a menu item or the `android:src` of an `ImageView`. + if (it.indexCount == 0) return@use null + IconicsAttrsExtractor( res = res, theme = theme, From ae354b75fb9d045d9c599180d16ab6d09f48d1c8 Mon Sep 17 00:00:00 2001 From: Mike Penz Date: Thu, 20 Aug 2026 18:34:06 +0200 Subject: [PATCH 2/2] refactor(core): drop the ico_ prefix check in the menu inflater Superseded by the null return in IconicsAttrsApplier: matching attribute names by their `ico_` prefix breaks if an attribute is ever renamed, and it only covered the menu inflater while IconicsFactory had the same problem. --- .../mikepenz/iconics/utils/IconicsMenuInflaterUtil.kt | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/iconics-core/src/main/java/com/mikepenz/iconics/utils/IconicsMenuInflaterUtil.kt b/iconics-core/src/main/java/com/mikepenz/iconics/utils/IconicsMenuInflaterUtil.kt index b2d94762..651c6711 100644 --- a/iconics-core/src/main/java/com/mikepenz/iconics/utils/IconicsMenuInflaterUtil.kt +++ b/iconics-core/src/main/java/com/mikepenz/iconics/utils/IconicsMenuInflaterUtil.kt @@ -168,18 +168,10 @@ object IconicsMenuInflaterUtil { menu: Menu ) { val attrsMap = mutableMapOf() - var hasIconicsAttrs = false repeat(attrs.attributeCount) { - val name = attrs.getAttributeName(it) - attrsMap[name] = attrs.getAttributeValue(it) - if (name.startsWith("ico_")) { - hasIconicsAttrs = true - } + attrsMap[attrs.getAttributeName(it)] = attrs.getAttributeValue(it) } - // Don't unset non-iconics menu item icon - if (!hasIconicsAttrs) return - attrsMap["id"] ?.replace("@", "") ?.removePrefix("+id/")