fix: update Linux tray icon in place on OS theme change#515
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]>
Drop the redundant `electron_var_re_local` local — `electron_var_re`
is already a sourced global from `_common.sh` with the same value.
Replace the silent `head -1` on `enabled_var` extraction with an
explicit count-and-bail. The grep matches `const X=fn("menuBarEnabled")`
across the whole file; today there's exactly one site (inside the
tray function), but if upstream ever ships a second the previous
code would silently bind to whichever the minifier emitted first.
Bail loudly with a count diagnostic instead.
Verified on the live 1.3883.0 build asar: all five extractions
resolve (`Nh`/`wAt`/`t`/`e`) — note the symbol drift vs. the
build-reference's `fh`/`CZe`. Fast-path injects, JS validates,
idempotent re-run confirmed, duplicate-icon repro gone on Nobara
KDE Plasma 6 (Wayland) under Appearance → Colors / Plasma Style /
Global Theme.
Co-Authored-By: Claude <[email protected]>
aaddrick
left a comment
aaddrick
left a comment
There was a problem hiding this comment.
Hey @IliyaBrook! Thanks for the follow-up on this. The dynamic extraction across all five identifiers is exactly what #491 needed, and the idempotency guards on the post-rename names rather than the pre-rename ones is the right call. I also like that the slow path stays intact for initial creation and the menuBarEnabled === false case, and the fast-path's e!==false lines up with what patch_menu_bar_default rewrites !!e into, so the two patches stay semantically aligned. The learnings doc is solid too — the race description and the Colors / Plasma Style / Global Theme trigger surfaces are going to save someone a lot of time later.
I pushed a small fixup on top (fef69f0) via maintainerCanModify so you don't see a stranger's commit on your branch and wonder. Three things: dropped a redundant electron_var_re_local in favor of $electron_var_re from _common.sh (same value), swapped the silent head -1 on enabled_var extraction for an explicit count-and-bail so a future second const X=fn("menuBarEnabled") declaration trips the build instead of binding silently to whichever the minifier emitted first, and a cosmetic double-blank-line fix between two functions.
Verified end-to-end on the live 1.3883.0 build asar. The current symbols are Nh / wAt / t / e for tray_var / menu_func / path_var / enabled_var, while the build-reference (older asar) carried fh / CZe / t / e. Symbol drift across two upstream releases makes the dynamic extraction load-bearing, and your patch handles both cleanly.
Built the AppImage on this branch and ran it on Nobara 43 / KDE Plasma 6 / Wayland. Pre-fix repro confirmed on all three Appearance triggers (Colors, Plasma Style, Global Theme). Post-fix: single icon, updates in place under all three. No regressions I could find. Approving.
Written by Claude Opus 4.7 via Claude Code
Co-Authored-By: Claude <[email protected]>
2 participants
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.