Skip to content

Commit d778ed2

Browse files
mdebbarmdebbar
authored
[web] Detect scrollable semantics nodes more reliably (#164491)
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 flutter/flutter#154741
1 parent 099e6d3 commit d778ed2

File tree

4 files changed

+45
-10
lines changed

4 files changed

+45
-10
lines changed

‎engine/src/flutter/lib/ui/semantics.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1054,7 +1054,7 @@ abstract class SemanticsUpdateBuilder {
10541054
///
10551055
/// For scrollable nodes `scrollPosition` describes the current scroll
10561056
/// position in logical pixel. `scrollExtentMax` and `scrollExtentMin`
1057-
/// describe the maximum and minimum in-rage values that `scrollPosition` can
1057+
/// describe the maximum and minimum in-range values that `scrollPosition` can
10581058
/// be. Both or either may be infinity to indicate unbound scrolling. The
10591059
/// value for `scrollPosition` can (temporarily) be outside this range, for
10601060
/// example during an overscroll. `scrollChildren` is the count of the

‎engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -233,21 +233,21 @@ class SemanticScrollable extends SemanticRole {
233233
// Note that on Android overflow:hidden also works. However, we prefer
234234
// "scroll" because it works both on Android and iOS.
235235
if (semanticsObject.isVerticalScrollContainer) {
236+
// This will reset both `overflow-x` and `overflow-y`.
237+
element.style.removeProperty('overflow');
236238
element.style.overflowY = 'scroll';
237-
} else {
238-
assert(semanticsObject.isHorizontalScrollContainer);
239+
} else if (semanticsObject.isHorizontalScrollContainer) {
240+
// This will reset both `overflow-x` and `overflow-y`.
241+
element.style.removeProperty('overflow');
239242
element.style.overflowX = 'scroll';
243+
} else {
244+
element.style.overflow = 'hidden';
240245
}
241246
case GestureMode.pointerEvents:
242247
// We use "hidden" instead of "scroll" so that the browser does
243248
// not "steal" pointer events. Flutter gesture recognizers need
244249
// all pointer events in order to recognize gestures correctly.
245-
if (semanticsObject.isVerticalScrollContainer) {
246-
element.style.overflowY = 'hidden';
247-
} else {
248-
assert(semanticsObject.isHorizontalScrollContainer);
249-
element.style.overflowX = 'hidden';
250-
}
250+
element.style.overflow = 'hidden';
251251
}
252252
}
253253

‎engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1365,7 +1365,11 @@ class SemanticsObject {
13651365
hasAction(ui.SemanticsAction.scrollLeft) || hasAction(ui.SemanticsAction.scrollRight);
13661366

13671367
/// Whether this object represents a scrollable area in any direction.
1368-
bool get isScrollContainer => isVerticalScrollContainer || isHorizontalScrollContainer;
1368+
///
1369+
/// When the scrollable container has no scroll extent, it won't have any scroll actions, but
1370+
/// it's still a scrollable container. In this case, we need to use the implicit scrolling flag
1371+
/// to check for scrollability.
1372+
bool get isScrollContainer => hasFlag(ui.SemanticsFlag.hasImplicitScrolling);
13691373

13701374
/// Whether this object has a non-empty list of children.
13711375
bool get hasChildren =>

‎engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_test.dart‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1321,6 +1321,30 @@ void _testContainer() {
13211321
}
13221322

13231323
void _testVerticalScrolling() {
1324+
test('recognizes scrollable node when scroll actions not available', () async {
1325+
semantics()
1326+
..debugOverrideTimestampFunction(() => _testTime)
1327+
..semanticsEnabled = true;
1328+
1329+
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
1330+
updateNode(
1331+
builder,
1332+
flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
1333+
transform: Matrix4.identity().toFloat64(),
1334+
rect: const ui.Rect.fromLTRB(0, 0, 50, 100),
1335+
);
1336+
1337+
owner().updateSemantics(builder.build());
1338+
expectSemanticsTree(owner(), '''
1339+
<sem role="group" style="touch-action: none">
1340+
<flt-semantics-scroll-overflow></flt-semantics-scroll-overflow>
1341+
</sem>''');
1342+
1343+
final DomElement scrollable = findScrollable(owner());
1344+
expect(scrollable.scrollTop, isZero);
1345+
semantics().semanticsEnabled = false;
1346+
});
1347+
13241348
test('renders an empty scrollable node', () async {
13251349
semantics()
13261350
..debugOverrideTimestampFunction(() => _testTime)
@@ -1329,6 +1353,7 @@ void _testVerticalScrolling() {
13291353
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
13301354
updateNode(
13311355
builder,
1356+
flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
13321357
actions: 0 | ui.SemanticsAction.scrollUp.index,
13331358
transform: Matrix4.identity().toFloat64(),
13341359
rect: const ui.Rect.fromLTRB(0, 0, 50, 100),
@@ -1353,6 +1378,7 @@ void _testVerticalScrolling() {
13531378
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
13541379
updateNode(
13551380
builder,
1381+
flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
13561382
actions: 0 | ui.SemanticsAction.scrollUp.index,
13571383
transform: Matrix4.identity().toFloat64(),
13581384
rect: const ui.Rect.fromLTRB(0, 0, 50, 100),
@@ -1405,6 +1431,7 @@ void _testVerticalScrolling() {
14051431
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
14061432
updateNode(
14071433
builder,
1434+
flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
14081435
actions: 0 | ui.SemanticsAction.scrollUp.index | ui.SemanticsAction.scrollDown.index,
14091436
transform: Matrix4.identity().toFloat64(),
14101437
rect: const ui.Rect.fromLTRB(0, 0, 50, 100),
@@ -1485,6 +1512,7 @@ void _testVerticalScrolling() {
14851512
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
14861513
updateNode(
14871514
builder,
1515+
flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
14881516
actions: 0 | ui.SemanticsAction.scrollUp.index | ui.SemanticsAction.scrollDown.index,
14891517
transform: Matrix4.identity().toFloat64(),
14901518
rect: const ui.Rect.fromLTRB(0, 0, 50, 100),
@@ -1554,6 +1582,7 @@ void _testHorizontalScrolling() {
15541582
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
15551583
updateNode(
15561584
builder,
1585+
flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
15571586
actions: 0 | ui.SemanticsAction.scrollLeft.index,
15581587
transform: Matrix4.identity().toFloat64(),
15591588
rect: const ui.Rect.fromLTRB(0, 0, 100, 50),
@@ -1576,6 +1605,7 @@ void _testHorizontalScrolling() {
15761605
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
15771606
updateNode(
15781607
builder,
1608+
flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
15791609
actions: 0 | ui.SemanticsAction.scrollLeft.index,
15801610
transform: Matrix4.identity().toFloat64(),
15811611
rect: const ui.Rect.fromLTRB(0, 0, 100, 50),
@@ -1628,6 +1658,7 @@ void _testHorizontalScrolling() {
16281658
final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder();
16291659
updateNode(
16301660
builder,
1661+
flags: 0 | ui.SemanticsFlag.hasImplicitScrolling.index,
16311662
actions: 0 | ui.SemanticsAction.scrollLeft.index | ui.SemanticsAction.scrollRight.index,
16321663
transform: Matrix4.identity().toFloat64(),
16331664
rect: const ui.Rect.fromLTRB(0, 0, 100, 50),

0 commit comments

Comments
 (0)