Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout the repository | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Setup .NET | ||
| uses: actions/setup-dotnet@v5 | ||
| with: | ||
| dotnet-version: | | ||
| 9.0.x | ||
| 10.0.x | ||
|
|
||
| - name: Restore the Uno library | ||
| run: dotnet restore "$UNO_LIB_PROJ_PATH" | ||
|
|
||
| - name: Build the Uno library on Linux | ||
| run: dotnet build "$UNO_LIB_PROJ_PATH" -c Release --no-restore | ||
|
|
||
| - name: Run X11 unit tests | ||
| run: dotnet test "$X11_TEST_PROJ_PATH" -c Release | ||
|
|
||
| - name: Check changed-file whitespace | ||
| if: github.event_name == 'pull_request' | ||
| shell: bash | ||
| run: git diff --check "$(git merge-base HEAD "origin/$GITHUB_BASE_REF")"..HEAD |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 111d560371
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| lock (_lifecycleLock) | ||
| invoke = !_isDisposed; | ||
| if (invoke) | ||
| action(); |
There was a problem hiding this comment.
When a D-Bus callback posts to the captured UI context and Destroy() runs before that callback executes, this check still permits the event because Destroy() clears _isVisible but does not set _isDisposed. The stale click or scroll can therefore be raised after the tray icon has been removed—or even after a subsequent Show() starts a new connection generation—potentially opening UI unexpectedly. Gate queued callbacks on the captured connection generation and visibility as well as disposal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
SystemTrayIcon tooltip handling removed null normalization, which can propagate null into D-Bus properties and cause runtime failures.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR strengthens the Linux/Uno X11 hosting stack for DesktopFlyouts by correcting X11 interop/property handling, improving activation/geometry behavior, and making the StatusNotifierItem tray lifecycle more robust while adding CI + targeted unit tests.
Changes:
- Fix LP64 X11 ABI/property interactions (layout corrections + passing
format=32properties as native-long arrays) and improve activation preservation/restore behavior. - Improve screen/work-area selection on X11 (XRandR “nearest monitor” selection + current-virtual-desktop work area application).
- Rework StatusNotifierItem D-Bus lifecycle so
Destroy()reliably unregisters (generation/cancellation-based lifecycle management), and add X11 tests + Ubuntu CI job.
File summaries
| File | Description |
|---|---|
| tests/DesktopFlyouts.X11.Tests/X11WindowActivationTests.cs | Adds tests covering override-redirect and activation-request policy decisions. |
| tests/DesktopFlyouts.X11.Tests/X11ScreenGeometryTests.cs | Adds tests for monitor selection, virtual-desktop work area parsing, and clipping behavior. |
| tests/DesktopFlyouts.X11.Tests/X11InteropLayoutTests.cs | Adds ABI/layout assertions for key X11/XRandR interop structs across 32/64-bit. |
| tests/DesktopFlyouts.X11.Tests/DesktopFlyouts.X11.Tests.csproj | Introduces a dedicated X11 test project that links production helper/interop sources. |
| src/DesktopFlyouts.Uno/GeneratorBackfill/DesktopFlyoutIslandTemplateSettings.Uno.cs | Adds a manual Uno DP implementation for BackdropCornerRadius (generator doesn’t run). |
| src/DesktopFlyouts.Uno/DesktopFlyouts.Uno.csproj | Normalizes packaging items and D-Bus generator configuration for Uno builds. |
| src/DesktopFlyouts.Shared/XamlIslandHostWindow.X11.cs | Improves X11 sizing/positioning, activation policy, active-window preservation, and theme-change forwarding. |
| src/DesktopFlyouts.Shared/X11PInvoke.cs | Corrects X11 interop types/layouts and adds XRandR + safer XChangeProperty handling for 32-bit formats. |
| src/DesktopFlyouts.Shared/SystemTrayIcon.X11.cs | Reworks StatusNotifierItem registration/unregistration to be connection-scoped and race-safe; dispatches callbacks via captured context. |
| src/DesktopFlyouts.Shared/Helpers/X11WindowActivation.cs | Centralizes activation/override-redirect policy decisions for X11. |
| src/DesktopFlyouts.Shared/Helpers/X11ScreenGeometry.cs | Introduces monitor selection + EWMH work-area parsing/clipping helpers for X11. |
| src/DesktopFlyouts.Shared/Helpers/WindowHelpers.X11.cs | Uses XRandR monitor geometry and current-desktop work area to compute flyout/work areas more accurately. |
| src/DesktopFlyouts.Shared/Helpers/GeneralHelpers.X11.cs | Retains/disposing portal theme watch and raises change notifications to hosts. |
| src/DesktopFlyouts.Shared/DesktopMenuFlyout.Uno.cs | Ensures Uno disposal path calls base.Dispose() and avoids member-hiding warnings explicitly. |
| src/DesktopFlyouts.Shared/DesktopMenuFlyout.cs | Imports Windows.Graphics in the UWP build path to keep RectInt32 usage building. |
| src/DesktopFlyouts.Shared/DesktopFlyouts.Shared.projitems | Adds the new X11 helper sources to the shared build inputs. |
| src/DesktopFlyouts.Shared/DesktopFlyoutIslandTemplateSettings.cs | Enables BackdropCornerRadius DP for WASDK (incl. Uno); keeps SystemBackdrop WASDK-only when not Uno. |
| src/DesktopFlyouts.Shared/DesktopFlyoutIsland.cs | Updates WASDK template-settings/backdrop update logic to match the new conditional compilation split. |
| src/DesktopFlyouts.Shared/DesktopFlyout.cs | Aligns Uno disposal signature/hiding semantics and calls base.Dispose() on Uno. |
| Directory.Packages.props | Adds centralized package versions for the new xUnit test project packages. |
| Directory.Build.targets | Avoids forcing Windows-only runtime identifiers for Uno and test projects (helps Linux CI). |
| DesktopFlyouts.slnx | Adds the new X11 test project to the solution structure. |
| .github/workflows/ci.yml | Adds an Ubuntu job to build the Uno library and run the X11 unit tests. |
Review details
Suppressed comments (1)
src/DesktopFlyouts.Shared/SystemTrayIcon.X11.cs:127
- Tooltip setter no longer normalizes null values before updating the StatusNotifierItem. If a caller passes null (possible even with nullable annotations), it can propagate into D-Bus properties via SetTitleAndTooltip and trigger runtime/serialization failures. Normalize to an empty string and use the normalized value for both the backing field and the handler update.
_tooltip = value;
handler = _isRegistered ? _sniHandler : null;
}
handler?.SetTitleAndTooltip(value);
- Files reviewed: 23/23 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| _id = id; | ||
| _iconPath = iconPath; | ||
| _tooltip = tooltip; | ||
| _eventContext = SynchronizationContext.Current; | ||
|
|
Fixes
override_redirecttoNeverActivatewindows._NET_ACTIVE_WINDOWrequests and preventNoActivate/NeverActivateflyouts from producing incorrect activation state.Destroy()unregisters it reliably, including race-safe initialization, reconnection, and callback dispatch.BackdropCornerRadiusdependency property used by the Linux template.Windows.GraphicsforRectInt32.Steps to validate
dotnet msbuild /restore:false src\DesktopFlyouts.Uno\DesktopFlyouts.Uno.csproj /p:Configuration=Debug /p:Platform=x64dotnet test tests\DesktopFlyouts.X11.Tests\DesktopFlyouts.X11.Tests.csproj -c Debug --no-restoredotnet msbuild /restore:false samples\DesktopFlyouts.Wasdk.Sample.App\DesktopFlyouts.Wasdk.Sample.App.csproj /p:Configuration=Debug /p:Platform=x64 /p:AppxBundle=Neverdotnet msbuild /restore:false src\DesktopFlyouts.Uwp\DesktopFlyouts.Uwp.csproj /p:Configuration=Debug /p:Platform=x64 /p:AppxBundle=Nevershell:AppsFolder; confirm it remains responsive and has a non-zeroMainWindowHandle.Destroy(), and live light/dark theme updates.git diff --checkandgit ls-files --eol; confirm no whitespace errors and that changed text files remain CRLF.