From da0b736e58e618803d4ff3007555fc83d55b3090 Mon Sep 17 00:00:00 2001 From: flutter-zl Date: Mon, 20 Oct 2025 12:05:46 -0700 Subject: [PATCH] Address review feedback: framework declaration takes precedence --- engine/src/flutter/lib/ui/semantics.dart | 15 +++-- .../lib/src/engine/semantics/semantics.dart | 66 +++++++++---------- .../test/engine/semantics/semantics_test.dart | 4 +- 3 files changed, 41 insertions(+), 44 deletions(-) diff --git a/engine/src/flutter/lib/ui/semantics.dart b/engine/src/flutter/lib/ui/semantics.dart index a493c795c2d..586d06c4c47 100644 --- a/engine/src/flutter/lib/ui/semantics.dart +++ b/engine/src/flutter/lib/ui/semantics.dart @@ -1193,15 +1193,16 @@ enum SemanticsHitTestBehavior { /// The semantic element is transparent to hit testing. /// - /// In the framework's hit testing model, transparent nodes receive hit test - /// events themselves but do not block siblings or children behind them from - /// also receiving events. + /// Transparent nodes do not receive hit test events and allow events to pass + /// through to elements behind them. + /// + /// Note: This differs from the framework's `HitTestBehavior.translucent`, + /// which receives events while also allowing pass-through. Web's binary + /// `pointer-events` property (all or none) cannot support true translucent + /// behavior. /// /// Platform implementations: - /// * On the web, this currently results in `pointer-events: none` CSS - /// property, which prevents the element from receiving events entirely. - /// This is a simplified implementation of the framework's transparent - /// behavior. + /// * On the web, this results in `pointer-events: none` CSS property. /// /// This is the default behavior for non-interactive semantic nodes when the /// platform infers behavior. diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart index 29ad3b8ca3c..fbed6dd87d8 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -586,50 +586,34 @@ abstract class SemanticRole { /// This boolean decides whether to set the `pointer-events` CSS property to /// `all` or to `none` on the semantics [element]. /// - /// The decision follows a three-tier policy: + /// The decision follows a two-tier policy: /// - /// **Tier 1: Interactive Behaviors** - Interactive elements (buttons, text - /// fields, sliders, etc.) always accept pointer events, regardless of explicit - /// [ui.SemanticsHitTestBehavior]. This ensures accessibility requirements are - /// met - interactive elements must be clickable. + /// **Tier 1: Framework Declaration** - When the framework provides an explicit + /// [ui.SemanticsHitTestBehavior] (other than `defer`), it takes absolute + /// precedence. The engine executes what the framework declares, even if it + /// makes an interactive element non-clickable. The framework layer is + /// responsible for ensuring valid semantic configurations. /// - /// **Tier 2: Framework Declaration** - When the framework provides an explicit - /// [ui.SemanticsHitTestBehavior] (other than `defer`), it takes precedence - /// over inference. This allows the framework to declaratively control pointer - /// event handling for non-interactive elements. - /// - /// **Tier 3: Engine Inference** - When [ui.SemanticsHitTestBehavior.defer] is - /// set (the default) and the element is not interactive, the engine infers the - /// appropriate behavior based on: + /// **Tier 2: Engine Inference** - When [ui.SemanticsHitTestBehavior.defer] is + /// set (the default), the engine infers the appropriate behavior based on: + /// - Interactive behaviors (buttons, text fields, sliders, etc.) /// - Route-scoped containers (dialogs, bottom sheets) /// - Default transparent for non-interactive leaf nodes /// - /// This approach ensures interactive elements always work, while allowing - /// framework control and providing intelligent fallback inference for backward - /// compatibility. + /// This approach allows framework full control when specified, with intelligent + /// fallback inference for backward compatibility. bool get acceptsPointerEvents { - // TIER 1: Interactive Behaviors - // Check if any interactive behavior requires pointer events. - // This takes highest precedence to ensure buttons, text fields, etc. - // are always clickable, even if explicit hitTestBehavior is set. - final behaviors = _behaviors; - if (behaviors != null) { - for (final behavior in behaviors) { - if (behavior.acceptsPointerEvents) { - return true; - } - } - } - final hitTestBehavior = semanticsObject.hitTestBehavior; - // TIER 2: Framework Declaration - // If framework provides explicit behavior (not defer), respect it. + // TIER 1: Framework Declaration + // If framework provides explicit behavior (not defer), respect it absolutely. + // The framework is responsible for ensuring valid configurations (e.g., not + // making buttons transparent). if (hitTestBehavior != ui.SemanticsHitTestBehavior.defer) { return _shouldAcceptPointerEventsFromBehavior(hitTestBehavior); } - // TIER 3: Engine Inference + // TIER 2: Engine Inference // When framework defers to engine, infer based on semantic properties. return _inferAcceptsPointerEvents(); } @@ -660,10 +644,22 @@ abstract class SemanticRole { /// Infers whether pointer events should be accepted based on semantic properties. /// - /// This method is called when [ui.SemanticsHitTestBehavior.defer] is set - /// and the element has no interactive behaviors, providing backward-compatible - /// inference logic for non-interactive elements. + /// This method is called when [ui.SemanticsHitTestBehavior.defer] is set, + /// providing backward-compatible inference logic. bool _inferAcceptsPointerEvents() { + // Check if any interactive behavior requires pointer events. + // Interactive behaviors (Tappable, SemanticTextField, SemanticIncrementable) + // override this to return true, ensuring buttons, text fields, and other + // interactive elements receive pointer events when framework defers. + final behaviors = _behaviors; + if (behaviors != null) { + for (final behavior in behaviors) { + if (behavior.acceptsPointerEvents) { + return true; + } + } + } + // Route-scoped containers accept pointer events. // This fixes the dialog dismissal bug (issue #149001) by ensuring that // dialog containers act as barriers, preventing clicks on empty space diff --git a/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_test.dart b/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_test.dart index 97f91ada09f..e1129ef7271 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_test.dart @@ -1765,9 +1765,9 @@ void _testContainer() { )!; expect( element.style.pointerEvents, - 'all', + 'none', reason: - 'Interactive behaviors (Tier 1) should take precedence over hitTestBehavior (Tier 2)', + 'Framework declaration (Tier 1) should take precedence over interactive behaviors (Tier 2)', ); }); });