Browse Source

Withdraw only the Help items the closing window itself promoted

The app-level NativeMenu is process-wide, so the withdrawal a window does
on Closed has to name the items that window put there. Withdrawing
"whatever is promoted right now" is correct only while one window exists
at a time: with two, closing the older one takes the newer one's About /
Check for Updates out of the macOS app menu, and nothing ever puts them
back. Not reachable today - MainWindow is [Shared] and Attach runs from
its ctor - but the failure mode is silent and permanent, and carrying the
list costs nothing. Removing an item that is already gone is a no-op, so
a superseded window's Closed stays harmless.

The promotion tests also have to leave the app menu as they found it:
it is declared on Application and outlives the test, it is not gated on
macOS, and on Windows and Linux nothing re-promotes over the leftovers.

Assisted-by: Claude:claude-opus-5:Claude Code
pull/4012/head
Christoph Wille 1 month ago committed by Siegfried Pammer
parent
commit
d4212b1965
  1. 76
      ILSpy.Tests/MainWindow/MainMenuTests.cs
  2. 44
      ILSpy/Views/MainMenu.axaml.cs

76
ILSpy.Tests/MainWindow/MainMenuTests.cs

@ -17,6 +17,7 @@ @@ -17,6 +17,7 @@
// DEALINGS IN THE SOFTWARE.
using System;
using System.Collections.Generic;
using System.Linq;
using Avalonia;
@ -106,25 +107,70 @@ public class MainMenuTests @@ -106,25 +107,70 @@ public class MainMenuTests
var appMenu = NativeMenu.GetMenu(Application.Current!);
appMenu.Should().NotBeNull("App.axaml declares the NativeMenu the Help items move into");
static (NativeMenu Root, System.Collections.Generic.Dictionary<string, NativeMenuItem> ByTag) WindowMenuWithHelp(string header)
MainMenu.PromoteHelpToMacAppMenu(
WindowMenuWithHelpItems("About (first window)", out var firstByTag), firstByTag);
var afterFirst = appMenu!.Items.Count;
var promoted = MainMenu.PromoteHelpToMacAppMenu(
WindowMenuWithHelpItems("About (second window)", out var secondByTag), secondByTag);
try
{
appMenu.Items.Count.Should().Be(afterFirst, "the second window's Help items replace the first window's");
appMenu.Items.OfType<NativeMenuItem>().Select(i => i.Header)
.Should().Contain("About (second window)")
.And.NotContain("About (first window)");
}
finally
{
var help = new NativeMenuItem { Header = "_Help", Menu = new NativeMenu() };
help.Menu.Items.Add(new NativeMenuItem { Header = header });
var root = new NativeMenu();
root.Items.Add(help);
return (root, new System.Collections.Generic.Dictionary<string, NativeMenuItem>(StringComparer.Ordinal) { ["_Help"] = help });
RestoreAppMenu(appMenu, promoted);
}
}
var first = WindowMenuWithHelp("About (first window)");
MainMenu.PromoteHelpToMacAppMenu(first.Root, first.ByTag);
var afterFirst = appMenu!.Items.Count;
var second = WindowMenuWithHelp("About (second window)");
MainMenu.PromoteHelpToMacAppMenu(second.Root, second.ByTag);
// The Help items a window promotes are withdrawn when it closes, but only that window's own:
// a window closing after a second one has promoted its items must leave those in the app menu,
// or macOS shows an app menu with no About / Check for Updates while the second window is still
// on screen and nothing ever puts them back.
[AvaloniaTest]
public void Closing_An_Earlier_Window_Leaves_A_Later_Window_Help_Items_In_Place()
{
var appMenu = NativeMenu.GetMenu(Application.Current!);
appMenu.Should().NotBeNull("App.axaml declares the NativeMenu the Help items move into");
var first = MainMenu.PromoteHelpToMacAppMenu(
WindowMenuWithHelpItems("About (first window)", out var firstByTag), firstByTag);
var second = MainMenu.PromoteHelpToMacAppMenu(
WindowMenuWithHelpItems("About (second window)", out var secondByTag), secondByTag);
try
{
// What the first window's Closed handler does, now that the second window has promoted.
MainMenu.WithdrawHelpItems(first);
appMenu.Items.Count.Should().Be(afterFirst, "the second window's Help items replace the first window's");
appMenu.Items.OfType<NativeMenuItem>().Select(i => i.Header)
.Should().Contain("About (second window)")
.And.NotContain("About (first window)");
appMenu!.Items.OfType<NativeMenuItem>().Select(i => i.Header)
.Should().Contain("About (second window)",
"the still-open window's Help items must survive an earlier window closing");
}
finally
{
RestoreAppMenu(appMenu!, second);
}
}
// The app menu is declared on Application and outlives every test, so a test that promotes
// placeholder items into it has to take them back out; otherwise a later test reading it
// (see MainMenu_top_level_items_are_File_View_Window_in_order) sees this test's leftovers.
static void RestoreAppMenu(NativeMenu appMenu, List<NativeMenuItemBase> promoted)
{
foreach (var item in promoted)
appMenu.Items.Remove(item);
}
static NativeMenu WindowMenuWithHelpItems(string header, out Dictionary<string, NativeMenuItem> byTag)
{
var help = new NativeMenuItem { Header = "_Help", Menu = new NativeMenu() };
help.Menu.Items.Add(new NativeMenuItem { Header = header });
var root = new NativeMenu();
root.Items.Add(help);
byTag = new Dictionary<string, NativeMenuItem>(StringComparer.Ordinal) { ["_Help"] = help };
return root;
}
// NativeMenuItem.Gesture is display-only when NativeMenuBar renders the menu inline

44
ILSpy/Views/MainMenu.axaml.cs

@ -73,16 +73,16 @@ public static class MainMenu @@ -73,16 +73,16 @@ public static class MainMenu
if (OperatingSystem.IsMacOS())
{
TranslateGesturesForMacOS(menu);
PromoteHelpToMacAppMenu(menu, topLevelByTag);
window.Closed += (_, _) => WithdrawPromotedHelpItems();
var promoted = PromoteHelpToMacAppMenu(menu, topLevelByTag);
window.Closed += (_, _) => WithdrawHelpItems(promoted);
}
RegisterGestureKeyBindings(window, menu);
NativeMenu.SetMenu(window, menu);
}
// The Help items currently promoted into the app-level NativeMenu (see PromoteHelpToMacAppMenu).
static readonly List<NativeMenuItemBase> promotedHelpItems = new();
// The Help items the most recent PromoteHelpToMacAppMenu put into the app-level NativeMenu.
static List<NativeMenuItemBase> promotedHelpItems = new();
// NativeMenuItem.Gesture is display-only when NativeMenuBar renders the menu inline:
// the managed fallback binds it to MenuItem.InputGesture, which never handles input.
@ -125,38 +125,46 @@ public static class MainMenu @@ -125,38 +125,46 @@ public static class MainMenu
// the exporter subscribes to that instance's Items, so inserting fires a re-export.
// Items go at the top, above the Services / Hide / Quit block the exporter appended.
// We then remove _Help from the window menu so the items don't appear in both places.
// The app menu outlives any one window, and each window's Help items are built over
// that window's own command instances, so the items an earlier window promoted are
// taken out first (and again when the window closes); leaving them in would keep
// that window's command graph, and everything those commands reach, alive for the
// life of the process.
internal static void PromoteHelpToMacAppMenu(NativeMenu rootMenu, Dictionary<string, NativeMenuItem> topLevelByTag)
// The app menu outlives any one window, and each window's Help items are built over that
// window's own command instances, so the items an earlier window promoted are taken out
// first, and each window takes its own back out when it closes; leaving them in would keep
// that window's command graph, and everything those commands reach, alive for the life of
// the process. The returned list is what that window has to withdraw.
internal static List<NativeMenuItemBase> PromoteHelpToMacAppMenu(NativeMenu rootMenu, Dictionary<string, NativeMenuItem> topLevelByTag)
{
WithdrawPromotedHelpItems();
WithdrawHelpItems(promotedHelpItems);
var promoted = new List<NativeMenuItemBase>();
if (Application.Current is null)
return;
return promoted;
var appMenu = NativeMenu.GetMenu(Application.Current);
if (appMenu is null)
return;
return promoted;
if (!topLevelByTag.TryGetValue("_Help", out var helpItem) || helpItem.Menu is null)
return;
return promoted;
var index = 0;
foreach (var item in helpItem.Menu.Items.ToArray())
{
helpItem.Menu.Items.Remove(item);
appMenu.Items.Insert(index++, item);
promotedHelpItems.Add(item);
promoted.Add(item);
}
rootMenu.Items.Remove(helpItem);
topLevelByTag.Remove("_Help");
promotedHelpItems = promoted;
return promoted;
}
static void WithdrawPromotedHelpItems()
// Takes exactly the listed items back out of the app-level NativeMenu -- not "whatever is
// promoted right now". A window that closes after a later window promoted its own Help items
// must leave those in place, or the app menu ends up with no About / Check for Updates while
// that later window is still on screen. Removing an item that is no longer there is a no-op,
// so withdrawing a superseded window's list is harmless.
internal static void WithdrawHelpItems(List<NativeMenuItemBase> items)
{
var appMenu = Application.Current is null ? null : NativeMenu.GetMenu(Application.Current);
foreach (var item in promotedHelpItems)
foreach (var item in items)
appMenu?.Items.Remove(item);
promotedHelpItems.Clear();
items.Clear();
}
static bool TryGetExports(

Loading…
Cancel
Save