Repository navigation
CmdPal: Unify context-menu behavior and shortcut handling - #50843
Merged
Michael Jolley (michaeljolley) merged 5 commits intoSep 28, 2026
Merged
Michael Jolley (michaeljolley) merged 5 commits into
Michael Jolley (michaeljolley) merged 5 commits into
Conversation
- Moves shortcut handling to the shell and active menu to prevent duplicate execution, while preserving the command bar's visual debounce. - Opens menus with explicit contexts and prepares them on demand using cached extension data. - Protects Enter/Ctrl+Enter, corrects shortcut hints, and keeps secondary actions and hydrated submenus current. - Adds dock shortcuts while preserving primary activation, invocation context and home-page history behavior. - Shares flyout handling and defers reopening until WinUI finishes closing, preventing lost opens and stale cleanup. - Separates selection updates from Narrator announcements and adds regression coverage for shortcuts, dock actions and flyout lifecycle.
# Conflicts: # src/modules/cmdpal/Microsoft.CmdPal.UI/Dock/DockControl.xaml # src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.UnitTests/Microsoft.CmdPal.UI.UnitTests.csproj
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Michael Jolley (michaeljolley)
enabled auto-merge (squash)
September 28, 2026 18:37
Michael Jolley (michaeljolley)
approved these changes
Sep 28, 2026
Michael Jolley (michaeljolley)
left a comment
Contributor
There was a problem hiding this comment.
LGTM
Michael Jolley (michaeljolley)
deleted the
feature/50807-unify-context-menu-behavior
branch
September 28, 2026 18:38
This was referenced Sep 30, 2026
This was referenced Sep 30, 2026
Clint Rutkas (crutkas)
added a commit
that referenced
this pull request
Sep 30, 2026
âĶSamplePagesExtension (#50917) ## Summary of the Pull Request Fixes an intermittent PowerToys CI failure. The main build sometimes fails while MSIX packaging trims the Command Palette sample extension (`SamplePagesExtension`): ``` Microsoft.NET.ILLink.targets(150,5): error MSB6006: "dotnet.exe" exited with code 57005. Microsoft.NET.ILLink.targets(103,5): error NETSDK1144: Optimizing assemblies for size failed. ``` The fix is one Release-only property in `SamplePagesExtension.csproj`: `TrimmerRemoveSymbols=true`. With it, ILLink doesn't rewrite debug symbols for this trim, so it never loads the native Windows PDB writer that crashes. The trimmed code doesn't change. The only difference is that the trimmed output has no PDBs. This is backlog item **BUILD-8** of the PowerToys speed & quality backlog. ## PR Checklist - [ ] Closes: N/A (backlog item BUILD-8, no GitHub issue) - [x] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected (part of the speed & quality backlog) - [ ] **Tests:** Added/updated and all pass (N/A: build-configuration change only, no product code. Validated with a local trimmed publish, repeated ILLink runs before and after the fix, and PR CI; see below) - [ ] **Localization:** All end-user-facing strings can be localized (N/A: no strings) - [ ] **Dev docs:** Added/updated (N/A) - [ ] **New binaries:** Added on the required places (N/A: no new binaries) - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: N/A ## Detailed Description of the Pull Request / Additional comments ### Root cause In the failing jobs, ILLink (`dotnet.exe`, Microsoft.NET.ILLink.Tasks 10.0.11) dies in its output step while it writes symbols: ``` Fatal error. 0xC0000005 at Mono.Cecil.Pdb.ISymUnmanagedWriter2.Close() at Mono.Cecil.Pdb.SymWriter.Close() at Mono.Cecil.Pdb.NativePdbWriter.Write() at Mono.Linker.CustomSymbolWriter.Write() ... at Mono.Linker.Steps.OutputStep.WriteAssembly(Mono.Cecil.AssemblyDefinition, System.String) ``` - **ILLink runs in every Release CI build.** `SamplePagesExtension.csproj` sets `GenerateAppxPackageOnBuild=true`, and in Release it also sets `PublishTrimmed=true`. MSIX packaging during the solution build publishes the app with `win-$(Platform).pubxml`, so `_RunILLink` trims it. It's the only ILLink invocation in PR CI. - **ILLink rewrites the symbols.** `TrimmerRemoveSymbols` defaults to `false` unless `DebuggerSupport=false`, so ILLink gets `-b` and rewrites the PDB of every assembly it trims. - **Those symbols are Windows PDBs.** `src\Common.Dotnet.props` sets Release `DebugType=pdbonly`. That covers the sample and also `Microsoft.CommandPalette.Extensions.Toolkit`, a `ProjectReference` that's trimmed in the same ILLink run. Mono.Cecil can only write Windows PDBs through the native COM `ISymUnmanagedWriter` (the .NET Framework's DiaSymReader). The access violation happens inside that native `Close()`, so we can't fix it in this repo. We can only stop using that writer. ### Affected builds and frequency I used the public Azure DevOps REST API to scan every build still retained for the **PowerToys CI** pipeline (shine-oss/PowerToys, 2026-08-31 to 2026-09-30), including earlier attempts of retried jobs. Each failure below has the same stack, in the SamplePagesExtension `_RunILLink` step: | Build | Trigger | Queued (UTC) | Failed job | | --- | --- | --- | --- | | [388605](https://dev.azure.com/shine-oss/PowerToys/_build/results?buildId=388605) | `main` (batched CI) | 2026-09-11 | Build Release_x64 | | [389658](https://dev.azure.com/shine-oss/PowerToys/_build/results?buildId=389658) | PR #50843 | 2026-09-28 | Build Release_arm64 | | [389667](https://dev.azure.com/shine-oss/PowerToys/_build/results?buildId=389667) | PR #50186 | 2026-09-28 | Build Release_arm64 | | [389871](https://dev.azure.com/shine-oss/PowerToys/_build/results?buildId=389871) | PR #50507 | 2026-09-30 | Build Release_x64 | | [389888](https://dev.azure.com/shine-oss/PowerToys/_build/results?buildId=389888) | PR #50884 | 2026-09-30 | Build Release_x64 | - **Rate:** 5 of 255 builds (~2%) and 5 of 486 x64/arm64 build jobs (~1%). In the last ~2 weeks (2026-09-14 to 2026-09-30) it was 4 of 196 builds (390 build jobs). PR builds only show up in this project from 2026-09-22, and 4 of those 125 PR builds (~3%) failed this way, all in the last three days. It hits both x64 and arm64 jobs. All five failures used ILLink 10.0.11, and every build I sampled from 2026-09-15 on used .NET SDK 10.0.303, so this isn't a new-toolset regression. The 30 scheduled "PowerToys CI (Caching)" builds had no hits. - **Cost:** A hit fails the whole build job after ~30â34 minutes, and the job has to be re-run. A green build job takes ~53 minutes (median), and the test jobs wait for it. In 4 of the 5 builds, this crash was the job's only error. 389888 also had two unrelated unit-test failures in the same task. ### Fix and why | Option | Result | | --- | --- | | (a) `DebugType=portable` for the sample (preferred, because it would keep symbols) | **Not enough.** The sample's PDB becomes portable, but the same ILLink run still rewrites `Microsoft.CommandPalette.Extensions.Toolkit.pdb` as a Windows PDB (verified locally, see Validation), so the native writer is still used. Switching the Toolkit to portable PDBs would change the symbol format of a shipping CmdPal/SDK binary, which is out of scope here. | | (b) `TrimmerRemoveSymbols=true` (**chosen**) | ILLink runs without `-b`, so it doesn't read or write any symbols and never loads the native writer. The trimmed assemblies are otherwise identical. | | (c) Don't package or trim the sample in CI | Not needed. It would also drop CI coverage of the trimmed, AOT-compatible sample that extension authors copy. | Effects of (b): - The sample's trimmed publish output, and so its MSIX, has no PDBs (locally, the publish folder had none). The WinAppSDK MSIX targets skip `_GenerateAppxSymbolPackage` when the payload has no PDBs, so the sample no longer gets an `.appxsym` (confirmed in PR CI). The sample isn't shipped to users (it's not in the installer). - The untrimmed build output keeps its PDB (`<Platform>\Release\WinUI3Apps\CmdPalExtensions\SamplePagesExtension\SamplePagesExtension.pdb`). That's the PDB CI's "Stage Unique Symbols" step already picks up. - Debug builds don't trim, so the extension-author inner loop is unchanged. The property sits in the existing Release property group, and the repo-wide `DebugType` isn't touched. ### Other projects checked - **ILLink in PR CI:** I grepped every build task log of a green PR build ([389905](https://dev.azure.com/shine-oss/PowerToys/_build/results?buildId=389905)) for ILLink and ILC invocations. That covered x64 and arm64 (main build, BugReportTool, StylesReportTool, the seven "Publish âĶ for Packaging" steps, DSC, and the installer) plus the "Build SDK" job. The only `illink.dll` invocation is SamplePagesExtension, once per platform, and nothing runs ILC. - **Other trimmed or AOT projects:** `Microsoft.CmdPal.Ext.PowerToys`, `Microsoft.CommandPalette.Extensions.Toolkit`, `UITestAutomation`, PowerOCR, PowerDisplay (+CLI), and PowerAccent.UI set `PublishAot` or `PublishTrimmed`. PR CI only builds them and never publishes them, so neither ILLink nor ILC runs. CmdPal UI uses Native AOT only in the release pipeline (`EnableCmdPalAOT=true`). Native AOT trims with ILC, not ILLink/Cecil, so it doesn't use this writer. The release pipeline builds the same solution, so its SamplePagesExtension trim gets this fix too. No follow-ups needed. - **`src\modules\cmdpal\ExtensionTemplate`:** Not built in CI (it's not in `PowerToys.slnx`, and `verifyCommonProps.ps1` skips it). It doesn't import `Common.Dotnet.props` or set `DebugType`, so it uses the SDK default portable PDBs. The Toolkit it gets from NuGet ships without a PDB, so a trimmed template publish only uses Cecil's managed portable-PDB writer. No template change needed. ## Validation Steps Performed **Local environment limits:** The dev box is ARM64 with VS 18 and has no C++ toolset or Windows SDK, so there's no `MakeAppx` and MSIX packaging can't run locally. I exercised ILLink with a Release|x64 publish that skips packaging: ```powershell tools\build\build.ps1 -Platform x64 -Configuration Release /restore /t:Publish /p:GenerateAppxPackageOnBuild=false ` /p:CsWinRTWindowsMetadata=<Microsoft.Windows.SDK.NET.Ref>\winmd ` /p:CustomAfterMicrosoftCommonTargets=<local hook that swaps .vcxproj references for prebuilt winmd/dll> ``` The hook is local only and isn't part of this PR. Restores were anonymous (`NUGET_PLUGIN_PATHS`, `NUGET_NETFX_PLUGIN_PATHS`, and `NUGET_NETCORE_PLUGIN_PATHS` pointed at an empty folder), and no new packages were needed. Local ILLink is 10.0.12 (SDK 10.0.401); CI uses 10.0.11. To tell PDB formats apart, I checked the file header: a Windows PDB starts with `Microsoft C/C++ MSF 7.00` and a portable PDB starts with `BSJB`. 1. **Before (`main`):** Publish exits 0. ILLink runs with `-b` and writes `SamplePagesExtension.pdb` and `Microsoft.CommandPalette.Extensions.Toolkit.pdb` as Windows PDBs, which means the native writer is used. 2. **Option (a), not committed:** `SamplePagesExtension.pdb` becomes portable (`BSJB`), but ILLink still writes `Microsoft.CommandPalette.Extensions.Toolkit.pdb` as a Windows PDB. 3. **After (this PR):** Publish exits 0 with the same two `IL2104` warnings as `main` (both already in `WarningsNotAsErrors`). The ILLink task gets `RemoveSymbols=True`, so there's no `-b`, and it returns `_ILLinkExitCode=0` without writing any PDBs. Compared with the "before" output, the trimmed `SamplePagesExtension.dll` (171 types, 743 methods, 251 fields) and `Microsoft.CommandPalette.Extensions.Toolkit.dll` (399 types, 1850 methods, 530 fields) have the same metadata counts and code size. The only change is the missing CodeView debug-directory entry (the pointer to the PDB). 4. **Property evaluation:** `dotnet msbuild -getProperty` gives `TrimmerRemoveSymbols=true` for Release|x64 and Release|ARM64 and leaves it empty for Debug, where `PublishTrimmed=false`. 5. **Repeated trims:** I replayed the exact ILLink command from the MSBuild log 20 times with the native arm64 `dotnet` and 20 times with the emulated x64 `dotnet` (closer to the x64 CI agents), before and after the fix. - Before: 40/40 passed, plus 10 more runs with `DOTNET_GCgen0size=0x20000`. Every run wrote two Windows PDBs through the native writer. - After: 40/40 passed, and no PDBs were written. - The crash didn't reproduce locally. That's consistent with a ~1% per-job rate, so PR CI is the real gate. 6. **PR CI ([389926](https://dev.azure.com/shine-oss/PowerToys/_build/results?buildId=389926)): green on x64 and arm64.** All jobs passed: Build Release_x64 (build + tests), Build Release_arm64, Build SDK, and both "Resolve affected UI test projects" jobs. In both main-build logs: - SamplePagesExtension's ILLink ran once (`illink.dll -a "obj\x64\Release\SamplePagesExtension.dll"` and `obj\ARM64\...`, ILLink 10.0.11) without `-b`. It has 253 arguments versus 254 in build 389905, which doesn't have this change, and the only one missing is `-b`. - There were no `Fatal error`, `0xC0000005`, `57005`, or `NETSDK1144` markers. The task had 0 errors and the same two `IL2104` warnings as 389905. - MSIX packaging still produced `SamplePagesExtension_1.0.0.0_x64.msix` and `SamplePagesExtension_1.0.0.0_arm64.msix`, and it skipped the sample's `.appxsym` as expected. - "Stage Unique Symbols" still links 202 PDBs per platform, the same as 389905, and that includes the untrimmed `SamplePagesExtension.pdb`. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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
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.
Summary of the Pull Request
This PR fixes context item shortcuts being invoked twice, which could navigate to the same page twice and require two Back or Escape presses. It also unifies context menu behavior across the palette and dock.
CommandBar, allowing the bar to be replaced or hidden without losing keyboard shortcuts.Pictures? Pictures!
Primary and secondary command shows either default shortcuts...
... but a custom requested shortcut takes precedence in the menu:
PR Checklist
RequestedShortcutnavigates twice, requiring two Esc / two Back presses to leave the page #48725Detailed Description of the Pull Request / Additional comments
Validation Steps Performed