[web] fix unexpected scrolling in semantics (#53922)

Always mark scrollable containers with `role="group"` to prevent the browser from merging all child elements into one giant string. Default to `preventScroll` to true across the web engine code, because it's a better default for Flutter than otherwise.

Fixes flutter/flutter#130950

Author: yjbanov

lib/web_ui/lib/src/engine/clipboard.dart

@@ -184,7 +184,7 @@ class ExecCommandCopyStrategy implements CopyToClipboardStrategy {
     // See: https://developers.google.com/web/updates/2015/04/cut-and-copy-commands
     final DomHTMLTextAreaElement tempTextArea = _appendTemporaryTextArea();
     tempTextArea.value = text;
-    tempTextArea.focus();
+    tempTextArea.focusWithoutScroll();
     tempTextArea.select();
     bool result = false;
     try {

lib/web_ui/lib/src/engine/dom.dart

@@ -661,15 +661,26 @@ extension DomElementExtension on DomElement {
   external JSNumber? get _tabIndex;
   double? get tabIndex => _tabIndex?.toDartDouble;
 
+  /// Consider not exposing this method publicly. It defaults `preventScroll` to
+  /// false, which is almost always wrong in Flutter. If you need to expose a
+  /// method that focuses and scrolls to the element, give it a more specific
+  /// and lengthy name, e.g. `focusAndScrollToElement`. See more details in
+  /// [focusWithoutScroll].
   @JS('focus')
   external JSVoid _focus(JSAny options);
 
-  void focus({bool? preventScroll, bool? focusVisible}) {
-    final Map<String, bool> options = <String, bool>{
-      if (preventScroll != null) 'preventScroll': preventScroll,
-      if (focusVisible != null) 'focusVisible': focusVisible,
-    };
-    _focus(options.toJSAnyDeep);
+  static final JSAny _preventScrollOptions = <String, bool>{ 'preventScroll': true }.toJSAnyDeep;
+
+  /// Calls DOM `Element.focus` with `preventScroll` set to true.
+  ///
+  /// This method exists because DOM `Element.focus` defaults to `preventScroll`
+  /// set to false. This default browser behavior is almost always wrong in the
+  /// Flutter context because the Flutter framework is in charge of scrolling
+  /// all of the widget content. See, for example, this issue:
+  ///
+  /// https://github.com/flutter/flutter/issues/130950
+  void focusWithoutScroll() {
+    _focus(_preventScrollOptions);
   }
 
   @JS('scrollTop')

lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart

@@ -51,7 +51,7 @@ final class ViewFocusBinding {
     if (state == ui.ViewFocusState.focused) {
       // Only move the focus to the flutter view if nothing inside it is focused already.
       if (viewId != _viewId(domDocument.activeElement)) {
-        viewElement?.focus(preventScroll: true);
+        viewElement?.focusWithoutScroll();
       }
     } else {
       viewElement?.blur();

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

@@ -49,7 +49,7 @@ class Focusable extends RoleManager {
   /// decide whether to stop searching for a node that should take focus.
   bool focusAsRouteDefault() {
     _focusManager._lastEvent = AccessibilityFocusManagerEvent.requestedFocus;
-    owner.element.focus();
+    owner.element.focusWithoutScroll();
     return true;
   }
 
@@ -279,7 +279,7 @@ class AccessibilityFocusManager {
       }
 
       _lastEvent = AccessibilityFocusManagerEvent.requestedFocus;
-      target.element.focus();
+      target.element.focusWithoutScroll();
     });
   }
 }

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

@@ -62,7 +62,7 @@ class Incrementable extends PrimaryRoleManager {
 
   @override
   bool focusAsRouteDefault() {
-    _element.focus();
+    _element.focusWithoutScroll();
     return true;
   }
 

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

@@ -99,7 +99,7 @@ abstract final class LabelRepresentationBehavior {
   /// https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/tabindex
   void focusAsRouteDefault() {
     focusTarget.tabIndex = -1;
-    focusTarget.focus();
+    focusTarget.focusWithoutScroll();
   }
 }
 

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

@@ -28,7 +28,12 @@ class Scrollable extends PrimaryRoleManager {
           PrimaryRole.scrollable,
           semanticsObject,
           preferredLabelRepresentation: LabelRepresentation.ariaLabel,
-        );
+        ) {
+    // Mark as group to prevent the browser from merging this element along with
+    // all the children into one giant node. This is what happened with the
+    // repro provided in https://github.com/flutter/flutter/issues/130950.
+    setAriaRole('group');
+  }
 
   /// Disables browser-driven scrolling in the presence of pointer events.
   GestureModeCallback? _gestureModeListener;

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

@@ -162,7 +162,7 @@ class SemanticsTextEditingStrategy extends DefaultTextEditingStrategy {
     if (hasAutofillGroup) {
       placeForm();
     }
-    activeDomElement.focus(preventScroll: true);
+    activeDomElement.focusWithoutScroll();
   }
 
   @override
@@ -219,7 +219,7 @@ class TextField extends PrimaryRoleManager {
 
   @override
   bool focusAsRouteDefault() {
-    editableElement.focus(preventScroll: true);
+    editableElement.focusWithoutScroll();
     return true;
   }
 
@@ -264,7 +264,7 @@ class TextField extends PrimaryRoleManager {
           semanticsObject.id, ui.SemanticsAction.focus, null);
     }));
     editableElement.addEventListener('click', createDomEventListener((DomEvent event) {
-      editableElement.focus(preventScroll: true);
+      editableElement.focusWithoutScroll();
     }));
     editableElement.addEventListener('blur', createDomEventListener((DomEvent event) {
       SemanticsTextEditingStrategy._instance?.deactivate(this);
@@ -283,7 +283,7 @@ class TextField extends PrimaryRoleManager {
     if (semanticsObject.hasFocus) {
       if (domDocument.activeElement != editableElement && semanticsObject.isEnabled) {
         semanticsObject.owner.addOneTimePostUpdateCallback(() {
-          editableElement.focus(preventScroll: true);
+          editableElement.focusWithoutScroll();
         });
       }
       SemanticsTextEditingStrategy._instance?.activate(this);

lib/web_ui/lib/src/engine/text_editing/text_editing.dart

@@ -1131,7 +1131,7 @@ class GloballyPositionedTextEditingStrategy extends DefaultTextEditingStrategy {
       // only after placing it to the correct position. Hence autofill menu
       // does not appear on top-left of the page.
       // Refocus on the elements after applying the geometry.
-      focusedFormElement!.focus(preventScroll: true);
+      focusedFormElement!.focusWithoutScroll();
       moveFocusToActiveDomElement();
     }
   }
@@ -1158,7 +1158,7 @@ class SafariDesktopTextEditingStrategy extends DefaultTextEditingStrategy {
   ///
   /// This method is similar to the [GloballyPositionedTextEditingStrategy].
   /// The only part different: this method does not call `super.placeElement()`,
-  /// which in current state calls `domElement.focus(preventScroll: true)`.
+  /// which in current state calls `domElement.focusWithoutScroll()`.
   ///
   /// Making an extra `focus` request causes flickering in Safari.
   @override
@@ -1571,7 +1571,7 @@ abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements
 
   /// Moves the focus to the [activeDomElement].
   void moveFocusToActiveDomElement() {
-    activeDomElement.focus(preventScroll: true);
+    activeDomElement.focusWithoutScroll();
   }
 
   /// Move the focus to the given [EngineFlutterView] in the next timer event.
@@ -1589,7 +1589,7 @@ abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements
       // move the focus, as it is likely that another widget already took the
       // focus.
       if (element == domDocument.activeElement) {
-        view?.dom.rootElement.focus(preventScroll: true);
+        view?.dom.rootElement.focusWithoutScroll();
       }
       if (removeElement) {
         element.remove();

lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart

@@ -41,7 +41,7 @@ void testMain() {
     test('The view is focusable but not reachable by keyboard when focused', () async {
       final EngineFlutterView view = createAndRegisterView(dispatcher);
 
-      view.dom.rootElement.focus();
+      view.dom.rootElement.focusWithoutScroll();
 
       // The root element should have a tabindex="-1" to make the flutter view
       // focusable but not reachable by the keyboard.
@@ -55,11 +55,11 @@ void testMain() {
       expect(view1.dom.rootElement.getAttribute('tabindex'), '0');
       expect(view2.dom.rootElement.getAttribute('tabindex'), '0');
 
-      view1.dom.rootElement.focus();
+      view1.dom.rootElement.focusWithoutScroll();
       expect(view1.dom.rootElement.getAttribute('tabindex'), '-1');
       expect(view2.dom.rootElement.getAttribute('tabindex'), '0');
 
-      view2.dom.rootElement.focus();
+      view2.dom.rootElement.focusWithoutScroll();
       expect(view1.dom.rootElement.getAttribute('tabindex'), '0');
       expect(view2.dom.rootElement.getAttribute('tabindex'), '-1');
 
@@ -77,11 +77,11 @@ void testMain() {
       expect(view1.dom.rootElement.getAttribute('tabindex'), isNull);
       expect(view2.dom.rootElement.getAttribute('tabindex'), isNull);
 
-      view1.dom.rootElement.focus();
+      view1.dom.rootElement.focusWithoutScroll();
       expect(view1.dom.rootElement.getAttribute('tabindex'), isNull);
       expect(view2.dom.rootElement.getAttribute('tabindex'), isNull);
 
-      view2.dom.rootElement.focus();
+      view2.dom.rootElement.focusWithoutScroll();
       expect(view1.dom.rootElement.getAttribute('tabindex'), isNull);
       expect(view2.dom.rootElement.getAttribute('tabindex'), isNull);
 
@@ -93,7 +93,7 @@ void testMain() {
     test('fires a focus event - a view was focused', () async {
       final EngineFlutterView view = createAndRegisterView(dispatcher);
 
-      view.dom.rootElement.focus();
+      view.dom.rootElement.focusWithoutScroll();
 
       expect(dispatchedViewFocusEvents, hasLength(1));
 
@@ -105,7 +105,7 @@ void testMain() {
     test('fires a focus event - a view was unfocused', () async {
       final EngineFlutterView view = createAndRegisterView(dispatcher);
 
-      view.dom.rootElement.focus();
+      view.dom.rootElement.focusWithoutScroll();
       view.dom.rootElement.blur();
 
       expect(dispatchedViewFocusEvents, hasLength(2));
@@ -123,12 +123,12 @@ void testMain() {
       final EngineFlutterView view1 = createAndRegisterView(dispatcher);
       final EngineFlutterView view2 = createAndRegisterView(dispatcher);
 
-      view1.dom.rootElement.focus();
-      view2.dom.rootElement.focus();
+      view1.dom.rootElement.focusWithoutScroll();
+      view2.dom.rootElement.focusWithoutScroll();
       // The statements simulate the user pressing shift + tab in the keyboard.
       // Synthetic keyboard events do not trigger focus changes.
       domDocument.body!.pressTabKey(shift: true);
-      view1.dom.rootElement.focus();
+      view1.dom.rootElement.focusWithoutScroll();
       domDocument.body!.releaseTabKey();
 
       expect(dispatchedViewFocusEvents, hasLength(3));
@@ -150,8 +150,8 @@ void testMain() {
       final EngineFlutterView view1 = createAndRegisterView(dispatcher);
       final EngineFlutterView view2 = createAndRegisterView(dispatcher);
 
-      view1.dom.rootElement.focus();
-      view2.dom.rootElement.focus();
+      view1.dom.rootElement.focusWithoutScroll();
+      view2.dom.rootElement.focusWithoutScroll();
       view2.dom.rootElement.blur();
 
       expect(dispatchedViewFocusEvents, hasLength(3));
@@ -254,7 +254,7 @@ void testMain() {
       final EngineFlutterView view = createAndRegisterView(dispatcher);
 
       view.dom.rootElement.append(input);
-      input.focus();
+      input.focusWithoutScroll();
 
       dispatcher.requestViewFocusChange(
         viewId: view.viewId,