fix: do not activate a disabled toggle from a tray icon click [patch]

TrayApp.Test/TrayApplicationTests.cs

@@ -122,4 +122,119 @@ public void Dispose_IsIdempotent()
 		// call throws, and the lifetime's Exit handler has usually disposed it already by then.
 		Assert.AreEqual(0, stops);
 	}
+
+	[TestMethod]
+	public void TrayIconClick_WithTheToggleDisabled_DoesNotRunTheToolsSetter()
+	{
+		int setterCalls = 0;
+		bool available = false;
+
+		TrayAppDefinition app = TrayAppBuilder.Create("demo")
+			.DisplayName("Demo")
+			.Toggle("Keep awake", () => false, _ => setterCalls++, isEnabled: () => available)
+			.Build();
+
+		using TrayApplication host = Host(app, out TrayMenu menu);
+
+		// What painting the menu would have read. IsEnabled is a cache of the last refresh, so without this
+		// the item still carries its default of enabled and the assertion would pass for the wrong reason.
+		menu.Refresh();
+
+		host.OnTrayIconClicked(this, EventArgs.Empty);
+
+		// The menu shows this greyed out, so a click on the icon has to agree with it. The native menu entry
+		// is refused by the toolkit; the icon is the path with nothing in the way.
+		Assert.AreEqual(0, setterCalls);
+	}
+
+	[TestMethod]
+	public void TrayIconClick_WithTheToggleEnabled_RunsTheToolsSetter()
+	{
+		int setterCalls = 0;
+
+		TrayAppDefinition app = TrayAppBuilder.Create("demo")
+			.DisplayName("Demo")
+			.Toggle("Keep awake", () => false, _ => setterCalls++, isEnabled: () => true)
+			.Build();
+
+		using TrayApplication host = Host(app, out TrayMenu menu);
+		menu.Refresh();
+
+		host.OnTrayIconClicked(this, EventArgs.Empty);
+
+		// The fast path still has to work. Guarding it is worth nothing if it guards everything.
+		Assert.AreEqual(1, setterCalls);
+	}
+
+	[TestMethod]
+	public void TrayIconClick_BecomingEnabledAgain_RunsTheToolsSetter()
+	{
+		int setterCalls = 0;
+		bool available = false;
+
+		TrayAppDefinition app = TrayAppBuilder.Create("demo")
+			.DisplayName("Demo")
+			.Toggle("Keep awake", () => false, _ => setterCalls++, isEnabled: () => available)
+			.Build();
+
+		using TrayApplication host = Host(app, out TrayMenu menu);
+
+		menu.Refresh();
+		host.OnTrayIconClicked(this, EventArgs.Empty);
+
+		available = true;
+		menu.Refresh();
+		host.OnTrayIconClicked(this, EventArgs.Empty);
+
+		// Refused while the tool said it was unavailable, served once it said otherwise. The guard reads the
+		// refreshed state rather than latching on the first answer.
+		Assert.AreEqual(1, setterCalls);
+	}
+
+	[TestMethod]
+	public void Activate_WithTheItemDisabled_DoesNotRunTheToolsSetter()
+	{
+		int setterCalls = 0;
+		bool available = true;
+
+		TrayAppDefinition app = TrayAppBuilder.Create("demo")
+			.DisplayName("Demo")
+			.Toggle("Keep awake", () => false, _ => setterCalls++, isEnabled: () => available)
+			.Build();
+
+		using TrayApplication host = Host(app, out TrayMenu menu);
+
+		menu.Refresh();
+		available = false;
+		menu.Refresh();
+
+		TrayMenuItem toggle = menu.Items.First(item => item is TrayToggleItem);
+		host.Activate(toggle);
+
+		// The backstop behind the native menu. The toolkit will not raise Click for a greyed-out item, but
+		// it greys it out as of the last refresh, and a click already on its way when the tool became
+		// unavailable still arrives here. NativeMenuItem exposes Click with no way to raise it, so this
+		// calls the handler's target rather than the event.
+		Assert.AreEqual(0, setterCalls);
+	}
+
+	[TestMethod]
+	public void TrayIconClick_WithNoToggleAtAll_DoesNothing()
+	{
+		int commandCalls = 0;
+
+		TrayAppDefinition app = TrayAppBuilder.Create("demo")
+			.DisplayName("Demo")
+			.Command("Two", () => commandCalls++)
+			.Build();
+
+		using TrayApplication host = Host(app, out TrayMenu menu);
+		menu.Refresh();
+
+		host.OnTrayIconClicked(this, EventArgs.Empty);
+
+		// The fast path is for toggles. A menu of commands must not have one picked for it, and quit least
+		// of all.
+		Assert.AreEqual(0, commandCalls);
+	}
 }

TrayApp/Tray/TrayApplication.cs

@@ -176,11 +176,25 @@ private void BuildNativeMenu()
 		}
 	}
 
-	private void OnTrayIconClicked(object? sender, EventArgs e)
+	/// <summary>
+	/// Flips the first toggle the user could have clicked in the menu.
+	/// </summary>
+	/// <remarks>
+	/// A disabled toggle is skipped, because this is the only activation path with nothing between the
+	/// click and the tool's setter. The menu path is projected onto <see cref="NativeMenuItem"/>s that
+	/// the toolkit greys out and refuses to raise <c>Click</c> for, so the tray icon is where a tool's
+	/// <c>isEnabled</c> predicate would otherwise be silently ignored - the menu would show the action
+	/// as unavailable while a click on the icon ran it anyway.
+	/// <para>
+	/// Internal so that it can be driven without a platform: everything above this reads menu state
+	/// that <see cref="TrayMenu.Refresh"/> populates with no display attached.
+	/// </para>
+	/// </remarks>
+	internal void OnTrayIconClicked(object? sender, EventArgs e)
 	{
 		foreach ((TrayMenuItem model, _) in entries)
 		{
-			if (model is TrayToggleItem)
+			if (model is TrayToggleItem { IsEnabled: true })
 			{
 				Activate(model);
 				return;
@@ -192,8 +206,30 @@ private void OnTrayIconClicked(object? sender, EventArgs e)
 
 	private void OnExit(object? sender, ControlledApplicationLifetimeExitEventArgs e) => Dispose();
 
-	private void Activate(TrayMenuItem item)
+	/// <summary>
+	/// Runs one item's action and brings the menu back in line with it.
+	/// </summary>
+	/// <remarks>
+	/// The enabled check here is the backstop rather than the guard that matters. A native menu item is
+	/// greyed out at the last refresh, so the toolkit already refuses to raise <c>Click</c> for it - but
+	/// "at the last refresh" is the gap: a tool's state can change between the paint and the click, and
+	/// nothing repaints a menu the user is already looking at. <see cref="TrayMenuItem.IsEnabled"/> is
+	/// the value that painting used, so this agrees with what the user saw rather than second-guessing
+	/// it.
+	/// <para>
+	/// Internal for the same reason as <see cref="OnTrayIconClicked"/>: the native <c>Click</c> that
+	/// reaches this cannot be raised from a test, since <see cref="NativeMenuItem"/> exposes the event
+	/// and no way to fire it, so the backstop is exercised by calling it.
+	/// </para>
+	/// </remarks>
+	/// <param name="item">The item to activate.</param>
+	internal void Activate(TrayMenuItem item)
 	{
+		if (!item.IsEnabled)
+		{
+			return;
+		}
+
 		bool succeeded = menu.Activate(item);
 
 		if (succeeded && item is TrayToggleItem toggle)