diff --git a/ILSpy.Tests/MainWindow/MainMenuTests.cs b/ILSpy.Tests/MainWindow/MainMenuTests.cs index 50ce4f43c..b770a8e9f 100644 --- a/ILSpy.Tests/MainWindow/MainMenuTests.cs +++ b/ILSpy.Tests/MainWindow/MainMenuTests.cs @@ -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 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 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().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(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().Select(i => i.Header) - .Should().Contain("About (second window)") - .And.NotContain("About (first window)"); + appMenu!.Items.OfType().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 promoted) + { + foreach (var item in promoted) + appMenu.Items.Remove(item); + } + + static NativeMenu WindowMenuWithHelpItems(string header, out Dictionary 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(StringComparer.Ordinal) { ["_Help"] = help }; + return root; } // NativeMenuItem.Gesture is display-only when NativeMenuBar renders the menu inline diff --git a/ILSpy/Views/MainMenu.axaml.cs b/ILSpy/Views/MainMenu.axaml.cs index 68aeca559..710d34700 100644 --- a/ILSpy/Views/MainMenu.axaml.cs +++ b/ILSpy/Views/MainMenu.axaml.cs @@ -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 promotedHelpItems = new(); + // The Help items the most recent PromoteHelpToMacAppMenu put into the app-level NativeMenu. + static List 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 // 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 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 PromoteHelpToMacAppMenu(NativeMenu rootMenu, Dictionary topLevelByTag) { - WithdrawPromotedHelpItems(); + WithdrawHelpItems(promotedHelpItems); + var promoted = new List(); 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 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(