[web] Detect scrollable semantics nodes more reliably by mdebbar · Pull Request #164491 · flutter/flutter

mdebbar · 2025-03-03T17:14:53Z

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 flag.

Fixes #154741

yjbanov · 2025-03-03T18:54:33Z

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

-          assert(semanticsObject.isHorizontalScrollContainer);
-          element.style.overflowX = 'hidden';
-        }
+        element.style.overflow = 'hidden';


When gesture mode flips from browser to pointer events, this will result in overflowY == 'scroll' and overflow == 'hidden'. Is that a good state to be in?

The vice versa situation (flipping back to browser gesture mode) seems equally questionable.

Tried in a jsfiddle, the following:

element.style.overflow = 'hidden'; element.style.overflowY = 'scroll';

it results in the element having overflowY: scroll and overflowX: hidden.

But in order to avoid this whole situation, we can just always do:

element.style.removeProperty('overflow');

whenever the gesture mode is changed. WDYT?

In that case, I think when we're setting overflow we should remove overflowX/Y?

chunhtai · 2025-03-03T19:05:03Z

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

  /// For scrollable nodes `scrollPosition` describes the current scroll
  /// 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


lol, nice catch

chunhtai · 2025-03-03T19:10:08Z

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

+  /// to check for scrollability.
+  bool get isScrollContainer =>
+      hasFlag(ui.SemanticsFlag.hasImplicitScrolling) ||
+      isVerticalScrollContainer ||


I will expect if either isVerticalScrollContainer or isHorizontalScrollContainer is true, the hasImplicitScrolling must also be true. so I think we can just check hasImplicitScrolling should be enough

yjbanov

chunhtai

LGTM

…r/flutter#164491)

flutter/flutter@2578d97...321fbc0 2025-03-06 [email protected] Roll Skia from fefecd49e03a to ccd8cc23aa94 (1 revision) (flutter/flutter#164712) 2025-03-06 [email protected] [web] Detect scrollable semantics nodes more reliably (flutter/flutter#164491) 2025-03-06 [email protected] [windows] wire the focus request and the focus events through the Windows platform (flutter/flutter#164296) 2025-03-06 [email protected] Roll Skia from 02897747c7d5 to fefecd49e03a (1 revision) (flutter/flutter#164701) 2025-03-06 [email protected] Roll Skia from e315b0ab7c84 to 02897747c7d5 (1 revision) (flutter/flutter#164677) 2025-03-06 [email protected] Roll Skia from 0c3880f94970 to e315b0ab7c84 (1 revision) (flutter/flutter#164669) 2025-03-06 [email protected] [Impeller] use device private on non-iOS devices. (flutter/flutter#164601) 2025-03-05 [email protected] Roll Skia from 43294a662fd0 to 0c3880f94970 (1 revision) (flutter/flutter#164661) 2025-03-05 [email protected] Add a workflow (only triggered from rest events) for hasing experiment (flutter/flutter#164657) 2025-03-05 [email protected] Roll Skia from 4cf9f0b77d41 to 43294a662fd0 (4 revisions) (flutter/flutter#164649) 2025-03-05 [email protected] Adds animateToItem to the CarouselController (flutter/flutter#162694) 2025-03-05 [email protected] Cleanup content context (flutter/flutter#164229) 2025-03-05 [email protected] Fix: Update CupertinoSheetRoute transition rounded corner (flutter/flutter#163700) 2025-03-05 [email protected] [Impeller] fix macOS managed memory. (flutter/flutter#164635) 2025-03-05 [email protected] [skwasm] Clear font collection cache when font is loaded manually. (flutter/flutter#164588) 2025-03-05 [email protected] Fix race condition causing crash when interacting with an animating scrollable (flutter/flutter#164392) 2025-03-05 [email protected] Use dwds 24.3.6 and pass uri for the reload scripts path to FrontendServerDdcLibraryBundleProvider (flutter/flutter#164582) 2025-03-05 [email protected] Roll Packages from 9e4684e to abba683 (8 revisions) (flutter/flutter#164630) 2025-03-05 [email protected] Roll Skia from 7e4323f72c9d to 4cf9f0b77d41 (1 revision) (flutter/flutter#164622) 2025-03-05 [email protected] Fix to Linux_pixel_7pro integration_ui_keyboard_resize test flakiness (flutter/flutter#162308) 2025-03-05 [email protected] Roll Skia from 03a3f653d64e to 7e4323f72c9d (1 revision) (flutter/flutter#164599) 2025-03-05 [email protected] Implement `clipPath` Mutator for hcpp (flutter/flutter#164525) 2025-03-05 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] use DeviceLocal textures for gifs on non-iOS devices. (#164573)" (flutter/flutter#164600) 2025-03-05 [email protected] [macos] prefer integrated GPU. (flutter/flutter#164569) 2025-03-05 [email protected] Enforce minSdk constraint for Android Flutter (flutter/flutter#164251) 2025-03-05 [email protected] Add `clipRSuperellipse`, and use them for dialogs (flutter/flutter#161111) 2025-03-05 [email protected] Roll Skia from 46705a22edc3 to 03a3f653d64e (1 revision) (flutter/flutter#164590) 2025-03-05 [email protected] when resetting FlutterPlatformViewsController, clear out some additional internal state to prevent it from carrying over across a Hot Restart (flutter/flutter#164456) 2025-03-05 [email protected] Roll Fuchsia Linux SDK from Rt6pxGFLVAJHduM0V... to fhm5z889sA5T1AQao... (flutter/flutter#164583) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…r/flutter#164491)

flutter/flutter@2578d97...321fbc0 2025-03-06 [email protected] Roll Skia from fefecd49e03a to ccd8cc23aa94 (1 revision) (flutter/flutter#164712) 2025-03-06 [email protected] [web] Detect scrollable semantics nodes more reliably (flutter/flutter#164491) 2025-03-06 [email protected] [windows] wire the focus request and the focus events through the Windows platform (flutter/flutter#164296) 2025-03-06 [email protected] Roll Skia from 02897747c7d5 to fefecd49e03a (1 revision) (flutter/flutter#164701) 2025-03-06 [email protected] Roll Skia from e315b0ab7c84 to 02897747c7d5 (1 revision) (flutter/flutter#164677) 2025-03-06 [email protected] Roll Skia from 0c3880f94970 to e315b0ab7c84 (1 revision) (flutter/flutter#164669) 2025-03-06 [email protected] [Impeller] use device private on non-iOS devices. (flutter/flutter#164601) 2025-03-05 [email protected] Roll Skia from 43294a662fd0 to 0c3880f94970 (1 revision) (flutter/flutter#164661) 2025-03-05 [email protected] Add a workflow (only triggered from rest events) for hasing experiment (flutter/flutter#164657) 2025-03-05 [email protected] Roll Skia from 4cf9f0b77d41 to 43294a662fd0 (4 revisions) (flutter/flutter#164649) 2025-03-05 [email protected] Adds animateToItem to the CarouselController (flutter/flutter#162694) 2025-03-05 [email protected] Cleanup content context (flutter/flutter#164229) 2025-03-05 [email protected] Fix: Update CupertinoSheetRoute transition rounded corner (flutter/flutter#163700) 2025-03-05 [email protected] [Impeller] fix macOS managed memory. (flutter/flutter#164635) 2025-03-05 [email protected] [skwasm] Clear font collection cache when font is loaded manually. (flutter/flutter#164588) 2025-03-05 [email protected] Fix race condition causing crash when interacting with an animating scrollable (flutter/flutter#164392) 2025-03-05 [email protected] Use dwds 24.3.6 and pass uri for the reload scripts path to FrontendServerDdcLibraryBundleProvider (flutter/flutter#164582) 2025-03-05 [email protected] Roll Packages from 9e4684e to abba683 (8 revisions) (flutter/flutter#164630) 2025-03-05 [email protected] Roll Skia from 7e4323f72c9d to 4cf9f0b77d41 (1 revision) (flutter/flutter#164622) 2025-03-05 [email protected] Fix to Linux_pixel_7pro integration_ui_keyboard_resize test flakiness (flutter/flutter#162308) 2025-03-05 [email protected] Roll Skia from 03a3f653d64e to 7e4323f72c9d (1 revision) (flutter/flutter#164599) 2025-03-05 [email protected] Implement `clipPath` Mutator for hcpp (flutter/flutter#164525) 2025-03-05 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] use DeviceLocal textures for gifs on non-iOS devices. (#164573)" (flutter/flutter#164600) 2025-03-05 [email protected] [macos] prefer integrated GPU. (flutter/flutter#164569) 2025-03-05 [email protected] Enforce minSdk constraint for Android Flutter (flutter/flutter#164251) 2025-03-05 [email protected] Add `clipRSuperellipse`, and use them for dialogs (flutter/flutter#161111) 2025-03-05 [email protected] Roll Skia from 46705a22edc3 to 03a3f653d64e (1 revision) (flutter/flutter#164590) 2025-03-05 [email protected] when resetting FlutterPlatformViewsController, clear out some additional internal state to prevent it from carrying over across a Hot Restart (flutter/flutter#164456) 2025-03-05 [email protected] Roll Fuchsia Linux SDK from Rt6pxGFLVAJHduM0V... to fhm5z889sA5T1AQao... (flutter/flutter#164583) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

[web] Detect scrollable semantics nodes more reliably

533a01b

mdebbar requested review from chunhtai and yjbanov March 3, 2025 17:14

github-actions bot added engine flutter/engine related. See also e: labels. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: scrolling Viewports, list views, slivers, etc. platform-web Web applications specifically labels Mar 3, 2025

yjbanov reviewed Mar 3, 2025

View reviewed changes

chunhtai reviewed Mar 3, 2025

View reviewed changes

mdebbar added 3 commits March 4, 2025 15:07

Merge branch 'main' into text_field_focus

883304c

rely solely on the hasImplicitScrolling flag

6ba7f6a

clear overflow before setting it on one axis only

5d6afc9

mdebbar requested review from chunhtai and yjbanov March 4, 2025 21:03

add clarifying comments

18e91cd

yjbanov approved these changes Mar 5, 2025

View reviewed changes

chunhtai approved these changes Mar 5, 2025

View reviewed changes

mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 5, 2025

auto-submit bot added this pull request to the merge queue Mar 6, 2025

Merged via the queue into flutter:master with commit d778ed2 Mar 6, 2025
176 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 6, 2025

engine-flutter-autoroll mentioned this pull request Mar 6, 2025

Roll Flutter from 2578d97f32c8 to d778ed25a485 (28 revisions) flutter/packages#8807

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

d778ed2 [web] Detect scrollable semantics nodes more reliably (flutte…

163f3c2

…r/flutter#164491)

engine-flutter-autoroll mentioned this pull request Mar 6, 2025

Roll Flutter from 2578d97f32c8 to 321fbc0e7e81 (29 revisions) flutter/packages#8808

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

d778ed2 [web] Detect scrollable semantics nodes more reliably (flutte…

d2041f7

…r/flutter#164491)

Piinks mentioned this pull request Mar 7, 2025

Flutter Notable Commits - 2025 #161460

Closed

mdebbar deleted the text_field_focus branch March 10, 2025 15:01

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

d778ed2 [web] Detect scrollable semantics nodes more reliably (flutte…

88d481d

…r/flutter#164491)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

d778ed2 [web] Detect scrollable semantics nodes more reliably (flutte…

ac0d4a7

…r/flutter#164491)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025

d778ed2 [web] Detect scrollable semantics nodes more reliably (flutte…

c7d41fd

…r/flutter#164491)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[web] Detect scrollable semantics nodes more reliably#164491

[web] Detect scrollable semantics nodes more reliably#164491
auto-submit[bot] merged 5 commits intoflutter:masterfrom
mdebbar:text_field_focus

mdebbar commented Mar 3, 2025

Uh oh!

yjbanov Mar 3, 2025

Uh oh!

mdebbar Mar 3, 2025

Uh oh!

mdebbar Mar 4, 2025

Uh oh!

yjbanov Mar 4, 2025

Uh oh!

chunhtai Mar 3, 2025

Uh oh!

chunhtai Mar 3, 2025

Uh oh!

mdebbar Mar 4, 2025

Uh oh!

yjbanov left a comment

Uh oh!

chunhtai left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mdebbar commented Mar 3, 2025

Uh oh!

yjbanov Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

mdebbar Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

mdebbar Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

yjbanov Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

chunhtai Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

chunhtai Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

mdebbar Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

yjbanov left a comment

Choose a reason for hiding this comment

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants