diff --git a/src/OpenClaw.Tray.WinUI/Automations/AutomationsRoot.cs b/src/OpenClaw.Tray.WinUI/Automations/AutomationsRoot.cs index afb60df43c..f9aa2ae40b 100644 --- a/src/OpenClaw.Tray.WinUI/Automations/AutomationsRoot.cs +++ b/src/OpenClaw.Tray.WinUI/Automations/AutomationsRoot.cs @@ -13,6 +13,7 @@ using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; using OpenClaw.Shared.Cron; +using OpenClawTray.Helpers; using OpenClawTray.Presentation; using static Microsoft.UI.Reactor.Factories; using static OpenClawTray.Automations.AutomationsVisuals; @@ -314,11 +315,12 @@ void RegisterRow(FrameworkElement row, string id) IsOpen = true, CloseButtonText = t["Cancel"], DefaultButton = ContentDialogButton.None, - OnOpened = () => { if (confirmDialogRef.Current is { } native) FocusConfirmationCancel(native); }, OnClosed = result => { confirmDialogRef.Current = null; pending.Result.TrySetResult(result == ContentDialogResult.Primary); setConfirm(null); }, }).Set(native => { confirmDialogRef.Current = native; + // Cancel must get initial focus deterministically, not race WinUI's own initial focus. + DialogInitialFocus.FocusCloseButton(native); ConfigureDialogWidth(native, dialogWidth); Microsoft.UI.Xaml.Automation.AutomationProperties.SetAutomationId(native, "AutomationsConfirmDialog"); Microsoft.UI.Xaml.Automation.AutomationProperties.SetName(native, pending.Title); @@ -384,19 +386,6 @@ private static Element RenderConfirmation(ConfirmRequest request, double width) .Padding(12).CornerRadius(6).Background(Theme.SubtleFill)) .MaxWidth(420); - private static void FocusConfirmationCancel(ContentDialog sender) => - sender.DispatcherQueue.TryEnqueue(Microsoft.UI.Dispatching.DispatcherQueuePriority.Low, - () => FindCloseButton(sender, sender.CloseButtonText)?.Focus(FocusState.Programmatic)); - - private static Button? FindCloseButton(DependencyObject parent, string label) - { - if (parent is Button button && (button.Name == "CloseButton" || string.Equals(button.Content as string, label, StringComparison.Ordinal))) - return button; - for (var i = 0; i < Microsoft.UI.Xaml.Media.VisualTreeHelper.GetChildrenCount(parent); i++) - if (FindCloseButton(Microsoft.UI.Xaml.Media.VisualTreeHelper.GetChild(parent, i), label) is { } close) - return close; - return null; - } private static Element RenderHeader(AutomationsHost host, AutomationsSnapshot state, AutomationsActions actions, AutomationsLook look, PageWidthTier tier) { diff --git a/src/OpenClaw.Tray.WinUI/Helpers/DialogInitialFocus.cs b/src/OpenClaw.Tray.WinUI/Helpers/DialogInitialFocus.cs new file mode 100644 index 0000000000..be72a15d9f --- /dev/null +++ b/src/OpenClaw.Tray.WinUI/Helpers/DialogInitialFocus.cs @@ -0,0 +1,108 @@ +// DialogInitialFocus: makes a ContentDialog's initial keyboard focus land on its Close (Cancel) button. +// +// When a ContentDialog opens, WinUI runs its own initial-focus transaction (with DefaultButton None +// it picks the first focusable element, for example selectable body text). A focus call merely +// queued from Opened can run before or after that transaction, so the outcome depends on timing. +// Redirecting the first GettingFocus into the dialog makes Cancel win every time. When a dialog +// opens right after another closes, Opened and even the first focus can arrive before the Cancel +// button is realized or focusable; the fallback then retries after each layout pass until Cancel takes +// focus. The fallback also covers a dialog that opened before this was attached. Once Cancel has focus, +// later focus moves are untouched. +// +// Usage (Reactor ContentDialog element): +// ContentDialog(...) with { DefaultButton = ContentDialogButton.None, CloseButtonText = "Cancel" } +// .Set(native => DialogInitialFocus.FocusCloseButton(native)); +using Microsoft.UI.Dispatching; +using Microsoft.UI.Xaml; +using Microsoft.UI.Xaml.Controls; +using Microsoft.UI.Xaml.Input; +using Microsoft.UI.Xaml.Media; + +namespace OpenClawTray.Helpers; + +internal static class DialogInitialFocus +{ + private const string CloseButtonPart = "CloseButton"; + + private static readonly DependencyProperty PendingProperty = DependencyProperty.RegisterAttached( + "Pending", typeof(object), typeof(DialogInitialFocus), new PropertyMetadata(null)); + + /// + /// Attaches once per native dialog and gives its first focus to Cancel. Callers mount a fresh keyed + /// dialog per confirmation (as AutomationsRoot does), so one open per native dialog is covered. + /// + public static void FocusCloseButton(ContentDialog dialog) + { + if (dialog.GetValue(PendingProperty) is not null) + return; + dialog.SetValue(PendingProperty, true); + dialog.GettingFocus += RedirectInitialFocus; + dialog.Opened += FocusOnOpened; + // Real user input ends the initial-focus duty, so a late fallback never takes focus back after + // the user starts interacting. Handled events count too (a TextBox handles its own keys). + dialog.AddHandler(UIElement.KeyDownEvent, new KeyEventHandler(EndOnUserInput), handledEventsToo: true); + dialog.AddHandler(UIElement.PointerPressedEvent, new PointerEventHandler(EndOnUserInput), handledEventsToo: true); + // A side-mounted dialog can open before the modifier that attaches this runs. + if (dialog.IsLoaded) + QueueFocus(dialog); + } + + internal static Button? FindCloseButton(DependencyObject parent) + { + if (parent is Button { Name: CloseButtonPart } button) + return button; + for (var index = 0; index < VisualTreeHelper.GetChildrenCount(parent); index++) + if (FindCloseButton(VisualTreeHelper.GetChild(parent, index)) is { } found) + return found; + return null; + } + + private static bool IsPending(ContentDialog dialog) => dialog.GetValue(PendingProperty) is true; + + private static void EndOnUserInput(object sender, RoutedEventArgs args) => + ((ContentDialog)sender).SetValue(PendingProperty, false); + + private static void RedirectInitialFocus(UIElement sender, GettingFocusEventArgs args) + { + var dialog = (ContentDialog)sender; + // Only the initial transaction (focus entering from outside); never redirect moves within the dialog. + if (!IsPending(dialog) || IsWithin(dialog, args.OldFocusedElement)) + return; + if (FindCloseButton(dialog) is not { } cancel || !args.TrySetNewFocusedElement(cancel)) + QueueFocus(dialog); + else + dialog.SetValue(PendingProperty, false); + } + + private static bool IsWithin(DependencyObject ancestor, DependencyObject? element) + { + for (var current = element; current is not null; current = VisualTreeHelper.GetParent(current)) + if (ReferenceEquals(current, ancestor)) + return true; + return false; + } + + private static void FocusOnOpened(ContentDialog sender, ContentDialogOpenedEventArgs args) => QueueFocus(sender); + + // Runs after WinUI's own initial-focus transaction; a no-op once Cancel has received focus. + private static void QueueFocus(ContentDialog dialog) => + dialog.DispatcherQueue.TryEnqueue(DispatcherQueuePriority.Low, () => FocusWhenRealized(dialog)); + + private static void FocusWhenRealized(ContentDialog dialog) + { + if (!IsPending(dialog) || !dialog.IsLoaded) + return; + if (FindCloseButton(dialog) is { } cancel && cancel.Focus(FocusState.Programmatic)) + { + dialog.SetValue(PendingProperty, false); + return; + } + // Cancel is not realized or not focusable yet: try again after the next layout pass. + void OnLayoutUpdated(object? sender, object args) + { + dialog.LayoutUpdated -= OnLayoutUpdated; + QueueFocus(dialog); + } + dialog.LayoutUpdated += OnLayoutUpdated; + } +} diff --git a/tests/OpenClaw.Tray.UITests/AutomationsUiTests.cs b/tests/OpenClaw.Tray.UITests/AutomationsUiTests.cs index 400b7345eb..4820453eb7 100644 --- a/tests/OpenClaw.Tray.UITests/AutomationsUiTests.cs +++ b/tests/OpenClaw.Tray.UITests/AutomationsUiTests.cs @@ -680,10 +680,12 @@ await WaitUiAsync(run, () => FindById(run, "AutomationsConfirmDialog") is not nu Assert.Equal($"It's removed from Fixture Gateway ({run.Profile.RunId[..8]}) along with its schedule and run history. This can't be undone.", removeText.DocumentRange.GetText(-1).TrimEnd('\r', '\n')); var removeDialog = FindById(run, "AutomationsConfirmDialog")!; - var cancel = removeDialog.FindFirst(TreeScope.Descendants, new AndCondition( - new PropertyCondition(AutomationElement.ControlTypeProperty, ControlType.Button), - new PropertyCondition(AutomationElement.NameProperty, "Cancel"))); - await WaitUiAsync(run, () => cancel.Current.HasKeyboardFocus, "Cancel receives destructive initial focus"); + // Re-acquire Cancel on every poll: a dialog re-render can replace the button's peer. + await WaitUiAsync(run, () => FindById(run, "AutomationsConfirmDialog")?.FindFirst(TreeScope.Descendants, + new AndCondition( + new PropertyCondition(AutomationElement.ControlTypeProperty, ControlType.Button), + new PropertyCondition(AutomationElement.NameProperty, "Cancel"))) is { Current.HasKeyboardFocus: true }, + "Cancel receives destructive initial focus"); Assert.Equal(ControlType.Window, removeDialog.Current.ControlType); Assert.True(((WindowPattern)removeDialog.GetCurrentPattern(WindowPattern.Pattern)).Current.IsModal); await ScanStateAsync("remove confirmation"); @@ -1564,8 +1566,11 @@ private static AutomationElement PageScrollPeer(GatewayFixtureRun run, bool dump var marker = window.FindFirst(TreeScope.Descendants, new PropertyCondition(AutomationElement.AutomationIdProperty, "CronPageMarker")); if (marker is null) return null; - Assert.True(NativeUiOwnership.IsOwnedDescendant(window, marker, run.AppProcessId), + // Polled while Automations re-renders: a marker peer that is detached or replaced proves nothing yet. + var markerEvidence = NativeUiOwnership.Classify(window, marker, run.AppProcessId); + Assert.True(markerEvidence != NativeUiOwnership.Evidence.Foreign, "Automations page marker must belong to the retained native window."); + if (markerEvidence != NativeUiOwnership.Evidence.Owned) return null; var markerIdentity = marker.GetRuntimeId(); bool IsMarker(AutomationElement? element) => element is not null && NativeUiOwnership.IsOwnedDescendant(window, element, run.AppProcessId) diff --git a/tests/OpenClaw.Tray.UITests/DialogInitialFocusTests.cs b/tests/OpenClaw.Tray.UITests/DialogInitialFocusTests.cs new file mode 100644 index 0000000000..12fd8017d2 --- /dev/null +++ b/tests/OpenClaw.Tray.UITests/DialogInitialFocusTests.cs @@ -0,0 +1,181 @@ +// Proves DialogInitialFocus gives a destructive ContentDialog's initial focus to Cancel even when the +// dialog content holds a focusable element: the first focus inside the dialog after Cancel exists is +// Cancel, and nothing else takes focus after it. Opened can fire before the template is realized, so +// focus is observed through the dialog's own GettingFocus, not sampled at Opened. A later focus move is +// never stolen back. Like AutomationsRoot, each confirmation mounts a fresh keyed dialog. +// Run with the DialogInitialFocusTests filter on an isolated Windows desktop. +using Microsoft.UI.Reactor; +using Microsoft.UI.Reactor.Core; +using Microsoft.UI.Reactor.Hosting; +using Microsoft.UI.Xaml; +using Microsoft.UI.Xaml.Controls; +using Microsoft.UI.Xaml.Input; +using OpenClawTray.Helpers; +using static Microsoft.UI.Reactor.Factories; + +namespace OpenClaw.Tray.UITests; + +[Collection(UICollection.Name)] +public sealed class DialogInitialFocusTests(UIThreadFixture ui) +{ + private static readonly TimeSpan DialogTransitionTimeout = TimeSpan.FromSeconds(10); + // Each run opens this many confirmations, each right after the previous one closes. + private const int Confirmations = 11; + + [Fact] + public async Task DestructiveDialog_CancelOwnsInitialFocusForEachConfirmationWithoutStealingLaterFocus() + { + await ui.ResetContainerAsync(); + ReactorHostControl? host = null; + Microsoft.UI.Xaml.Controls.ContentDialog? native = null; + Element? current = null; + var opened = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var closed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + // Each focus change routed through the dialog, read synchronously in GettingFocus after the helper's + // handler (so the final target after any redirect), with whether Cancel existed at that moment. + var focusLog = new List<(DependencyObject Element, bool CancelRealized)>(); + var recorded = new HashSet(ReferenceEqualityComparer.Instance); + void RecordFocus(UIElement sender, GettingFocusEventArgs args) + { + if (!args.Cancel && args.NewFocusedElement is { } element) + focusLog.Add((element, DialogInitialFocus.FindCloseButton(sender) is not null)); + } + void AssertCancelKeptFocus(Microsoft.UI.Xaml.Controls.ContentDialog dialog, string key) + { + var cancel = DialogInitialFocus.FindCloseButton(dialog)!; + Assert.Same(cancel, FocusManager.GetFocusedElement(dialog.XamlRoot)); + var inside = focusLog.Where(entry => IsWithin(dialog, entry.Element)).ToArray(); + // Before Cancel exists nothing can redirect to it; once it exists, the first focus must be Cancel. + var firstRealized = inside.FirstOrDefault(entry => entry.CancelRealized).Element; + Assert.True(ReferenceEquals(firstRealized, cancel), + $"{key}: first focus after Cancel was realized went to {firstRealized?.GetType().Name ?? "nothing"}."); + // WinUI can raise GettingFocus again for the same target; only a different element is a move. + var afterCancel = inside.SkipWhile(entry => !ReferenceEquals(entry.Element, cancel)) + .Where(entry => !ReferenceEquals(entry.Element, cancel)).ToArray(); + Assert.True(afterCancel.Length == 0, + $"{key}: focus moved inside the dialog after Cancel: {string.Join(", ", afterCancel.Select(entry => entry.Element.GetType().Name))}"); + } + Element Dialog(string key) => (ContentDialog("", TextBox("selectable body", _ => { }).AutomationName("Body"), "Remove") with + { + IsOpen = true, + CloseButtonText = "Cancel", + DefaultButton = ContentDialogButton.None, + OnClosed = _ => closed.TrySetResult(), + OnOpened = () => opened.TrySetResult(), + }).Set(dialog => + { + native = dialog; + DialogInitialFocus.FocusCloseButton(dialog); + // After the helper's handler, so the recorder sees the final target. OnMountAdd would receive + // Reactor's placeholder, not this native dialog. + AttachRecorder(dialog); + }).WithKey(key); + void AttachRecorder(Microsoft.UI.Xaml.Controls.ContentDialog dialog) + { + if (recorded.Add(dialog)) + dialog.GettingFocus += RecordFocus; + } + void Render(Element next) + { + host!.Content = current is null + ? host.Reconciler.Mount(next, static () => { }) + : host.Reconciler.Reconcile(current, next, (UIElement)host.Content!, static () => { }); + current = next; + } + async Task ConfirmAndAssertCancelAsync(string key) + { + opened = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + closed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + focusLog.Clear(); + await ui.RunOnUIAsync(() => Render(Dialog(key))); + await opened.Task.WaitAsync(DialogTransitionTimeout); + return await ui.RunOnUIAsync(async () => + { + var dialog = native!; + // Opened can precede the template and the first focus. Wait until Cancel holds focus, drain + // queued focus work, then require that Cancel was the first focus once it existed and kept it. + await TestSupport.WaitForRenderedConditionAsync(() => + DialogInitialFocus.FindCloseButton(dialog) is { } realized && dialog.XamlRoot is { } root && + ReferenceEquals(FocusManager.GetFocusedElement(root), realized), + $"{key}: Cancel receives initial focus in the realized dialog", + () => $"focused={FocusManager.GetFocusedElement(dialog.XamlRoot)?.GetType().Name ?? "none"}"); + await ui.YieldToRenderAsync(); + AssertCancelKeptFocus(dialog, key); + return dialog; + }); + } + async Task CloseAndRemoveAsync() + { + await ui.RunOnUIAsync(() => native!.Hide()); + await closed.Task.WaitAsync(DialogTransitionTimeout); + await ui.RunOnUIAsync(() => Render(Empty())); + } + try + { + await ui.RunOnUIAsync(() => + { + host = new ReactorHostControl { Width = 560, Height = 400 }; + ui.Container.Children.Add(host); + }); + var first = await ConfirmAndAssertCancelAsync("confirmation-0"); + // Once Cancel has focus the initial-focus duty is over: a later move (here to the body, which + // also runs layout for any pending retry) must not be taken back. + TextBox? body = null; + await ui.RunOnUIAsync(() => + { + body = FindDescendant(first); + Assert.NotNull(body); + Assert.True(body.Focus(FocusState.Programmatic)); + }); + await ui.YieldToRenderAsync(); + await ui.RunOnUIAsync(() => Assert.Same(body, FocusManager.GetFocusedElement(first.XamlRoot))); + + var previous = first; + for (var index = 1; index < Confirmations; index++) + { + if (index > 1) + await ui.RunOnUIAsync(() => AssertCancelKeptFocus(previous, $"confirmation-{index - 1} before close")); + await CloseAndRemoveAsync(); + var next = await ConfirmAndAssertCancelAsync($"confirmation-{index}"); + Assert.NotSame(previous, next); + previous = next; + } + // The last confirmation stays open through the test's own async steps; Cancel must still hold focus. + await ui.RunOnUIAsync(() => AssertCancelKeptFocus(previous, $"confirmation-{Confirmations - 1} before close")); + await CloseAndRemoveAsync(); + } + finally + { + await ui.RunOnUIAsync(() => + { + foreach (var dialog in recorded) + dialog.GettingFocus -= RecordFocus; + if (current is not null && native is { IsLoaded: true }) + native.Hide(); + if (host is null) return; + ui.Container.Children.Remove(host); + host.Content = null; + host.Dispose(); + }); + } + } + + private static bool IsWithin(DependencyObject ancestor, DependencyObject element) + { + for (DependencyObject? current = element; current is not null; current = Microsoft.UI.Xaml.Media.VisualTreeHelper.GetParent(current)) + if (ReferenceEquals(current, ancestor)) + return true; + return false; + } + + private static T? FindDescendant(DependencyObject parent) where T : DependencyObject + { + for (var index = 0; index < Microsoft.UI.Xaml.Media.VisualTreeHelper.GetChildrenCount(parent); index++) + { + var child = Microsoft.UI.Xaml.Media.VisualTreeHelper.GetChild(parent, index); + if (child is T match) return match; + if (FindDescendant(child) is { } nested) return nested; + } + return null; + } +} diff --git a/tests/OpenClaw.Tray.UITests/GatewayFixtureSystemsUiTests.cs b/tests/OpenClaw.Tray.UITests/GatewayFixtureSystemsUiTests.cs index 3763f5032d..ad330c6196 100644 --- a/tests/OpenClaw.Tray.UITests/GatewayFixtureSystemsUiTests.cs +++ b/tests/OpenClaw.Tray.UITests/GatewayFixtureSystemsUiTests.cs @@ -410,11 +410,25 @@ await WaitUiAsync(run, () => { var element = parent.FindFirst(TreeScope.Descendants, new PropertyCondition(AutomationElement.AutomationIdProperty, id)); - if (element is not null) - Assert.True(NativeUiOwnership.IsOwnedDescendant(window, element, run.AppProcessId), - $"Systems observation '{id}' must belong to the retained native window and process."); + if (element is null) + return null; + // Polls run while Systems re-renders (refresh, error and live readings). A peer that is + // detached or replaced is not observable yet: reject the whole observation so a found but + // unproven element can never satisfy an absence check. Foreign evidence still fails. + var evidence = NativeUiOwnership.Classify(window, element, run.AppProcessId); + Assert.True(evidence != NativeUiOwnership.Evidence.Foreign, + $"Systems observation '{id}' must belong to the retained native window and process."); + if (evidence != NativeUiOwnership.Evidence.Owned) + throw new ElementNotAvailableException($"Systems observation '{id}' ownership is not observable yet."); return element; } + bool OwnedNow(AutomationElement element, string what) + { + var evidence = NativeUiOwnership.Classify(window, element, run.AppProcessId); + Assert.True(evidence != NativeUiOwnership.Evidence.Foreign, + $"{what} must belong to the captured native window and process."); + return evidence == NativeUiOwnership.Evidence.Owned; + } AutomationElement? Find(string id) => FindUnder(window, id); var hwnd = new IntPtr(window.Current.NativeWindowHandle); var scale = SystemsWindowDpi(hwnd) / 96d; @@ -433,15 +447,30 @@ await WaitUiAsync(run, () => }, "normal Systems window"); Assert.NotNull(paneToggle); var records = new List(); - async Task Capture(string state, params AutomationElement[] requiredVisible) + // Finders run inside the poll: Systems re-renders live readings, so a control found earlier + // can be replaced before the capture asserts it. + async Task Capture(string state, params Func[] requiredVisible) { var name = $"{theme}-{state}.png"; OwnedWindowCapture.ViewportSnapshot? frame = null; await WaitUiAsync(run, () => - (frame = OwnedWindowCapture.TryReadViewport(window, run.AppProcessId, "SystemsPageScroll")) is not null, - $"owned Systems viewport ready for {state}"); + { + frame = OwnedWindowCapture.TryReadViewport(window, run.AppProcessId, "SystemsPageScroll"); + if (frame is null) + return false; + var required = new List { frame.Element }; + foreach (var find in requiredVisible) + { + // A replacement peer can exist before its first layout; Save asserts settled geometry. + if (find() is not { Current.IsOffscreen: false } element || + element.Current.BoundingRectangle is not { Width: > 0, Height: > 0 }) + return false; + required.Add(element); + } + OwnedWindowCapture.Save(run, window, name, [.. required]); + return true; + }, $"owned Systems viewport and required controls captured for {state}"); Assert.NotNull(frame); - OwnedWindowCapture.Save(run, window, name, [frame.Element, .. requiredVisible]); frame.AssertUnchanged(); var bounds = frame.WindowBounds; var viewport = frame.Bounds; @@ -495,7 +524,7 @@ await WaitUiAsync(run, () => }, $"collapsed {width} DIP Systems content remains below navigation chrome"); Assert.NotNull(pageMarker); - await Capture($"collapsed-{width}-top", pageMarker); + await Capture($"collapsed-{width}-top", () => Find("SystemsPageMarker")); if (width == 1000) { currentScroll.SetScrollPercent(ScrollPattern.NoScroll, 35); @@ -518,10 +547,11 @@ await WaitUiAsync(run, () => previousCollapsedWidth = -1d; return false; } - Assert.True(NativeUiOwnership.IsOwnedDescendant(window, frame.Element, run.AppProcessId), - "Systems scroll viewport must belong to the captured native window and process."); - Assert.True(NativeUiOwnership.IsOwnedDescendant(window, marker, run.AppProcessId), - "Systems page marker must belong to the captured native window and process."); + if (!OwnedNow(frame.Element, "Systems scroll viewport") || !OwnedNow(marker, "Systems page marker")) + { + previousCollapsedWidth = -1d; + return false; + } var viewport = frame.Bounds; var pattern = (ScrollPattern)frame.Element.GetCurrentPattern(ScrollPattern.Pattern); if (viewport.IsEmpty || viewport.Width <= 0 || viewport.Height <= 0 || @@ -568,7 +598,7 @@ await WaitUiAsync(run, () => "expanding navigation removes the Systems scroll-only inset"); Assert.NotNull(currentViewport); Assert.NotNull(pageMarker); - await Capture("expanded-1000-top", pageMarker); + await Capture("expanded-1000-top", () => Find("SystemsPageMarker")); currentViewport.AssertUnchanged(); var chromeWidth = currentViewport.WindowBounds.Width - currentViewport.Bounds.Width; var statsWindowWidth = 1000 * scale + chromeWidth; @@ -711,8 +741,8 @@ await WaitUiAsync(run, () => memory.Current.BoundingRectangle.Width / scale >= 160 && disk.Current.BoundingRectangle.Width / scale >= 160)) return false; - Assert.True(NativeUiOwnership.IsOwnedDescendant(window, gatewayRow, run.AppProcessId)); - Assert.True(NativeUiOwnership.IsOwnedDescendant(window, studioRow, run.AppProcessId)); + if (!OwnedNow(gatewayRow, "Gateway machine row") || !OwnedNow(studioRow, "Studio machine row")) + return false; var gatewayPattern = (ExpandCollapsePattern)gatewayRow.GetCurrentPattern(ExpandCollapsePattern.Pattern); var studioPattern = (ExpandCollapsePattern)studioRow.GetCurrentPattern(ExpandCollapsePattern.Pattern); cpuTile = cpu; @@ -746,7 +776,8 @@ await File.WriteAllTextAsync(Path.Combine(run.ArtifactsDirectory, "tile-visibili Assert.NotNull(cpuTile); Assert.NotNull(memoryTile); Assert.NotNull(diskTile); - await Capture("expanded-1000-host-stat-tiles", cpuTile, memoryTile, diskTile); + await Capture("expanded-1000-host-stat-tiles", + () => Studio("SystemsStat_cpu"), () => Studio("SystemsStat_memory"), () => Studio("SystemsStat_disk")); gatewayDisclosure.Expand(); studioDisclosure.Collapse(); studioDisclosure.Expand(); @@ -768,7 +799,7 @@ await WaitUiAsync(run, () => !gatewayRow.Current.IsOffscreen, "both expanded machine rows occupy the viewport"); Assert.NotNull(selectedName); - await Capture("two-open-rows", selectedName); + await Capture("two-open-rows", () => Find("SystemsSelectedName")); statsPattern.SetScrollPercent(ScrollPattern.NoScroll, 0); AutomationElement? issuesButton = null; await WaitUiAsync(run, () => @@ -813,7 +844,7 @@ await WaitUiAsync(run, () => Assert.NotNull(retryButton); Assert.NotNull(inventoryConnection); Assert.Contains(run.Gateway.Requests, request => request.Method == "environments.list" && request.Outcome == "error:UNAVAILABLE"); - await Capture("inventory-error-recovery", retryButton, inventoryConnection); + await Capture("inventory-error-recovery", () => Find("SystemsRetry"), () => Find("SystemsInventoryConnection")); run.Gateway.SetSystemsInventoryUnavailable(false); Invoke(retryButton); await WaitUiAsync(run, () => Find("SystemsRetry") is null && @@ -847,7 +878,6 @@ await WaitUiAsync(run, () => var inventoryIds = new[] { "SystemsRow_gateway", "SystemsRow_node:fixture-studio", "SystemsRow_node:fixture-macbook", "SystemsRow_node:fixture-pi" }; AutomationElement? disconnectedConnection = null; - AutomationElement[] disconnectedRows = []; await WaitUiAsync(run, () => { if (Find("SystemsDisconnectedConnection") is not { Current.IsOffscreen: false } connection) @@ -864,7 +894,6 @@ await WaitUiAsync(run, () => rows.Add(row); } var gatewayPattern = (ExpandCollapsePattern)rows[0].GetCurrentPattern(ExpandCollapsePattern.Pattern); - disconnectedRows = rows.ToArray(); disconnectedConnection = connection; disconnectedGatewayDisclosure = gatewayPattern; return true; @@ -872,7 +901,8 @@ await WaitUiAsync(run, () => "settled disconnected inventory retains all four collapsed last-known machines"); Assert.NotNull(disconnectedConnection); await Capture("disconnected-last-known", - disconnectedRows.Prepend(disconnectedConnection).ToArray()); + [() => Find("SystemsDisconnectedConnection"), + .. inventoryIds.Select(id => (Func)(() => Find(id)))]); disconnectedGatewayDisclosure.Expand(); AutomationElement? knownGateway = null; OwnedWindowCapture.ViewportSnapshot? knownScrollFrame = null; @@ -880,10 +910,8 @@ await Capture("disconnected-last-known", await WaitUiAsync(run, () => { var row = Find("SystemsRow_gateway"); - if (row is null) + if (row is null || !OwnedNow(row, "Last-known Gateway row")) return false; - Assert.True(NativeUiOwnership.IsOwnedDescendant(window, row, run.AppProcessId), - "Last-known Gateway row must belong to the captured native window and process."); if (((ExpandCollapsePattern)row.GetCurrentPattern(ExpandCollapsePattern.Pattern)) .Current.ExpandCollapseState != ExpandCollapseState.Expanded) return false; @@ -929,7 +957,9 @@ await WaitUiAsync(run, () => Assert.NotNull(knownStatistics); Assert.NotNull(knownSample); Assert.NotNull(knownDisk); - await Capture("disconnected-expanded-resources", knownCapabilities, knownStatistics, knownSample, knownDisk); + await Capture("disconnected-expanded-resources", + () => GatewayContent("SystemsCapabilitiesHeading"), () => GatewayContent("SystemsStatisticsHeading"), + () => GatewayContent("SystemsStatsSampled"), () => GatewayContent("SystemsStat_disk:/data")); await run.WriteReportAsync("passed"); output.WriteLine($"{theme} daily Systems frames: {run.ArtifactsDirectory}"); } diff --git a/tests/OpenClaw.Tray.UITests/NativeUiOwnership.cs b/tests/OpenClaw.Tray.UITests/NativeUiOwnership.cs index 48ab4ebecd..ddbd708cda 100644 --- a/tests/OpenClaw.Tray.UITests/NativeUiOwnership.cs +++ b/tests/OpenClaw.Tray.UITests/NativeUiOwnership.cs @@ -1,5 +1,6 @@ // Verifies a UIA control against an owned native window, including windowless XAML fragments. // Use IsOwnedDescendant(ownedWindow, control, appProcessId) before claiming control ownership. +// Inside a polling wait, use Classify: keep polling on Unproven, fail immediately on Foreign. using System.Runtime.InteropServices; using System.Windows.Automation; @@ -10,37 +11,112 @@ internal static class NativeUiOwnership { private const int MaxAncestorDepth = 64; + /// + /// Owned: the raw ancestry reaches the retained window inside the owning process. + /// Unproven: no contrary evidence, but ownership cannot be shown yet. The peer is torn down or + /// not yet attached (null parent, unavailable element) or reports no process id. + /// Foreign: positive contrary evidence, such as a different process, a foreign native window, an + /// ancestry that reaches another window or the desktop, or an invalid owner window. + /// + internal enum Evidence { Owned, Unproven, Foreign } + /// Rejects foreign processes, other windows, and ancestry without a verified native owner. - internal static bool IsOwnedDescendant(AutomationElement window, AutomationElement element, int processId) + internal static bool IsOwnedDescendant(AutomationElement window, AutomationElement element, int processId) => + Classify(window, element, processId) == Evidence.Owned; + + /// + /// Classifies ownership. Inside a polling wait, treat as "not yet" and + /// as an immediate failure. + /// + internal static Evidence Classify(AutomationElement window, AutomationElement element, int processId) => + Classify(window, element, processId, ReadFacts, TreeWalker.RawViewWalker.GetParent, NativeProcessId); + + /// + /// UIA facts the ancestry walk uses; separated so the decision rules are testable without a desktop. + /// A null marks a partial read: the peer disappeared after the facts before it. + /// + internal readonly record struct NodeFacts(int ProcessId, string? FrameworkId, int NativeWindowHandle, bool IsWindow, int[]? RuntimeId); + + /// + /// The ownership walk over any node source. Positive contrary evidence (Foreign) is checked before + /// missing evidence (Unproven) at every node, so an unknown process id never hides a foreign window. + /// + internal static Evidence Classify(T window, T element, int processId, Func read, + Func parentOf, Func windowProcessId) where T : class { - var owner = window.Current; - if (processId <= 0 || owner.ProcessId != processId || owner.ControlType != ControlType.Window || - owner.NativeWindowHandle == 0 || NativeProcessId(owner.NativeWindowHandle) != (uint)processId) - return false; + try + { + var owner = read(window); + if (processId <= 0 || owner.ProcessId != processId || + owner.NativeWindowHandle != 0 && windowProcessId(owner.NativeWindowHandle) != (uint)processId) + return Evidence.Foreign; + if (owner.RuntimeId is null) return Evidence.Unproven; + if (!owner.IsWindow || owner.NativeWindowHandle == 0) return Evidence.Foreign; + if (owner.RuntimeId.Length == 0) return Evidence.Unproven; + + T? current = element; + var unproven = false; + for (var depth = 0; current is not null && depth < MaxAncestorDepth; depth++) + { + var info = read(current); + if (info.NativeWindowHandle != 0) + { + // Zero means the HWND is already destroyed (a closing popup): missing, not contrary, evidence. + var windowProcess = windowProcessId(info.NativeWindowHandle); + if (windowProcess == 0) unproven = true; + else if (windowProcess != (uint)processId) return Evidence.Foreign; + } + if (info.ProcessId != processId && info.ProcessId != 0) + return Evidence.Foreign; + // XAML's windowless provider can return an explicit zero PID. Other frameworks give no proof, + // but keep walking so contrary evidence higher up still wins. + if (info.RuntimeId is null) return Evidence.Unproven; + if (info.ProcessId == 0 && !string.Equals(info.FrameworkId, "XAML", StringComparison.Ordinal)) + unproven = true; - var ownerRuntimeId = window.GetRuntimeId(); - if (ownerRuntimeId.Length == 0) return false; + if (info.RuntimeId.SequenceEqual(owner.RuntimeId)) + { + // The owner changed identity during the walk: contrary evidence, not missing evidence. + var again = read(window); + if (again.ProcessId != processId || + again.NativeWindowHandle != 0 && again.NativeWindowHandle != owner.NativeWindowHandle) + return Evidence.Foreign; + if (again.RuntimeId is null) return Evidence.Unproven; + if (again.NativeWindowHandle != owner.NativeWindowHandle || + !again.RuntimeId.SequenceEqual(owner.RuntimeId) || + windowProcessId(owner.NativeWindowHandle) != (uint)processId) + return Evidence.Foreign; + return unproven ? Evidence.Unproven : Evidence.Owned; + } - AutomationElement? current = element; - for (var depth = 0; current is not null && depth < MaxAncestorDepth; depth++) + // Another window's ancestry ends at the desktop root, whose process is not the app's. + current = parentOf(current); + } + // Detached (null parent) or unreasonably deep: no proof either way. + return Evidence.Unproven; + } + catch (ElementNotAvailableException) + { + // A peer torn down mid-walk proves nothing about ownership. + return Evidence.Unproven; + } + } + + // Each property is a separate live read. Read ownership facts first and keep them if the peer then + // disappears, so evidence already read is never discarded. + private static NodeFacts ReadFacts(AutomationElement element) + { + var current = element.Current; + var processId = current.ProcessId; + int handle; + try { handle = current.NativeWindowHandle; } + catch (ElementNotAvailableException) { return new(processId, null, 0, false, null); } + try { - var info = current.Current; - // XAML's windowless provider can return an explicit zero PID. It is not ownership evidence. - if (info.ProcessId != processId && - (info.ProcessId != 0 || !string.Equals(info.FrameworkId, "XAML", StringComparison.Ordinal))) - return false; - if (info.NativeWindowHandle != 0 && NativeProcessId(info.NativeWindowHandle) != (uint)processId) - return false; - - if (current.GetRuntimeId().SequenceEqual(ownerRuntimeId)) - return window.Current.NativeWindowHandle == owner.NativeWindowHandle && - window.Current.ProcessId == processId && - window.GetRuntimeId().SequenceEqual(ownerRuntimeId) && - NativeProcessId(owner.NativeWindowHandle) == (uint)processId; - - current = TreeWalker.RawViewWalker.GetParent(current); + return new(processId, current.FrameworkId, handle, + current.ControlType == ControlType.Window, element.GetRuntimeId()); } - return false; + catch (ElementNotAvailableException) { return new(processId, null, handle, false, null); } } private static uint NativeProcessId(int hwnd) diff --git a/tests/OpenClaw.Tray.UITests/NativeUiOwnershipTests.cs b/tests/OpenClaw.Tray.UITests/NativeUiOwnershipTests.cs index 693b82e0bb..1611944f29 100644 --- a/tests/OpenClaw.Tray.UITests/NativeUiOwnershipTests.cs +++ b/tests/OpenClaw.Tray.UITests/NativeUiOwnershipTests.cs @@ -44,6 +44,12 @@ public async Task OwnedXaml_RejectsAnotherWindowAndWrongOrUnverifiableOwners() Assert.False(NativeUiOwnership.IsOwnedDescendant(owner, ownedControl, Environment.ProcessId + 1)); Assert.False(NativeUiOwnership.IsOwnedDescendant(owner, AutomationElement.RootElement, Environment.ProcessId)); Assert.False(NativeUiOwnership.IsOwnedDescendant(ownedControl, ownedControl, Environment.ProcessId)); + // Every rejection above carries positive contrary evidence; polls must fail on these, not wait. + Assert.Equal(NativeUiOwnership.Evidence.Foreign, NativeUiOwnership.Classify(owner, otherControl, Environment.ProcessId)); + Assert.Equal(NativeUiOwnership.Evidence.Foreign, NativeUiOwnership.Classify(other, ownedControl, Environment.ProcessId)); + Assert.Equal(NativeUiOwnership.Evidence.Foreign, NativeUiOwnership.Classify(owner, ownedControl, Environment.ProcessId + 1)); + Assert.Equal(NativeUiOwnership.Evidence.Foreign, NativeUiOwnership.Classify(owner, AutomationElement.RootElement, Environment.ProcessId)); + Assert.Equal(NativeUiOwnership.Evidence.Foreign, NativeUiOwnership.Classify(ownedControl, ownedControl, Environment.ProcessId)); } finally { @@ -205,3 +211,111 @@ private static Window CreateWindow() return window; } } + +/// +/// Deterministic Owned/Unproven/Foreign rules over synthetic ancestry. SystemsDaily failed on CI when a +/// re-rendered peer was unavailable mid-walk; polls must keep observing on Unproven, but any positive +/// evidence of another process or window must stay Foreign so it fails immediately. +/// +public sealed class NativeUiOwnershipClassificationTests +{ + private const int AppPid = 4242; + private const int OtherPid = 777; + private const int OwnerHwnd = 0x100; + private const int OtherAppWindowHwnd = 0x200; + private const int ForeignHwnd = 0x300; + private const int DesktopHwnd = 0x10010; + private const int DestroyedHwnd = 0x400; + + private sealed class Node(int processId, string framework = "XAML", int hwnd = 0, bool isWindow = false, Node? parent = null) + { + private static int nextId = 1; + public int ProcessId = processId; + public int Hwnd = hwnd; + public bool Unavailable; + public bool PartialRead; + public Node? Parent = parent; + public readonly int[] RuntimeId = [42, Interlocked.Increment(ref nextId)]; + public NativeUiOwnership.NodeFacts Facts => Unavailable + ? throw new ElementNotAvailableException() + : PartialRead ? new(ProcessId, null, Hwnd, false, null) + : new(ProcessId, framework, Hwnd, isWindow, RuntimeId); + } + + private static uint WindowProcess(int hwnd) => hwnd switch + { + OwnerHwnd or OtherAppWindowHwnd => AppPid, + ForeignHwnd or DesktopHwnd => OtherPid, + _ => 0, + }; + + private static NativeUiOwnership.Evidence Classify(Node window, Node element) => + NativeUiOwnership.Classify(window, element, AppPid, node => node.Facts, node => node.Parent, WindowProcess); + + private static Node Owner() => new(AppPid, "Win32", OwnerHwnd, isWindow: true); + + [Fact] + public void WindowlessXamlReachingTheOwner_IsOwned() + { + var owner = Owner(); + var control = new Node(0, parent: new Node(AppPid, parent: owner)); + Assert.Equal(NativeUiOwnership.Evidence.Owned, Classify(owner, control)); + } + + [Fact] + public void MissingEvidence_IsUnproven() + { + var owner = Owner(); + var pane = new Node(AppPid, parent: owner); + Assert.Equal(NativeUiOwnership.Evidence.Unproven, Classify(owner, new Node(AppPid))); + Assert.Equal(NativeUiOwnership.Evidence.Unproven, Classify(owner, new Node(0, "Win32", parent: pane))); + Assert.Equal(NativeUiOwnership.Evidence.Unproven, Classify(owner, new Node(AppPid, hwnd: DestroyedHwnd, parent: pane))); + var torn = new Node(AppPid, parent: pane) { Unavailable = true }; + Assert.Equal(NativeUiOwnership.Evidence.Unproven, Classify(owner, new Node(AppPid, parent: torn))); + Assert.Equal(NativeUiOwnership.Evidence.Unproven, Classify(owner, torn)); + // The peer vanished after its process id was read: no contrary evidence, so not proven yet. + Assert.Equal(NativeUiOwnership.Evidence.Unproven, Classify(owner, new Node(AppPid, parent: pane) { PartialRead = true })); + } + + [Fact] + public void PositiveContraryEvidence_IsForeign() + { + var owner = Owner(); + var pane = new Node(AppPid, parent: owner); + var desktop = new Node(OtherPid, "Win32", DesktopHwnd); + Assert.Equal(NativeUiOwnership.Evidence.Foreign, Classify(owner, new Node(OtherPid, parent: pane))); + Assert.Equal(NativeUiOwnership.Evidence.Foreign, Classify(owner, + new Node(AppPid, parent: new Node(AppPid, "Win32", OtherAppWindowHwnd, isWindow: true, parent: desktop)))); + // Missing evidence never hides contrary evidence, at the same node or higher up. + Assert.Equal(NativeUiOwnership.Evidence.Foreign, Classify(owner, new Node(0, "Win32", ForeignHwnd, parent: pane))); + Assert.Equal(NativeUiOwnership.Evidence.Foreign, Classify(owner, new Node(0, "Win32", parent: new Node(OtherPid)))); + // Ownership facts read before the peer vanished are kept, so a foreign process still wins. + Assert.Equal(NativeUiOwnership.Evidence.Foreign, Classify(owner, new Node(OtherPid, parent: pane) { PartialRead = true })); + Assert.Equal(NativeUiOwnership.Evidence.Foreign, Classify(owner, new Node(AppPid, hwnd: ForeignHwnd, parent: pane) { PartialRead = true })); + Assert.Equal(NativeUiOwnership.Evidence.Foreign, Classify(new Node(AppPid, "Win32", OwnerHwnd), pane)); + Assert.Equal(NativeUiOwnership.Evidence.Foreign, Classify(new Node(OtherPid, "Win32", ForeignHwnd, isWindow: true), pane)); + } + [Fact] + public void OwnerIdentityChangingDuringTheWalk_IsForeign() + { + Assert.Equal(NativeUiOwnership.Evidence.Foreign, ClassifyWithOwnerChangedOnRecheck(partialRecheck: false)); + // A partial recheck still keeps the changed window handle it read before the owner vanished. + Assert.Equal(NativeUiOwnership.Evidence.Foreign, ClassifyWithOwnerChangedOnRecheck(partialRecheck: true)); + } + + private static NativeUiOwnership.Evidence ClassifyWithOwnerChangedOnRecheck(bool partialRecheck) + { + var owner = Owner(); + var control = new Node(AppPid, parent: new Node(AppPid, parent: owner)); + var reads = 0; + return NativeUiOwnership.Classify(owner, control, AppPid, node => + { + if (ReferenceEquals(node, owner) && ++reads == 3) + { + owner.Hwnd = OtherAppWindowHwnd; + owner.PartialRead = partialRecheck; + } + return node.Facts; + }, node => node.Parent, WindowProcess); + } +} \ No newline at end of file diff --git a/tests/OpenClaw.Tray.UITests/OwnedWindowCapture.cs b/tests/OpenClaw.Tray.UITests/OwnedWindowCapture.cs index c897257448..3a29e0a4ea 100644 --- a/tests/OpenClaw.Tray.UITests/OwnedWindowCapture.cs +++ b/tests/OpenClaw.Tray.UITests/OwnedWindowCapture.cs @@ -109,8 +109,11 @@ internal void AssertUnchanged() new PropertyCondition(AutomationElement.AutomationIdProperty, automationId)); if (element is null) return null; - Assert.True(NativeUiOwnership.IsOwnedDescendant(window, element, processId), + var evidence = NativeUiOwnership.Classify(window, element, processId); + Assert.True(evidence != NativeUiOwnership.Evidence.Foreign, "Viewport must belong to the capture window, not another window in the process."); + if (evidence != NativeUiOwnership.Evidence.Owned) + return null; var windowBounds = window.Current.BoundingRectangle; var bounds = element.Current.BoundingRectangle; static bool Usable(global::System.Windows.Rect area) => @@ -163,8 +166,13 @@ private static void SaveCore(int appProcessId, string runId, string artifactsDir qualifiedGeometry is null ? null : ReadCurrentProcessGeometry(handle)); var assertions = assertedElements.Select(element => { - Assert.True(NativeUiOwnership.IsOwnedDescendant(window, element, appProcessId), + var evidence = NativeUiOwnership.Classify(window, element, appProcessId); + Assert.True(evidence != NativeUiOwnership.Evidence.Foreign, "Asserted control must belong to the captured native window and process."); + // A re-rendered peer proves nothing yet. Polling callers observe again; others fail as unavailable. + if (evidence != NativeUiOwnership.Evidence.Owned) + throw new ElementNotAvailableException( + "Asserted control ownership is not observable yet; the control was detached or re-rendered."); Assert.False(element.Current.IsOffscreen); var area = element.Current.BoundingRectangle; Assert.True(area.Width > 0 && area.Height > 0);