Skip to content

CmdPal: Unify context-menu behavior and shortcut handling - #50843

Merged
Michael Jolley (michaeljolley) merged 5 commits into
mainfrom
feature/50807-unify-context-menu-behavior
Sep 28, 2026
Merged

Michael Jolley (michaeljolley) merged 5 commits into
mainfrom
feature/50807-unify-context-menu-behavior

Conversation

@jiripolasek

@jiripolasek Jiří PolÃĄÅĄek (jiripolasek) commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

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.

  • Adds Ctrl+Enter, Ctrl+K, and extension shortcuts to focused dock items while preserving normal button activation and invocation context.
  • Moves item shortcuts and menu orchestration out of CommandBar, allowing the bar to be replaced or hidden without losing keyboard shortcuts.
  • Keeps keyboard actions tied to the live selection while preserving the command bar's 50ms debounce to avoid visual churn when browsing.
  • Opens menus with an explicit context, prepares their state on demand, refreshes open submenus when commands change, and releases subscriptions after closing.
  • Reserves Enter and Ctrl+Enter for host actions and displays shortcut hints according to each command's role. Nested menus no longer inherit misleading root action hints.
  • Handles deferred keyboard opens, cancellation, rapid close/reopen requests, focus restoration, and Narrator announcements.
  • Adds a new menu flyout with menu style animations (slide up/down) that follow actual placement, including screen edge reversals. May seem like quite a stretch for this PR, but this fixes menu behavior and avoid a workaround for WinUI that would make this code more complicated.

Pictures? Pictures!

Primary and secondary command shows either default shortcuts...

image

... but a custom requested shortcut takes precedence in the menu:

image

PR Checklist

  • Communication: I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected
  • Tests: Added/updated and all pass
  • Localization: All end-user-facing strings can be localized
  • Dev docs: Added/updated
  • New binaries: Added on the required places
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

- 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
@github-actions github-actions Bot added Product-Command Palette Refers to the Command Palette utility Ready for review labels Sep 28, 2026
@jiripolasek

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@michaeljolley
Michael Jolley (michaeljolley) merged commit 5ca3b33 into main Sep 28, 2026
11 of 13 checks passed
@michaeljolley
Michael Jolley (michaeljolley) deleted the feature/50807-unify-context-menu-behavior branch September 28, 2026 18:38
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Product-Command Palette Refers to the Command Palette utility Ready for review

Projects

None yet

3 participants