fix(linux): keep tray menu labels from rendering blank - #574
Conversation
libappindicator exports the menu in two passes: the structure first, then the item properties ~50ms later as `ItemsPropertiesUpdated` plus a second `LayoutUpdated`. GNOME's AppIndicator extension cannot absorb that. Its `GetLayout` deliberately asks for `type`/`children-display` only and fetches labels separately on an idle callback; the second `LayoutUpdated` cancels that pending fetch, and the re-run finds the item ids already known and skips re-requesting them. The signal that did carry the labels arrived before the extension had created those items, so it was dropped. The labels then keep the extension's default empty string and the menu renders as blank rows — 5 of 6 launches on a GNOME 50 / Wayland box. Publish a freshly built menu once startup has quiesced. New item ids force the extension to create the items again and fetch their properties, with nothing racing the request that time.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe tray module now centralizes menu construction, assigns a named tray identifier, and republishes a rebuilt menu after a 1500 ms delay through the main thread. ChangesTray menu republishing
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change rebuilds and republishes the Linux tray menu shortly after startup to prevent blank menu labels. No current merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant install
participant republish_menu
participant BackgroundThread
participant MainThread
participant TrayIcon
install->>republish_menu: schedule menu republish
republish_menu->>BackgroundThread: sleep 1500 ms
BackgroundThread->>MainThread: dispatch menu refresh
MainThread->>TrayIcon: rebuild and set menu
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
xiufengsun
left a comment
There was a problem hiding this comment.
Reviewed at exact head 54f6485. The delayed menu rebuild is scoped to the Linux tray startup race, preserves the existing item handlers, integrates cleanly with current main, and the exact-head Linux/full CI and CodeQL checks are green.
Problem
On Linux the tray menu intermittently renders as blank rows — the two entries are there, correctly sized, with no text. Restarting the app sometimes fixes it, sometimes not.
Measured on GNOME 50.3 / Wayland / Fedora, with
dbus-monitoroninterface='com.canonical.dbusmenu': 5 of 6 launches never fetched the labels at all.Cause
Two independent halves that combine badly.
Our side — libappindicator exports the menu in two passes: the structure first, then the item properties ~47ms later as
ItemsPropertiesUpdatedplus a secondLayoutUpdated. This happens on every launch, once the GTK main loop starts:The extension side (
appindicatorsupport@rgcjonas.gmail.com/dbusMenu.js) cannot absorb that:GetLayoutdeliberately requests onlytypeandchildren-display, neverlabel. Labels come from a separateGetGroupPropertiesscheduled on an idle callback.LayoutUpdatedcancels the previous update's cancellable (_layoutUpdateUpdateAsync), killing that pending fetch.this._itemsand early-returns before_requestProperties(line 420).ItemsPropertiesUpdatedthat did carry the labels arrived before the extension had created those items, so_onPropertiesUpdateddropped it (if (!item) return).The label property therefore never reaches the shell, and its default is
''— blank rows. Whether a given launch wins or loses the race is what makes it intermittent.Fix
Publish a freshly built menu 1.5s after startup, once the export storm is over. A new menu gets new dbusmenu item ids, so the extension has to create the items from scratch and fetch their properties, and nothing is racing the request that time.
This is a mitigation for a race in the GNOME extension, not a fix for the extension itself — but it is the only side we control.
Testing
cargo test,cargo fmt --check— pass.dbusmenutraffic.Summary by CodeRabbit