Skip to content

Commit 8c515d3

Browse files
bezzadclaude
andcommitted
revert(tray): restore unconditional tray Clicked handler — Linux gate hid the icon entirely
fa6b925 gated TrayIcon.Clicked→ShowWindow off on Linux to fix the right-click menu; on the author's Ubuntu box the tray icon then stopped appearing at all (before, it appeared with only the menu broken). Restore the exact configuration of every build where the icon showed, drop the gate + its policy test, and rewrite the SKILL.md tray note: no more speculative changes to Linux tray code — the still-open menu bug needs on-device evidence first. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YPmwwAZ3FXZymxtMemRiz6
1 parent fa6b925 commit 8c515d3

3 files changed

Lines changed: 6 additions & 41 deletions

File tree

.claude/skills/downloader-desktop/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ Skip the discovery grep; jump straight to the file. `src/Downloader.Desktop/`:
6868
- **Queues page = a real queue manager** (`Views/QueuesView.axaml`, `ViewModels/QueuesViewModel.cs`): per-queue card shows live aggregate stats (`RunningCount/WaitingCount/DoneCount/FailedCount`, `TotalSpeedText`, `SummaryText`) + a combined `OverallProgress` bar (average of item `Progress`), a run/pause `ToggleSwitch`, the concurrency cap, and the queue's downloads with per-item progress + pause/resume/retry/cancel/remove + reorder (`ChevronUp/Down` → `manager.MovePriority(vm, ±1)`) + move-between-queues (a `MenuFlyout` of `QueueMoveTarget`s → `manager.MoveToQueue(vm, queueId)`, hidden when only one queue). Rows are wrapped in `QueueItemViewModel` (holds the real `DownloadItemViewModel` as `Item` + the reorder/move commands); the card's `Items` is an `ObservableCollection<QueueItemViewModel>` rebuilt on `ListChanged` (order = master `Items` order = pump priority), and aggregates refresh on both `ListChanged` and `StatsChanged` (live). `QueueRowViewModel.Detach()` unsubscribes both events. **Pump order follows master-list order**, so `MovePriority` just `Items.Move`s past the same-queue neighbour. `Initialize` now backfills `QueueId=DefaultQueue.Id` for items saved without one (older configs) so they always appear on a queue. `DownloadItemViewModel.FormatBytes` is now `public static` for reuse.
6969

7070
## System tray / startup / auto-update (Round 11) — patterns worth caching
71-
- **Tray** (`Services/TrayService`, static): create `TrayIcon` in code and register via `TrayIcon.SetIcons(Application.Current, new TrayIcons { icon })`. **`NativeMenuItemToggleType` does NOT exist in this Avalonia 12** — don't use `ToggleType`/`IsChecked` on `NativeMenuItem`; reflect state by swapping the item's `Header` ("Disable/Enable notifications"). Wrap creation in try/catch — headless/no-session platforms throw; on failure leave `_tray=null` so close-to-tray fails soft. **Tray menu not showing / stale-corrupted on Linux (Ubuntu GNOME/AppIndicator):** (1) **NEVER subscribe `TrayIcon.Clicked → ShowWindow` ON LINUX.** This has now flip-flopped twice and the empirical answer is settled: `2654f9a` dropped the handler and the menu worked; `3aac545` re-added it "for resilience" and the menu broke again (re-reported 2026-07-22); it is now gated behind `TrayService.HandlesTrayClick` (`!OperatingSystem.IsLinux()`), pinned by `TrayServiceTests.Tray_click_is_not_handled_on_linux_so_the_context_menu_can_open`. Mechanism: on the DBus StatusNotifierItem (Ubuntu GNOME AppIndicator) the click that opens the menu is ALSO delivered as an activation, and `WindowActivation.BringToFront` (Show + Activate + Topmost flip) steals focus from the popup the shell is opening, so the menu never appears. Note the earlier "Clicked never fires on GNOME so it's a harmless no-op" reasoning was WRONG — it does fire. Also note Avalonia wires `_impl.OnClicked` unconditionally in the `TrayIcon` ctor, so this is about what OUR handler does (raising the window), not about the subscription changing any SNI property (`ItemIsMenu` is never set by Avalonia; it defaults false). Keep the handler on Windows/macOS, where Clicked is a genuine separate activation. On Linux the menu's "Open Downloader" item is the way back from the tray. (2) **Use a SMALL tray icon** (downscale to 64×64 via `Bitmap.CreateScaledBitmap`) — the 1080×1080 app PNG is a ~4.6 MB pixmap over DBus and can make the SNI item render while its menu fails to attach. Both fixed for v1.4.x. Linux tray menus are NOT verifiable in the headless test box — needs on-device confirmation.
71+
- **Tray** (`Services/TrayService`, static): create `TrayIcon` in code and register via `TrayIcon.SetIcons(Application.Current, new TrayIcons { icon })`. **`NativeMenuItemToggleType` does NOT exist in this Avalonia 12** — don't use `ToggleType`/`IsChecked` on `NativeMenuItem`; reflect state by swapping the item's `Header` ("Disable/Enable notifications"). Wrap creation in try/catch — headless/no-session platforms throw; on failure leave `_tray=null` so close-to-tray fails soft. **Linux tray (Ubuntu GNOME/AppIndicator) — DO NOT make speculative changes here. Ever.** This code has flip-flopped three times (`2654f9a` removed the Clicked handler, `3aac545` re-added it, `fa6b925` gated it off for Linux) and each theory about the `TrayIcon.Clicked → ShowWindow` handler was later contradicted by on-device behavior: after `fa6b925` gated the handler off, the author reported the tray **icon stopped appearing at all** (previously it appeared and only the right-click menu was broken) — so the handler was reverted to unconditional, which is the configuration of every build where the icon did show. Conclusions that ARE settled: (a) keep the unconditional `_tray.Clicked += → ShowWindow` subscription; (b) **use a SMALL tray icon** (downscale to 64×64 via `Bitmap.CreateScaledBitmap` — the 1080×1080 PNG is a ~4.6 MB pixmap over DBus and can make the SNI item render while its menu fails to attach). The right-click-menu-doesn't-open bug is still OPEN and **cannot be diagnosed from this headless box** (no desktop session, no DBus StatusNotifierWatcher): any further change requires on-device evidence first — e.g. run the app on the Ubuntu box with logging enabled, `dbus-monitor` the `org.kde.StatusNotifierItem` traffic, or test a minimal Avalonia tray repro — never another code-only guess.
7272
- **Close-to-tray**: handle `window.Closing`, `e.Cancel=true; window.Hide()` — but gate on **`TrayService.IsActive`**, NOT the setting, or a failed tray strands the window with no way back. Real quit sets a `_quitting` flag then `window.Close()` (ShutdownMode is OnMainWindowClose → App.ShutdownRequested still saves). Wired in `MainViewModel.SetupAppShell()` after config loads (needs `View as Window`).
7373
- **Run-at-startup** (`Services/StartupService`): no extra deps — Windows via `reg.exe add/query/delete HKCU\...\Run` (avoids `Microsoft.Win32.Registry` package on the non-windows TFM), Linux `~/.config/autostart/downloader.desktop`, macOS `~/Library/LaunchAgents/*.plist`. Launches with `--minimized`; `MainViewModel` hides the window at startup if that arg is present AND tray active. Coupling lives in `SettingViewModel`: disabling tray disables startup; enabling startup enables tray.
7474
- **Auto-update** (`Services/UpdateService` + `UpdateFlow`): **version compare uses `Assembly.GetName().Version` (`CurrentVersion` = `Major.Minor.Build`), NOT `InformationalVersion`** (that has the date-derived revision and would never compare sensibly to a `v1.1.0` tag). **Versioning fix (#update-false-alarm, 2026-06-19):** `VersionPrefix` is now the FULL 3-part semver (e.g. `1.1.2`) and `AssemblyVersion=$(VersionPrefix).0` so the app reports its real patch — the old `major.minor.0.0` pin made it always report `x.y.0`, so every patch release (e.g. `v1.1.1`) looked "newer" forever → false "update available". `release.yml` stamps `-p:VersionPrefix=<tag-without-v>` from the tag so a released build reports exactly the tag; About card (`SettingViewModel.AppVersion`) shows `UpdateService.CurrentVersion` so About + update status + release tag all agree. **Keep `VersionPrefix` three-part.** `UpdateService.IsNewer(tag, current)` + `Normalize(tag)` are pure/tested. Flow: GitHub `releases/latest` → if newer, the in-app `UpdateFlow.PromptUpdate` dialog (Download/Later) + a passive OS notification (NOT a clickable toast — see the OS-only note below) → download the per-RID asset (`ExpectedAssetName()` matches release.yml names) via a throwaway `DownloadService` → `ApplyDownloadedArchive` spawns a detached unix `.sh`/win `.cmd` that waits for the PID to exit, extracts over the app dir, relaunches → `UpdateFlow.RequestShutdown` (= MainViewModel.Quit). The self-swap is untestable here; only the version logic has tests.

src/Downloader.Desktop.Tests/UI/TrayServiceTests.cs

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -61,24 +61,4 @@ public void Tray_icon_bitmap_is_downscaled_to_64x64_not_the_full_size_app_icon()
6161
Assert.Equal(TrayService.TraySize.Height, scaled.PixelSize.Height);
6262
Assert.True(TrayService.TraySize.Width <= 128, "Tray icon must stay small for DBus SNI hosts.");
6363
}
64-
65-
/// <summary>
66-
/// Regression for the recurring Ubuntu/GNOME "tray right-click menu doesn't open" bug. On Linux the
67-
/// click that opens the menu is also delivered as an activation; raising the window from that callback
68-
/// steals focus from the popup being opened and the menu never shows. Dropping the Clicked handler
69-
/// fixed it once (2654f9a) and re-adding it regressed it (3aac545) — this pins the policy so the
70-
/// handler can't be reintroduced for Linux a third time.
71-
/// <para>
72-
/// Whether a native menu actually pops up cannot be asserted headlessly (no desktop session, no DBus
73-
/// StatusNotifierWatcher here), so this locks the decision itself rather than the visual outcome.
74-
/// </para>
75-
/// </summary>
76-
[Fact]
77-
public void Tray_click_is_not_handled_on_linux_so_the_context_menu_can_open()
78-
{
79-
Assert.Equal(!OperatingSystem.IsLinux(), TrayService.HandlesTrayClick);
80-
81-
if (OperatingSystem.IsLinux())
82-
Assert.False(TrayService.HandlesTrayClick, "Handling tray clicks on Linux suppresses the SNI context menu.");
83-
}
8464
}

src/Downloader.Desktop/Services/TrayService.cs

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,23 +23,6 @@ public static class TrayService
2323

2424
public static bool IsActive => _tray != null;
2525

26-
/// <summary>
27-
/// Whether to open the main window from <see cref="TrayIcon.Clicked"/>. False on Linux — and that is
28-
/// load-bearing, not a preference.
29-
/// <para>
30-
/// On Linux the tray is a DBus StatusNotifierItem (Ubuntu GNOME's AppIndicator). There the click that
31-
/// the user makes to get the menu is ALSO delivered to us as an activation, and raising + focusing the
32-
/// main window from that callback steals focus from the popup the shell is in the middle of opening, so
33-
/// the menu never appears — the reported "right-click menu doesn't open" bug. Dropping this handler is
34-
/// what fixed it in 2654f9a; re-adding it for "resilience" in 3aac545 brought the bug straight back.
35-
/// Don't re-enable it for Linux: on that platform the menu IS the interaction, and its
36-
/// "Open Downloader" item is the way back from the tray.
37-
/// </para>
38-
/// Windows and macOS raise Clicked as a genuine, separate activation (the context menu has its own
39-
/// right-click path), so the handler is safe and useful there.
40-
/// </summary>
41-
internal static bool HandlesTrayClick => !OperatingSystem.IsLinux();
42-
4326
/// <summary>Wire the window + quit action once at startup (before Enable/Disable).</summary>
4427
public static void Init(Window window, Action onQuit)
4528
{
@@ -101,9 +84,11 @@ private static void BuildTray()
10184
menu.Items.Add(quit);
10285
_tray.Menu = menu;
10386

104-
// Open the main window when the icon is activated — but NOT on Linux. See HandlesTrayClick.
105-
if (HandlesTrayClick)
106-
_tray.Clicked += (_, _) => ShowWindow();
87+
// Open the main window when the icon is activated. Do NOT platform-gate this: gating it off on
88+
// Linux (fa6b925) coincided with the tray icon not appearing at all on the author's Ubuntu box,
89+
// and this exact handler was present in every build where the icon DID show. The Linux
90+
// right-click-menu bug is a separate, on-device-only issue — see SKILL.md.
91+
_tray.Clicked += (_, _) => ShowWindow();
10792

10893
TrayIcon.SetIcons(Application.Current!, new TrayIcons { _tray });
10994
}

0 commit comments

Comments
 (0)