diff --git a/src/Reactor/Core/Reconciler.cs b/src/Reactor/Core/Reconciler.cs index fc2983b1..61268073 100644 --- a/src/Reactor/Core/Reconciler.cs +++ b/src/Reactor/Core/Reconciler.cs @@ -2683,14 +2683,45 @@ public static void ApplyDefaultAutomationName(FrameworkElement fe, string? capti public static void UpdateDefaultAutomationName(FrameworkElement fe, string? oldCaption, string? newCaption) { if (fe is null) return; + // Two arms: a whitespace new caption has nothing to write, so we return before + // even reading the Name (main also returns first on whitespace newCaption, so + // this is behavior-identical — it just avoids that one interop read). For any + // NON-whitespace caption we still read the live Name and let the pure helper + // decide. The actual P3 saving lives inside that helper: it skips the SetName + // write when the live Name already equals the caption-derived default (the + // redundant same-value write). GetName is NOT skipped for unchanged captions — + // only the redundant SetName is. We perform the SetName write only when the helper + // returns a value (the GetName read above always runs for a non-whitespace caption). if (string.IsNullOrWhiteSpace(newCaption)) return; var current = Microsoft.UI.Xaml.Automation.AutomationProperties.GetName(fe); + var resolved = ResolveDefaultAutomationNameUpdate(current, oldCaption, newCaption); + if (resolved is not null) + Microsoft.UI.Xaml.Automation.AutomationProperties.SetName(fe, resolved); + } + + // Pure decision for UpdateDefaultAutomationName — no DP/UIA interop, so the + // caption/override policy is unit-testable headlessly (Reactor.Tests). Returns + // the Name to write, or null to leave the live automation Name untouched. Models + // main's GetName + author-override + SetName logic exactly, plus one safe saving: + // the idempotent-write guard below. + internal static string? ResolveDefaultAutomationNameUpdate(string? current, string? oldCaption, string? newCaption) + { + if (string.IsNullOrWhiteSpace(newCaption)) return null; bool authorOverride = !string.IsNullOrEmpty(current) && (oldCaption is null || !string.Equals(current, oldCaption, StringComparison.Ordinal)); - if (authorOverride) return; + if (authorOverride) return null; var trimmed = newCaption.Length > 100 ? newCaption.Substring(0, 100) : newCaption; - Microsoft.UI.Xaml.Automation.AutomationProperties.SetName(fe, trimmed); + // Idempotent-write guard — the P3 saving. When the live Name already equals the + // caption-derived default, the SetName main would issue is a value no-op, so skip + // it (this is the steady-state hot path: an unchanged caption means current == + // trimmed). Crucially we still WRITE when the default must be (re)applied — e.g. + // a modifier removal this render cleared the Name (current empty) even though the + // caption itself is unchanged — so a removed .AutomationName() override correctly + // falls back to the caption default, matching main. Behaviorally identical to main + // minus the redundant same-value write. + if (string.Equals(current, trimmed, StringComparison.Ordinal)) return null; + return trimmed; } internal static string? ExtractElementCaption(Element? element) => element switch diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/CoreCoverageFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/CoreCoverageFixtures.cs index 4d829576..f0a203fb 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/CoreCoverageFixtures.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/CoreCoverageFixtures.cs @@ -751,7 +751,7 @@ public override async Task RunAsync() .Landmark(Microsoft.UI.Xaml.Automation.Peers.AutomationLandmarkType.Main) ); } - else + else if (phase == 1) { return VStack( Button("UpdateA11y", () => set(2)), @@ -768,6 +768,19 @@ public override async Task RunAsync() .TabNavigation(Microsoft.UI.Xaml.Input.KeyboardNavigationMode.Cycle) ); } + else + { + // phase 2: the .AutomationName() override is removed while the caption + // ("Accessible") is unchanged. ApplyModifiers clears the live UIA Name; + // the P3 idempotent-write guard must still restore the caption-derived + // default (current "" != caption => a real write) rather than leave the + // Name empty. Exercises the live seam's restore-default branch end-to-end. + return VStack( + Button("UpdateA11y", () => set(3)), + TextBlock("Accessible") + .HelpText("Updated help text") + ); + } }); await Harness.Render(); @@ -776,6 +789,9 @@ public override async Task RunAsync() H.Check("A11y_Mounted", tb is not null); H.Check("A11y_HelpText", Microsoft.UI.Xaml.Automation.AutomationProperties.GetHelpText(tb!) == "This is help text"); + // The author-set .AutomationName() override wins over the caption default at mount. + H.Check("A11y_Name_Override", + Microsoft.UI.Xaml.Automation.AutomationProperties.GetName(tb!) == "test-text"); // Update accessibility modifiers H.ClickButton("UpdateA11y"); @@ -786,6 +802,19 @@ public override async Task RunAsync() H.Check("A11y_LiveSetting", Microsoft.UI.Xaml.Automation.AutomationProperties.GetLiveSetting(tb!) == Microsoft.UI.Xaml.Automation.Peers.AutomationLiveSetting.Polite); + // A changed override still flows through (the author name updates, not the caption). + H.Check("A11y_Name_OverrideUpdated", + Microsoft.UI.Xaml.Automation.AutomationProperties.GetName(tb!) == "test-text-updated"); + + // Remove the .AutomationName() override with the caption unchanged. ApplyModifiers + // clears the Name; the P3 guard (live seam) must restore the caption-derived default + // "Accessible" rather than leave it empty. TEETH: a blanket unchanged-caption skip + // (or dropping the live SetName) leaves the Name cleared here and this flips. + H.ClickButton("UpdateA11y"); + await Harness.Render(); + tb = H.FindText("Accessible"); + H.Check("A11y_Name_RestoredToCaptionDefault", + Microsoft.UI.Xaml.Automation.AutomationProperties.GetName(tb!) == "Accessible"); } } diff --git a/tests/Reactor.Tests/ReconcilerAutomationNameTests.cs b/tests/Reactor.Tests/ReconcilerAutomationNameTests.cs new file mode 100644 index 00000000..cbd72c3d --- /dev/null +++ b/tests/Reactor.Tests/ReconcilerAutomationNameTests.cs @@ -0,0 +1,112 @@ +using System; +using Microsoft.UI.Reactor.Core; +using Xunit; + +namespace Microsoft.UI.Reactor.Tests; + +/// +/// Pins the decision policy of via its +/// pure, DP-free helper (P3 — trim +/// the redundant per-cell automation write). +/// +/// The optimization is an idempotent-write guard: the live method still reads the UIA Name, but +/// skips the SetName write when the Name already equals the caption-derived default (the +/// steady-state hot path). These tests pin that saving AND the three correctness guarantees that +/// must survive it: an author-set Name is never clobbered, a genuine caption change still flows +/// through, and a Name cleared this render (e.g. a removed .AutomationName() override) is +/// restored to the caption default even when the caption itself is unchanged. +/// +public class ReconcilerAutomationNameTests +{ + // ──────────────────────────────────────────────────────────────── + // The P3 idempotent-write guard (teeth: revert it → these flip) + // ──────────────────────────────────────────────────────────────── + + [Fact] + public void Unchanged_Caption_Skips_Write_When_Name_Already_Matches() + { + // Steady-state hot path: the live Name already equals the caption-derived default, so the + // SetName main would issue is a value no-op → return null (skip). + // TEETH: remove the `current == trimmed` guard and the helper falls through to + // `return trimmed` ("X", a redundant write) → this assertion fails. + Assert.Null(Reconciler.ResolveDefaultAutomationNameUpdate(current: "X", oldCaption: "X", newCaption: "X")); + } + + [Fact] + public void Cleared_Name_Restores_Caption_Default_When_Caption_Unchanged() + { + // Regression guard (the bug a blanket unchanged-caption skip would introduce): a removed + // `.AutomationName()` override makes ApplyModifiers clear the live Name to empty *before* + // this runs, even though the caption is unchanged. The default must be re-applied — so an + // empty current with an unchanged caption "X" resolves to a WRITE of "X", matching main. + // TEETH the other way: re-add `if (oldCaption == newCaption) return null;` → this fails. + Assert.Equal("X", Reconciler.ResolveDefaultAutomationNameUpdate(current: "", oldCaption: "X", newCaption: "X")); + } + + [Theory] + [InlineData("X", null)] + [InlineData("X", "")] + [InlineData("X", " ")] + public void Empty_Or_Whitespace_New_Caption_Returns_Null(string? current, string? newCaption) + { + // No caption to project onto the Name → never touch it (matches the original guard). + Assert.Null(Reconciler.ResolveDefaultAutomationNameUpdate(current, oldCaption: "anything", newCaption)); + } + + // ──────────────────────────────────────────────────────────────── + // Author-override preservation (MED-risk invariant — must not regress) + // ──────────────────────────────────────────────────────────────── + + [Fact] + public void Author_Override_Survives_Caption_Change() + { + // The live Name ("custom") differs from the previous caption ("A") → the author set it. + // A caption change A→B must NOT clobber the author's value. + Assert.Null(Reconciler.ResolveDefaultAutomationNameUpdate(current: "custom", oldCaption: "A", newCaption: "B")); + } + + [Fact] + public void Author_Override_Survives_When_Old_Caption_Unknown() + { + // oldCaption null but a non-empty live Name is present → treat as author-owned, leave it. + Assert.Null(Reconciler.ResolveDefaultAutomationNameUpdate(current: "custom", oldCaption: null, newCaption: "B")); + } + + [Fact] + public void Author_Override_Survives_Unchanged_Caption() + { + // Unchanged caption "X" but the live Name is an author override ("custom" ≠ oldCaption) → + // the idempotent guard must NOT fire (custom ≠ trimmed "X"); author-override wins → null. + Assert.Null(Reconciler.ResolveDefaultAutomationNameUpdate(current: "custom", oldCaption: "X", newCaption: "X")); + } + + // ──────────────────────────────────────────────────────────────── + // The default still follows a genuine caption change + // ──────────────────────────────────────────────────────────────── + + [Fact] + public void Default_Follows_Caption_Change() + { + // Live Name equals the previous caption ("A") → our default owns it → update to "B". + Assert.Equal("B", Reconciler.ResolveDefaultAutomationNameUpdate(current: "A", oldCaption: "A", newCaption: "B")); + } + + [Theory] + [InlineData("")] + [InlineData(null)] + public void FirstTime_Set_When_Live_Name_Empty_And_Caption_Changed(string? current) + { + // No author Name yet + a real (changed) caption → set it. + Assert.Equal("B", Reconciler.ResolveDefaultAutomationNameUpdate(current, oldCaption: "A", newCaption: "B")); + } + + [Fact] + public void Long_Changed_Caption_Is_Trimmed_To_100_Chars() + { + var longCaption = new string('a', 250); + var resolved = Reconciler.ResolveDefaultAutomationNameUpdate(current: "old", oldCaption: "old", newCaption: longCaption); + Assert.NotNull(resolved); + Assert.Equal(100, resolved!.Length); + Assert.Equal(new string('a', 100), resolved); + } +}