Skip to content

fix(desktop-file): verify theme icons before rewriting icon - #6374

Open
soumyaDghosh wants to merge 1 commit into
canonical:mainfrom
soumyaDghosh:main
Open

fix(desktop-file): verify theme icons before rewriting icon#6374
soumyaDghosh wants to merge 1 commit into
canonical:mainfrom
soumyaDghosh:main

Conversation

@soumyaDghosh

Copy link
Copy Markdown
Contributor

Theme-resolved icon path was written to the desktop file without checking the file exists, and the subsequent existence check ran against the bare icon name stored in icon variable, emitting a spurious "not found" warning. Assign the resolved path to the local var first so the same check covers theme icons.

Fixes #6364

IMO, this complete function should be refactored, including early return when icon is None, converting these 2 lines into a single one liner and remove any prefixed $SNAP as well

@@ -89,11 +89,8 @@ class DesktopFile:
             if icon_path is not None:
                 icon = icon_path
 
-            # Strip any leading slash.
-            icon = icon[1:] if icon.startswith("/") else icon
-
-            # Strip any leading ${SNAP}.
-            icon = icon[8:] if icon.startswith("${SNAP}") else icon
+            # Strip any leading slash, ${SNAP} and $SNAP variable
+            icon.removeprefix("/").removeprefix("${SNAP}").removeprefix("$SNAP")
 
             # If icon is just a name (no path separator), try to resolve it from the hicolor icon theme.
             if "/" not in icon:

  • I've followed the contribution guidelines.
  • I've signed the CLA.
  • I've successfully run make lint && make test.
  • I've added or updated any relevant documentation.
  • In documents I changed, I added a meta description if one was missing.
  • I've updated the relevant release notes.

Theme-resolved icon path was written to the desktop file without
checking the file exists, and the subsequent existence check ran
against the bare icon name stored in `icon` variable, emitting a spurious "not found" warning.
Assign the resolved path to the local var first so the same check
covers theme icons.

Signed-off-by: Soumyadeep Ghosh <soumyadeepghosh2004@zohomail.in>
Copilot AI review requested due to automatic review settings July 27, 2026 07:23
@soumyaDghosh
soumyaDghosh requested a review from mr-cal as a code owner July 27, 2026 07:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes incorrect “Icon not found” warnings when a desktop file’s Icon= value is a theme icon name that gets resolved from the hicolor theme during desktop file rewriting (Fixes #6364).

Changes:

  • Update theme icon resolution logic to assign the resolved theme path to the local icon variable (so the subsequent existence check validates the resolved path).
  • Avoid writing the theme-resolved ${SNAP}/… icon path into the desktop file before the file existence check has passed.

Comment on lines +101 to +105
theme_icon_path := get_icon_from_theme(
os.fspath(self._prime_dir), "hicolor", icon
)
) is not None:
self._parser[section]["Icon"] = os.path.join("${SNAP}", icon_path)
icon = theme_icon_path
Comment on lines 99 to 103
if "/" not in icon:
if (
icon_path := get_icon_from_theme(
theme_icon_path := get_icon_from_theme(
os.fspath(self._prime_dir), "hicolor", icon
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Icons resolved from icon theme still prints error messages

2 participants