fix: update Linux tray icon in place on OS theme change#515
Open
IliyaBrook wants to merge 1 commit intoaaddrick:mainfrom
Open
fix: update Linux tray icon in place on OS theme change#515IliyaBrook wants to merge 1 commit intoaaddrick:mainfrom
IliyaBrook wants to merge 1 commit intoaaddrick:mainfrom
Conversation
Avoids a StatusNotifierItem re-registration race on KDE Plasma
where the old SNI remains registered when the new one appears,
resulting in two tray icons side by side until session logout.
`patch_tray_menu_handler` already bounds the race with a 250 ms
delay after `tray.destroy()`, but that's not enough on all setups
(reproduced on Fedora 43 KDE Plasma 6.6.4 + Wayland). Widening the
delay just moves the goalposts; the race is structural.
Fix: inject a fast-path before the existing destroy+recreate block
in the tray rebuild function. When the tray already exists and
isn't being disabled, update its icon and context menu in place
via `setImage` + `setContextMenu` — the existing StatusNotifierItem
stays registered, no DBus re-registration, no race. The slow path
(destroy + delay + re-create) is kept for the initial creation and
the tray-disable cases where it's unavoidable.
All five minified locals needed by the fast-path (tray function,
tray variable, electron module, menu function, icon path const,
menuBarEnabled flag) are extracted dynamically; the idempotency
guard re-keys off the post-rename `setImage(...)` sequence.
Triggered in KDE System Settings by any of Appearance → Colors /
Plasma Style / Global Theme, which all fire the same
`nativeTheme.on('updated')` signal.
Follow-up to aaddrick#491. The broader submenu work from that PR stays
parked on features/change-icon-color pending the scope discussion
in aaddrick#492; this PR ships only the duplicate-tray-icon fix that
@aaddrick asked to split out.
Co-Authored-By: Claude <[email protected]>
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.
What's in this PR
As asked in the #491 review:
patch_tray_inplace_updateinscripts/patches/tray.sh— fast-path that updates the existingStatusNotifierItemin place viasetImage+setContextMenuinstead of destroy + recreatescripts/patches/app-asar.shwiring — calls the new patch betweenpatch_tray_icon_selectionandpatch_menu_bar_defaultdocs/learnings/tray-rebuild-race.md— scoped-down learnings covering the race, the fix, and the trigger surface. The Appearance → Theme note you asked for is folded in: Colors / Plasma Style / Global Theme dropdowns all fire the samenativeTheme.on('updated')and all reproduce the duplicate icon.CLAUDE.mdlearnings index entryAddressing the review blocker from #491
tande— both literal minified locals called out as a blocker are now extracted alongsidetray_var/menu_func/electron_var:path_varfrom thenew Tray(nativeImage.createFromPath(X))call,enabled_varvia the same pattern aspatch_menu_bar_default.${tray_var}.setImage(${electron_var}.nativeImage.createFromPath(${path_var}))sequence using post-rename names, instead of the off-cursor(t))marker that would've drifted silently.