firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[web] Detect scrollable semantics nodes more reliably (#164491)

Browse Source 
When a text field is inside a scrollable, and the virtual keyboard shows
up, it (sometimes) makes the scrollable semantics node have a 0 extent.
In that case, the scrollable node has no scroll actions attached. In the
web engine, we detect that as a change of roles (from scrollable to
generic) which causes a DOM mutation above the text field, so the
browser shifts focus to the `<body>`.

In order to avoid this bug, this PR changes how we detect a scrollable
node by checking for the
[`hasImplicitScrolling`](https://api.flutter.dev/flutter/dart-ui/SemanticsFlag/hasImplicitScrolling-constant.html)
flag.

Fixes https://github.com/flutter/flutter/issues/154741
This commit is contained in:
Mouad Debbar 2025-03-06 09:56:27 -05:00 committed by GitHub
parent 099e6d39fe
commit d778ed25a4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 45 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
engine/src/flutter/lib/ui/semantics.dart
View File

@ -1054,7 +1054,7 @@ abstract class SemanticsUpdateBuilder {
/// ///
/// For scrollable nodes `scrollPosition` describes the current scroll /// For scrollable nodes `scrollPosition` describes the current scroll
/// position in logical pixel. `scrollExtentMax` and `scrollExtentMin` /// position in logical pixel. `scrollExtentMax` and `scrollExtentMin`
/// describe the maximum and minimum in-rage values that `scrollPosition` can /// describe the maximum and minimum in-range values that `scrollPosition` can
/// be. Both or either may be infinity to indicate unbound scrolling. The /// be. Both or either may be infinity to indicate unbound scrolling. The
/// value for `scrollPosition` can (temporarily) be outside this range, for /// value for `scrollPosition` can (temporarily) be outside this range, for
/// example during an overscroll. `scrollChildren` is the count of the /// example during an overscroll. `scrollChildren` is the count of the

16
engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart
View File

@ -233,21 +233,21 @@ class SemanticScrollable extends SemanticRole {
// Note that on Android overflow:hidden also works. However, we prefer // Note that on Android overflow:hidden also works. However, we prefer
// "scroll" because it works both on Android and iOS. // "scroll" because it works both on Android and iOS.
if (semanticsObject.isVerticalScrollContainer) { if (semanticsObject.isVerticalScrollContainer) {
// This will reset both `overflow-x` and `overflow-y`.
element.style.removeProperty('overflow');
element.style.overflowY = 'scroll'; element.style.overflowY = 'scroll';
} else { } else if (semanticsObject.isHorizontalScrollContainer) {
assert(semanticsObject.isHorizontalScrollContainer); // This will reset both `overflow-x` and `overflow-y`.
element.style.removeProperty('overflow');
element.style.overflowX = 'scroll'; element.style.overflowX = 'scroll';
} else {
element.style.overflow = 'hidden';
} }
case GestureMode.pointerEvents: case GestureMode.pointerEvents:
// We use "hidden" instead of "scroll" so that the browser does // We use "hidden" instead of "scroll" so that the browser does
// not "steal" pointer events. Flutter gesture recognizers need // not "steal" pointer events. Flutter gesture recognizers need
// all pointer events in order to recognize gestures correctly. // all pointer events in order to recognize gestures correctly.
if (semanticsObject.isVerticalScrollContainer) { element.style.overflow = 'hidden';
element.style.overflowY = 'hidden';
} else {
assert(semanticsObject.isHorizontalScrollContainer);
element.style.overflowX = 'hidden';
}
} }
} }

6
engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart
View File

@ -1365,7 +1365,11 @@ class SemanticsObject {
hasAction(ui.SemanticsAction.scrollLeft) || hasAction(ui.SemanticsAction.scrollRight); hasAction(ui.SemanticsAction.scrollLeft) || hasAction(ui.SemanticsAction.scrollRight);
/// Whether this object represents a scrollable area in any direction. /// Whether this object represents a scrollable area in any direction.
bool get isScrollContainer => isVerticalScrollContainer || isHorizontalScrollContainer; ///
/// When the scrollable container has no scroll extent, it won't have any scroll actions, but
/// it's still a scrollable container. In this case, we need to use the implicit scrolling flag
/// to check for scrollability.
bool get isScrollContainer => hasFlag(ui.SemanticsFlag.hasImplicitScrolling);
/// Whether this object has a non-empty list of children. /// Whether this object has a non-empty list of children.
bool get hasChildren => bool get hasChildren =>

31
engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_test.dart
View File

@ -1321,6 +1321,30 @@ void _testContainer() {
} }
void _testVerticalScrolling() { void _testVerticalScrolling() {
test('recognizes scrollable node when scroll 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 DomElement scrollable = findScrollable(owner());
expect(scrollable.scrollTop, isZero);
semantics().semanticsEnabled = false;
});
test('renders an empty scrollable node', () async { test('renders an empty scrollable node', () async {
semantics() semantics()
..debugOverrideTimestampFunction(() => _testTime) ..debugOverrideTimestampFunction(() => _testTime)
@ -1329,6 +1353,7 @@ void _testVerticalScrolling() {
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
updateNode( updateNode(
builder, builder,
flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
actions: 0 | ui.SemanticsAction.scrollUp.index, actions: 0 | ui.SemanticsAction.scrollUp.index,
transform: Matrix4.identity().toFloat64(), transform: Matrix4.identity().toFloat64(),
rect: const ui.Rect.fromLTRB(0, 0, 50, 100), rect: const ui.Rect.fromLTRB(0, 0, 50, 100),
@ -1353,6 +1378,7 @@ void _testVerticalScrolling() {
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
updateNode( updateNode(
builder, builder,
flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
actions: 0 | ui.SemanticsAction.scrollUp.index, actions: 0 | ui.SemanticsAction.scrollUp.index,
transform: Matrix4.identity().toFloat64(), transform: Matrix4.identity().toFloat64(),
rect: const ui.Rect.fromLTRB(0, 0, 50, 100), rect: const ui.Rect.fromLTRB(0, 0, 50, 100),
@ -1405,6 +1431,7 @@ void _testVerticalScrolling() {
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
updateNode( updateNode(
builder, builder,
flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
actions: 0 | ui.SemanticsAction.scrollUp.index | ui.SemanticsAction.scrollDown.index, actions: 0 | ui.SemanticsAction.scrollUp.index | ui.SemanticsAction.scrollDown.index,
transform: Matrix4.identity().toFloat64(), transform: Matrix4.identity().toFloat64(),
rect: const ui.Rect.fromLTRB(0, 0, 50, 100), rect: const ui.Rect.fromLTRB(0, 0, 50, 100),
@ -1485,6 +1512,7 @@ void _testVerticalScrolling() {
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
updateNode( updateNode(
builder, builder,
flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
actions: 0 | ui.SemanticsAction.scrollUp.index | ui.SemanticsAction.scrollDown.index, actions: 0 | ui.SemanticsAction.scrollUp.index | ui.SemanticsAction.scrollDown.index,
transform: Matrix4.identity().toFloat64(), transform: Matrix4.identity().toFloat64(),
rect: const ui.Rect.fromLTRB(0, 0, 50, 100), rect: const ui.Rect.fromLTRB(0, 0, 50, 100),
@ -1554,6 +1582,7 @@ void _testHorizontalScrolling() {
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
updateNode( updateNode(
builder, builder,
flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
actions: 0 | ui.SemanticsAction.scrollLeft.index, actions: 0 | ui.SemanticsAction.scrollLeft.index,
transform: Matrix4.identity().toFloat64(), transform: Matrix4.identity().toFloat64(),
rect: const ui.Rect.fromLTRB(0, 0, 100, 50), rect: const ui.Rect.fromLTRB(0, 0, 100, 50),
@ -1576,6 +1605,7 @@ void _testHorizontalScrolling() {
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
updateNode( updateNode(
builder, builder,
flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
actions: 0 | ui.SemanticsAction.scrollLeft.index, actions: 0 | ui.SemanticsAction.scrollLeft.index,
transform: Matrix4.identity().toFloat64(), transform: Matrix4.identity().toFloat64(),
rect: const ui.Rect.fromLTRB(0, 0, 100, 50), rect: const ui.Rect.fromLTRB(0, 0, 100, 50),
@ -1628,6 +1658,7 @@ void _testHorizontalScrolling() {
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
updateNode( updateNode(
builder, builder,
flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
actions: 0 | ui.SemanticsAction.scrollLeft.index | ui.SemanticsAction.scrollRight.index, actions: 0 | ui.SemanticsAction.scrollLeft.index | ui.SemanticsAction.scrollRight.index,
transform: Matrix4.identity().toFloat64(), transform: Matrix4.identity().toFloat64(),
rect: const ui.Rect.fromLTRB(0, 0, 100, 50), rect: const ui.Rect.fromLTRB(0, 0, 100, 50),