[web] Fix semantic scrollable when there are no scroll actions (#165064)

mdebbar · web-flow · commit b02c436c344e · 2025-03-25T15:18:54.000Z
diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:meta/meta.dart';
 import 'package:ui/src/engine.dart';
 import 'package:ui/ui.dart' as ui;
 
@@ -55,12 +56,17 @@ class SemanticScrollable extends SemanticRole {
   ///
   /// This gesture is converted to [ui.SemanticsAction.scrollUp] or
   /// [ui.SemanticsAction.scrollDown], depending on the direction.
-  DomEventListener? _scrollListener;
+  @visibleForTesting
+  DomEventListener? scrollListener;
 
   /// The value of the "scrollTop" or "scrollLeft" property of this object's
   /// [element] that has zero offset relative to the [scrollPosition].
   int _effectiveNeutralScrollPosition = 0;
 
+  /// Whether this scrollable can scroll vertically or horizontally.
+  bool get _canScroll =>
+      semanticsObject.isVerticalScrollContainer || semanticsObject.isHorizontalScrollContainer;
+
   /// Responds to browser-detected "scroll" gestures.
   void _recomputeScrollPosition() {
     if (_domScrollPosition != _effectiveNeutralScrollPosition) {
@@ -135,7 +141,9 @@ class SemanticScrollable extends SemanticRole {
       semanticsObject.updateChildrenPositionAndSize();
     });
 
-    if (_scrollListener == null) {
+    _updateCssOverflow();
+
+    if (scrollListener == null) {
       // We need to set touch-action:none explicitly here, despite the fact
       // that we already have it on the <body> tag because overflow:scroll
       // still causes the browser to take over pointer events in order to
@@ -146,20 +154,22 @@ class SemanticScrollable extends SemanticRole {
       // CSS property. In Safari the `PointerBinding` uses `preventDefault`
       // to prevent browser scrolling.
       element.style.touchAction = 'none';
-      _gestureModeDidChange();
 
       // Memoize the tear-off because Dart does not guarantee that two
       // tear-offs of a method on the same instance will produce the same
       // object.
       _gestureModeListener = (_) {
-        _gestureModeDidChange();
+        _updateCssOverflow();
       };
       EngineSemantics.instance.addGestureModeListener(_gestureModeListener!);
 
-      _scrollListener = createDomEventListener((_) {
+      scrollListener = createDomEventListener((_) {
+        if (!_canScroll) {
+          return;
+        }
         _recomputeScrollPosition();
       });
-      addEventListener('scroll', _scrollListener);
+      addEventListener('scroll', scrollListener);
     }
   }
 
@@ -207,7 +217,7 @@ class SemanticScrollable extends SemanticRole {
       semanticsObject
         ..verticalScrollAdjustment = _effectiveNeutralScrollPosition.toDouble()
         ..horizontalScrollAdjustment = 0.0;
-    } else {
+    } else if (semanticsObject.isHorizontalScrollContainer) {
       // Place the _scrollOverflowElement at the end of the content and
       // make sure that when we neutralize the scrolling position,
       // it doesn't scroll into the visible area.
@@ -223,10 +233,21 @@ class SemanticScrollable extends SemanticRole {
       semanticsObject
         ..verticalScrollAdjustment = 0.0
         ..horizontalScrollAdjustment = _effectiveNeutralScrollPosition.toDouble();
+    } else {
+      _scrollOverflowElement.style
+        ..transform = 'translate(0px,0px)'
+        ..width = '0px'
+        ..height = '0px';
+      element.scrollLeft = 0.0;
+      element.scrollTop = 0.0;
+      _effectiveNeutralScrollPosition = 0;
+      semanticsObject
+        ..verticalScrollAdjustment = 0.0
+        ..horizontalScrollAdjustment = 0.0;
     }
   }
 
-  void _gestureModeDidChange() {
+  void _updateCssOverflow() {
     switch (EngineSemantics.instance.gestureMode) {
       case GestureMode.browserGestures:
         // overflow:scroll will cause the browser report "scroll" events when
@@ -261,9 +282,9 @@ class SemanticScrollable extends SemanticRole {
     style.removeProperty('overflowY');
     style.removeProperty('overflowX');
     style.removeProperty('touch-action');
-    if (_scrollListener != null) {
-      removeEventListener('scroll', _scrollListener);
-      _scrollListener = null;
+    if (scrollListener != null) {
+      removeEventListener('scroll', scrollListener);
+      scrollListener = null;
     }
     if (_gestureModeListener != null) {
       EngineSemantics.instance.removeGestureModeListener(_gestureModeListener!);
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
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:js_interop';
 import 'dart:typed_data';
 
 import 'package:quiver/testing/async.dart';
@@ -1560,6 +1561,33 @@ void _testVerticalScrolling() {
     semantics().semanticsEnabled = false;
   });
 
+  test('scroll events ignored when actions not available', () async {
+    semantics()
+      ..debugOverrideTimestampFunction(() => _testTime)
+      ..semanticsEnabled = true;
+
+    final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
+    updateNode(
+      builder,
+      flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
+      transform: Matrix4.identity().toFloat64(),
+      rect: const ui.Rect.fromLTRB(0, 0, 50, 100),
+    );
+
+    owner().updateSemantics(builder.build());
+    expectSemanticsTree(owner(), '''
+  <sem role="group" style="touch-action: none">
+  <flt-semantics-scroll-overflow></flt-semantics-scroll-overflow>
+  </sem>''');
+
+    final scrollable = owner().debugSemanticsTree![0]!.semanticRole! as SemanticScrollable;
+    final scrollEvent = createDomEvent('Event', 'scroll') as JSAny;
+    final listener = scrollable.scrollListener! as JSFunction;
+    expect(() => listener.callAsFunction(null, scrollEvent), returnsNormally);
+
+    semantics().semanticsEnabled = false;
+  });
+
   test('renders an empty scrollable node', () async {
     semantics()
       ..debugOverrideTimestampFunction(() => _testTime)
