From b02c436c344e322505b783e6bb81d09fd13a75ef Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 25 Mar 2025 11:18:54 -0400 Subject: [PATCH] [web] Fix semantic scrollable when there are no scroll actions (#165064) --- .../lib/src/engine/semantics/scrollable.dart | 43 ++++++++++++++----- .../test/engine/semantics/semantics_test.dart | 28 ++++++++++++ 2 files changed, 60 insertions(+), 11 deletions(-) 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 index 08e96af35d..a281ae4881 100644 --- 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 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 index 163489847c..983982f3b5 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 @@ -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(), ''' + + + '''); + + 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)