fix(linux): nativeImage for window icon; hicolor theme for panel/launcher icon#615
Merged
Merged
Conversation
BrowserWindow's icon option requires a real filesystem path on Linux; the OS window manager cannot read from inside an asar archive. Even though Electron patches fs.existsSync to return true for asar-internal paths, the path string passed to the native BrowserWindow constructor is not resolved by Electron's asar shim, so the window icon silently disappears in the installed .deb. Switching to nativeImage.createFromPath() on Linux reads the PNG data into memory inside the Electron process (which is asar-aware), then passes the decoded image object to BrowserWindow. This works in both dev mode (yarn dev) and the packaged app without requiring asarUnpack. Closes emuflight#614 Ref: emuflight#609 (comment) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
maker-deb installs the app icon only to /usr/share/pixmaps/ (legacy location). XFCE panel, GNOME, and KDE resolve Icon= names via the XDG hicolor theme (/usr/share/icons/hicolor/) rather than pixmaps, so the panel launcher icon shows as blank even though the .desktop file has the correct Icon=emuflight-configurator entry. Add Debian postinst/prerm scripts (via electron-installer-debian's options.scripts) that copy the icon to /usr/share/icons/hicolor/128x128/apps/ on install and remove it on uninstall, running gtk-update-icon-cache after each operation. Fixes: XFCE (and other XDG DE) panel launcher showing no icon. Related: emuflight#614 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
A failure in postinst/prerm must not abort dpkg installation or removal. Icon placement is cosmetic — the app functions fully without it. Add || true to the install and rm commands so any filesystem or permission error is silently ignored rather than propagating as a dpkg error. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughDebian packaging now runs post-install and pre-removal scripts that manage the hicolor icon and refresh the icon cache. The Electron main process now loads the Linux window icon with ChangesDebian icon packaging and runtime loading
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@main.js`:
- Around line 76-78: The Linux icon selection in the main icon-loading logic
stops too early when NativeImage.createFromPath returns an empty image, so later
fallback candidates are never tried. Update the candidate-scanning flow in the
icon resolution code around nativeImage.createFromPath and the Linux branch so
an empty img is treated like a failed candidate and the loop continues to the
next path instead of returning undefined immediately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c59950c-9c35-4500-a403-eab83eff8efe
📒 Files selected for processing (4)
forge.config.jsmain.jsscripts/deb-postinst.shscripts/deb-prerm.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build (macos-26-intel, macos-x86_64)
- GitHub Check: build (ubuntu-24.04, linux-x64)
- GitHub Check: build (macos-26, macos-arm64)
- GitHub Check: build (windows-2022, windows-x64, x64)
- GitHub Check: build (windows-2022, windows-ia32, ia32)
🔇 Additional comments (3)
forge.config.js (1)
104-110: LGTM!scripts/deb-postinst.sh (1)
1-13: LGTM!scripts/deb-prerm.sh (1)
1-9: LGTM!
If a Linux icon path exists (fs.existsSync) but the file is unreadable or corrupt, nativeImage.createFromPath() returns an empty NativeImage. Returning undefined immediately skips all remaining fallback candidates. Use continue so the loop advances to the next path instead. Addresses: emuflight#615 (review) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AI Generated pull-request
Summary
main.js— usenativeImage.createFromPath()on Linux forBrowserWindow({ icon }). Passing a raw path string silently fails when the app is asar-packaged:fs.existsSyncreturnstruefor virtual asar paths (Electron patchesfs), but the OS window manager receives the path string directly and cannot read inside the archive.nativeImagereads the PNG into memory within Electron's asar-aware process and passes the decoded image object instead.forge.config.js+scripts/— add Debianpostinst/prermscripts that install/remove the icon in/usr/share/icons/hicolor/128x128/apps/.maker-debinstalls only to/usr/share/pixmaps/(legacy GTK fallback); XDG desktop environments (XFCE, GNOME, KDE) resolveIcon=emuflight-configuratorvia the hicolor theme, so the panel launcher icon was blank without it. All icon operations are guarded with|| trueso any filesystem or permission failure is non-fatal and never abortsdpkg.Closes #614
Ref: #609 (comment)
Test plan
yarn make:devproduces a.debwithout errorssudo dpkg -iinstalls cleanly;/usr/share/icons/hicolor/128x128/apps/emuflight-configurator.pngpresent after installsudo dpkg -r emuflight-configuratoruninstalls cleanly; hicolor icon removedyarn devstill shows correct taskbar icon on Linux