diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d22982..aefda57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,10 @@ This project follows [Semantic Versioning 2.0.0](https://semver.org/spec/v2.0.0. - **`dusk:doctor` check 3 (snapshot enrichers) now emits INFO when no enrichers are registered, instead of WARN.** Enrichers are opt-in; zero is a valid state, not a problem. The WARN reading alongside "integration wired" (check 5) created false contradiction. Touches `lib/src/commands/dusk_doctor_command.dart`; test case updated in `test/src/commands/dusk_doctor_command_test.dart`. +- **`q` (find / observe) taps now dispatch at the target's own rect instead of the viewport centre.** `_entryFromSemanticsNode` anchored the `RefEntry` at the root element, so `dispatchRectOf` returned `_liveRectOf(root)` (the whole viewport) and every find/observe gesture fired at screen centre. A centred target coincidentally worked; off-centre controls (a submit button, a checkbox, a sidebar item) were missed silently. The entry now resolves the element whose `RenderBox` contributes the node (via `debugSemantics` identity, matching `ext_observe`), so the gesture lands on the addressed widget. Touches `lib/src/extensions/ext_find.dart`; covered by `test/src/extensions/ext_find_test.dart`. + +- **`find`-by-label now prefers the first INTERACTIVE match when a label collides with inert text.** A visible `Text` naming an adjacent control (a settings label beside a switch that shares its `semanticLabel`) or a heading repeating a button's text (a "Sign In" heading over the submit button) sits first in tree order, so the handle resolved to the inert node and the tap landed on the label. `find` now resolves to the first node exposing `SemanticsAction.tap` (button / switch / text field) when the label spans an interactive and a non-interactive node, falling back to the first match otherwise. `matchCount` / `diagnostic` still report the collision. Touches `lib/src/extensions/ext_find.dart`; covered by `test/src/extensions/ext_find_test.dart`. + - **`dusk:type` now targets the editable inside the ref's own Semantics rect, not the first `EditableText` in the tree.** The handler resolved the field to type into by walking to the first editable under the isolate, so on a form with several inputs a `type` against `q3` (Password) could land in the first field (Email). It now maps the ref's `SemanticsNode` to its global rect (`localToGlobal`) and selects the editable whose render box OVERLAPS that rect by the largest area (falling back to the nearest-center editable when none overlaps), so the value goes into the addressed field. The same rect-based selection also backs `dusk:clear`. Touches `lib/src/extensions/ext_text_input.dart`; covered by `test/src/extensions/ext_text_input_test.dart`. --- diff --git a/doc/commands/dusk-find.md b/doc/commands/dusk-find.md index 4adf359..ada54cf 100644 --- a/doc/commands/dusk-find.md +++ b/doc/commands/dusk-find.md @@ -228,6 +228,22 @@ label 'Password' matched 2 nodes; refine with --key, --text, or --contains list; each candidate includes role, bounds, and enricher fields that let the agent identify the correct target before minting the handle. +**Interactive nodes win a mixed collision.** When a label spans one interactive +node (a button, switch, or text field exposing `SemanticsAction.tap`) and one or +more inert nodes (a `Text` heading or a settings label that repeats the control's +copy), the handle resolves to the first interactive node rather than the first +node in tree order. A visible label naming an adjacent control, or a heading +repeating a button's text, otherwise sits first in tree order and the tap would +land on the inert label. When every match is interactive or every match is inert, +the handle falls back to the first node in tree order. `matchCount` and +`diagnostic` still report the full collision either way, so an agent that needs a +specific one of several interactive matches should still refine the predicate. + +Each `qN` gesture then dispatches at the resolved node's own on-screen rect (the +`RenderBox` bounds of the widget that contributes the node), so off-centre +controls (a sidebar item, a submit button below the fold) receive the tap at +their real position rather than the viewport centre. + --- diff --git a/lib/src/extensions/ext_find.dart b/lib/src/extensions/ext_find.dart index 4f5e55f..5e4b323 100644 --- a/lib/src/extensions/ext_find.dart +++ b/lib/src/extensions/ext_find.dart @@ -340,13 +340,15 @@ SemanticsNode? _findSemanticsNodeByLabelContains(String needle) { /// count reflects ALL matches in the tree. When `count > 1` the caller /// should surface an ambiguity diagnostic to the agent. (SemanticsNode?, int) _findSemanticsNodeByLabelWithCount(String needle) { - SemanticsNode? found; + SemanticsNode? firstMatch; + SemanticsNode? firstInteractive; int count = 0; void visit(SemanticsNode node) { if (node.label == needle) { count += 1; - found ??= node; + firstMatch ??= node; + firstInteractive ??= _isInteractiveNode(node) ? node : null; } node.visitChildren((SemanticsNode child) { visit(child); @@ -362,7 +364,21 @@ SemanticsNode? _findSemanticsNodeByLabelContains(String needle) { } visitOwner(RendererBinding.instance.rootPipelineOwner); - return (found, count); + // Prefer an interactive match (a button, switch, or text field) over a plain + // node when one label collides across both: a visible label WText that names + // an adjacent control, or a heading that repeats a button's text, would + // otherwise resolve to the non-interactive copy and land the tap on the label + // instead of the control. Falls back to the first match when none is + // interactive. + return (firstInteractive ?? firstMatch, count); +} + +/// Whether [node] is something an agent would act on (button / switch / text +/// field / anything exposing a tap action) rather than inert content. +bool _isInteractiveNode(SemanticsNode node) { + final flags = node.flagsCollection; + if (flags.isButton || flags.isTextField) return true; + return node.getSemanticsData().hasAction(SemanticsAction.tap); } /// Cross-checks an Element-tree match against the supplied query's @@ -413,24 +429,72 @@ RefEntry? _entryFromElement(Element element) { ); } -/// Materialises a [RefEntry] from a [SemanticsNode]. The element field is -/// set to the root element (best-effort anchor for EditableText focus -/// lookups); the rect is mapped from the node's local space up through -/// each ancestor's transform to obtain global coordinates that -/// pointer-event dispatch can hit. +/// Materialises a [RefEntry] from a [SemanticsNode]. +/// +/// The entry's [RefEntry.element] is the live [Element] whose render object +/// contributes [node] to the semantics tree, NOT the root element. This is +/// load-bearing: pointer dispatch re-resolves the tap point from +/// `dispatchRectOf(entry) = _liveRectOf(entry.element)`, and EditableText focus +/// walks descendants of `entry.element`. Anchoring the entry at the root made +/// both resolve against the whole viewport, so every q-ref tap dispatched at +/// the SCREEN center (missing the target) and text-input landed on the first +/// editable in the tree. Resolving the owning element fixes both at the source. +/// +/// The rect is taken from that element's [RenderBox] via `localToGlobal` +/// (LOGICAL px, the space dusk's pointer events use). It falls back to the +/// root element and the semantics-transform rect only when no render object +/// owns the node (the DPR-divided `_globalRectFromSemantics`, kept logical). RefEntry? _entryFromSemanticsNode(SemanticsNode node) { final Element? root = WidgetsBinding.instance.rootElement; if (root == null) return null; final bool isTextField = node.flagsCollection.isTextField; + final Element? target = _elementForSemanticsNode(node); + final RenderObject? renderObject = target?.findRenderObject(); + // Promote to a RenderBox local so the rect calculation and the RefEntry field + // share one non-null handle; a separate `sized` boolean would not promote + // `renderObject` for the localToGlobal/size reads below. + final RenderBox? box = + renderObject is RenderBox && renderObject.attached && renderObject.hasSize + ? renderObject + : null; + final Rect resolvedRect = box != null + ? box.localToGlobal(Offset.zero) & box.size + : globalRectFromSemantics(node); return RefEntry( - rect: _globalRectFromSemantics(node), - element: root, + rect: resolvedRect, + element: target ?? root, groupId: _kQueryGroupId, isTextField: isTextField, + renderObject: box, node: node, ); } +/// The live [Element] whose [RenderBox] contributes [node] to the semantics +/// tree, matching on `debugSemantics` identity (debug-only, and dusk is +/// `kDebugMode`-gated). Returns `null` when no render object owns [node] +/// (a purely synthetic or merged-away node); callers fall back to the root. +Element? _elementForSemanticsNode(SemanticsNode node) { + final Element? root = WidgetsBinding.instance.rootElement; + if (root == null) return null; + Element? result; + void visit(Element element) { + if (result != null) return; + final RenderObject? renderObject = element.renderObject; + if (renderObject is RenderBox && + renderObject.attached && + renderObject.hasSize && + identical(renderObject.debugSemantics, node)) { + result = element; + return; + } + element.visitChildElements(visit); + } + + root.visitChildElements(visit); + return result; +} + /// Walks up the [SemanticsNode] ancestor chain, applying each ancestor's /// transform to map [node]'s local rect into global coordinates. /// @@ -439,7 +503,13 @@ RefEntry? _entryFromSemanticsNode(SemanticsNode node) { /// parent. Composing transforms from the leaf upward yields the global /// rect; pointer dispatch consumes `rect.center` and must be in the same /// space as the view's logical viewport. -Rect _globalRectFromSemantics(SemanticsNode node) { +/// +/// Public and `@visibleForTesting`: this fallback only runs for a synthetic or +/// non-RenderBox-owned node (see [_elementForSemanticsNode]), which cannot be +/// produced through [extDuskFindHandler] with a real widget tree, so the +/// transform-composition + DPR-correction math is verified directly. +@visibleForTesting +Rect globalRectFromSemantics(SemanticsNode node) { Rect rect = node.rect; SemanticsNode? current = node; while (current != null) { @@ -449,6 +519,22 @@ Rect _globalRectFromSemantics(SemanticsNode node) { } current = current.parent; } + // The semantics ancestor transforms fold in the root view's device pixel + // ratio, so `rect` is in PHYSICAL pixels. dusk dispatches pointer events and + // compares against render-tree localToGlobal rects in LOGICAL pixels, so + // divide the DPR back out; otherwise, on a DPR!=1 display, every tap that + // falls back to this rect lands at N times the intended offset and misses. + final double dpr = WidgetsBinding + .instance.platformDispatcher.implicitView?.devicePixelRatio ?? + 1.0; + if (dpr > 0 && dpr != 1.0) { + rect = Rect.fromLTRB( + rect.left / dpr, + rect.top / dpr, + rect.right / dpr, + rect.bottom / dpr, + ); + } return rect; } diff --git a/test/src/extensions/ext_find_test.dart b/test/src/extensions/ext_find_test.dart index f1e7453..53f9ca7 100644 --- a/test/src/extensions/ext_find_test.dart +++ b/test/src/extensions/ext_find_test.dart @@ -1,11 +1,13 @@ import 'dart:convert'; import 'package:flutter/material.dart'; +import 'package:flutter/semantics.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:fluttersdk_dusk/src/extensions/ext_find.dart'; import 'package:fluttersdk_dusk/src/extensions/ext_pointer.dart'; import 'package:fluttersdk_dusk/src/ref_registry.dart'; +import 'package:fluttersdk_dusk/src/utils/actionability_gate.dart'; import 'package:fluttersdk_dusk/src/utils/dusk_exceptions.dart'; import 'package:fluttersdk_dusk/src/utils/error_envelope.dart'; @@ -336,6 +338,298 @@ void main() { }, ); + testWidgets( + '(c) q-ref tap dispatches at the target, not the viewport center ' + '(off-center target)', + (WidgetTester tester) async { + tester.view.physicalSize = const Size(800, 600); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.resetPhysicalSize); + addTearDown(tester.view.resetDevicePixelRatio); + + // A 120x60 target pinned to the top-left corner: its center (60, 30) + // is far from the 800x600 viewport center (400, 300). A q-ref that + // anchored its RefEntry at the ROOT element resolved its live dispatch + // rect (`dispatchRectOf` = `_liveRectOf(entry.element)`) to the whole + // viewport, so the tap fired at (400, 300) and missed the target. This + // is the exact defect that made real buttons (Sign In, checkbox) ignore + // q-ref taps while a centered target coincidentally still worked. + int taps = 0; + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Align( + alignment: Alignment.topLeft, + child: GestureDetector( + onTap: () => taps += 1, + child: Semantics( + label: 'corner-target', + button: true, + container: true, + child: Container( + width: 120, + height: 60, + color: const Color(0xFF000000), + ), + ), + ), + ), + ), + ), + ); + await tester.pump(); + + final findResponse = await extDuskFindHandler( + 'ext.dusk.find', + {'semanticsLabel': 'corner-target'}, + ); + final String qRef = (jsonDecode(findResponse.result!) + as Map)['ref'] as String; + expect(qRef, startsWith('q')); + + // The entry anchors on the owning element, so the live dispatch rect + // is the target's top-left box, NOT the whole viewport. + final RefEntry? entry = resolveRefForAction(qRef); + expect(entry, isNotNull); + final Rect dispatchRect = dispatchRectOf(entry!)!; + expect(dispatchRect.center.dx, lessThan(200)); + expect(dispatchRect.center.dy, lessThan(120)); + + final future = aiTestTapHandler( + 'ext.dusk.tap', + { + 'ref': qRef, + 'checkStable': 'false', + 'checkReceivesEvents': 'false', + }, + ); + await tester.pump(const Duration(milliseconds: 100)); + await tester.pump(); + await tester.pump(); + final response = await future; + + expect(response.errorDetail, isNull); + expect(taps, equals(1)); + }, + ); + + testWidgets( + '(c) q-ref tap fires onTap for an excludeSemantics button ' + '(WAnchor semanticLabel / WSwitch pattern)', + (WidgetTester tester) async { + // DPR=2 mirrors the live web run (retina); logical stays 800x600. + tester.view.physicalSize = const Size(1600, 1200); + tester.view.devicePixelRatio = 2.0; + addTearDown(tester.view.resetPhysicalSize); + addTearDown(tester.view.resetDevicePixelRatio); + + // Mirrors WAnchor's `semanticLabel != null` branch (used by WSwitch and + // icon-only buttons): a Semantics(button, onTap, excludeSemantics: true) + // wrapping the real gesture. The exclusion drops the descendant + // GestureDetector's node from the semantics tree, so the ONLY semantics + // node is this annotation node. A q-ref must still resolve to the + // owning element (not fall back to the root) and its pointer tap must + // reach the underlying GestureDetector. This reproduces the live bug + // where MSSwitch toggles (notification prefs, status-page monitor + // assignment) ignored dusk taps. + int taps = 0; + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Align( + alignment: Alignment.topLeft, + child: Semantics( + // Outer boundary the real WSwitch wraps around WAnchor: + // Semantics(container, toggled) -> MergeSemantics -> WAnchor's + // Semantics(button, onTap, excludeSemantics: true). This is the + // full live structure the earlier bare reproduction omitted. + container: true, + toggled: false, + child: MergeSemantics( + child: Semantics( + button: true, + label: 'toggle-x', + onTap: () => taps += 1, + excludeSemantics: true, + child: MouseRegion( + child: GestureDetector( + behavior: HitTestBehavior.translucent, + onTap: () => taps += 1, + child: const SizedBox(width: 80, height: 40), + ), + ), + ), + ), + ), + ), + ), + ), + ); + await tester.pump(); + + final findResponse = await extDuskFindHandler( + 'ext.dusk.find', + {'semanticsLabel': 'toggle-x'}, + ); + final String qRef = (jsonDecode(findResponse.result!) + as Map)['ref'] as String; + expect(qRef, startsWith('q')); + + // The entry must anchor on the switch box (top-left), NOT the viewport: + // a root fallback here is exactly what made the live MSSwitch taps miss. + final RefEntry? entry = resolveRefForAction(qRef); + expect(entry, isNotNull); + final Rect dispatchRect = dispatchRectOf(entry!)!; + expect(dispatchRect.center.dx, lessThan(200)); + expect(dispatchRect.center.dy, lessThan(120)); + + final future = aiTestTapHandler( + 'ext.dusk.tap', + { + 'ref': qRef, + 'checkStable': 'false', + 'checkReceivesEvents': 'false', + }, + ); + await tester.pump(const Duration(milliseconds: 100)); + await tester.pump(); + await tester.pump(); + final response = await future; + + expect(response.errorDetail, isNull); + expect(taps, greaterThan(0)); + }, + ); + + testWidgets( + '(c) find prefers the interactive node when a label collides with text', + (WidgetTester tester) async { + tester.view.physicalSize = const Size(800, 600); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.resetPhysicalSize); + addTearDown(tester.view.resetDevicePixelRatio); + + // A visible label Text and the control it names share a label (the + // settings-toggle / MSSwitch pattern, and the login heading-vs-button + // pattern). The plain Text sits FIRST in tree order. find must resolve + // the interactive control, not the inert text, so the tap lands on the + // control. + int taps = 0; + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Column( + children: [ + const Text('Email'), + Semantics( + button: true, + label: 'Email', + onTap: () => taps += 1, + excludeSemantics: true, + child: GestureDetector( + behavior: HitTestBehavior.opaque, + onTap: () => taps += 1, + child: const SizedBox(width: 80, height: 40), + ), + ), + ], + ), + ), + ), + ); + await tester.pump(); + + final findResponse = await extDuskFindHandler( + 'ext.dusk.find', + {'semanticsLabel': 'Email'}, + ); + final decoded = + jsonDecode(findResponse.result!) as Map; + final String qRef = decoded['ref'] as String; + // Both the Text and the button carry the label. + expect(decoded['matchCount'], 2); + + final future = aiTestTapHandler( + 'ext.dusk.tap', + { + 'ref': qRef, + 'checkStable': 'false', + 'checkReceivesEvents': 'false', + }, + ); + await tester.pump(const Duration(milliseconds: 100)); + await tester.pump(); + await tester.pump(); + await future; + // The button's onTap fired, not the inert Text. + expect(taps, greaterThan(0)); + }, + ); + + testWidgets( + '(c) find prefers a tappable non-button node over colliding inert text', + (WidgetTester tester) async { + tester.view.physicalSize = const Size(800, 600); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.resetPhysicalSize); + addTearDown(tester.view.resetDevicePixelRatio); + + // A tappable navigation row (a ListTile-style control) exposes + // SemanticsAction.tap WITHOUT the button flag, so the interactive + // preference must fall past isButton/isTextField to the tap-action + // check. An inert Text sharing the label sits first in tree order. + int taps = 0; + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Column( + children: [ + const Text('Monitors'), + Semantics( + label: 'Monitors', + onTap: () => taps += 1, + excludeSemantics: true, + child: GestureDetector( + behavior: HitTestBehavior.opaque, + onTap: () => taps += 1, + child: const SizedBox(width: 120, height: 44), + ), + ), + ], + ), + ), + ), + ); + await tester.pump(); + + final findResponse = await extDuskFindHandler( + 'ext.dusk.find', + {'semanticsLabel': 'Monitors'}, + ); + final decoded = + jsonDecode(findResponse.result!) as Map; + final String qRef = decoded['ref'] as String; + // Both the Text and the tappable row carry the label. + expect(decoded['matchCount'], 2); + + final future = aiTestTapHandler( + 'ext.dusk.tap', + { + 'ref': qRef, + 'checkStable': 'false', + 'checkReceivesEvents': 'false', + }, + ); + await tester.pump(const Duration(milliseconds: 100)); + await tester.pump(); + await tester.pump(); + await future; + // The tappable row fired, not the inert Text: the tap-action branch of + // the interactive check selected it despite the absent button flag. + expect(taps, greaterThan(0)); + }, + ); + testWidgets( '(c) q-ref survives a widget rebuild that would invalidate an eN', (WidgetTester tester) async { @@ -660,4 +954,47 @@ void main() { expect(next, equals('q1')); }); }); + + group('globalRectFromSemantics (transform + DPR fallback)', () { + // This fallback fires only for a synthetic / non-RenderBox-owned node. + // Every node matched through extDuskFindHandler with a real widget tree + // resolves to its owning RenderBox (proven across scroll/merge/link/list + // structures), so the fallback is unreachable via the public handler. Its + // transform-composition + DPR-correction math is verified directly here. + + testWidgets( + 'composes the node transform and divides the rect by the DPR', + (WidgetTester tester) async { + tester.view.devicePixelRatio = 2.0; + addTearDown(tester.view.resetDevicePixelRatio); + + final SemanticsNode node = SemanticsNode() + ..rect = const Rect.fromLTWH(0, 0, 200, 100) + ..transform = Matrix4.identity(); + + // Semantics rects fold in the DPR (physical px); the fallback divides + // it back to the logical space dusk dispatches pointers in. + expect( + globalRectFromSemantics(node), + const Rect.fromLTWH(0, 0, 100, 50), + ); + }, + ); + + testWidgets( + 'leaves the rect untouched at DPR 1 and with no transform', + (WidgetTester tester) async { + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.resetDevicePixelRatio); + + final SemanticsNode node = SemanticsNode() + ..rect = const Rect.fromLTWH(10, 20, 60, 40); + + expect( + globalRectFromSemantics(node), + const Rect.fromLTWH(10, 20, 60, 40), + ); + }, + ); + }); }