From 1b37c5441c037f1fe438790d29625c84628c68c8 Mon Sep 17 00:00:00 2001 From: David Iglesias Date: Fri, 28 Jun 2024 20:47:25 -0700 Subject: [PATCH] [web] Fixes drag scrolling in embedded mode. (flutter/engine#53647) When Flutter web runs embedded in a page, scrolling by dragging is interrupted very early by the browser. It turns out that our `pointer` events receive a `pointercancel` + `pointerleave` by the browser because they happen in an area (the flutter glasspane) that is not really scrollable. [See documentation](https://developer.mozilla.org/en-US/docs/Web/API/Element/pointercancel_event). > [!NOTE] > After the pointercancel event is fired, the browser will also send [pointerout](https://developer.mozilla.org/en-US/docs/Web/API/Element/pointerout_event) followed by [pointerleave](https://developer.mozilla.org/en-US/docs/Web/API/Element/pointerleave_event). (Firefox is a good browser to test this, because the browser will cancel our events **only if there's scrollable areas around the embedded flutter app**.) There's several solutions, but one of them (used by PixiJS as well) is to cancel the `touchstart` event that fires with the `pointerdown` event. (This PR also removes an unnecessary call to `addEventListener` in the `Listener` helper class, and adds some testing to it). ## Testing * Added a happy case test for the fix (preventDefault on 'touchstart' events) * Deployed demo app here: https://dit-multiview-scroll.web.app ## Issues * Fixes https://github.com/flutter/flutter/issues/138985 * Fixes https://github.com/flutter/flutter/issues/146732 * Related to https://github.com/flutter/flutter/issues/139263 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- .../lib/web_ui/lib/src/engine/dom.dart | 5 ++ .../lib/src/engine/pointer_binding.dart | 36 +++++++--- .../test/engine/pointer_binding_test.dart | 66 +++++++++++++++++++ 3 files changed, 97 insertions(+), 10 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart index 42f6cfa1a2..32d100df8f 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart @@ -431,6 +431,9 @@ extension DomEventExtension on DomEvent { external JSString get _type; String get type => _type.toDart; + external JSBoolean? get _cancelable; + bool get cancelable => _cancelable?.toDart ?? true; + external JSVoid preventDefault(); external JSVoid stopPropagation(); @@ -729,6 +732,8 @@ extension DomElementExtension on DomElement { removeChild(firstChild!); } } + + external void setPointerCapture(num? pointerId); } @JS() diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/pointer_binding.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/pointer_binding.dart index 741761c434..b914a37125 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/pointer_binding.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/pointer_binding.dart @@ -432,8 +432,10 @@ class PointerSupportDetector { String toString() => 'pointers:$hasPointerEvents'; } -class _Listener { - _Listener._({ +/// Encapsulates a DomEvent registration so it can be easily unregistered later. +@visibleForTesting +class Listener { + Listener._({ required this.event, required this.target, required this.handler, @@ -448,7 +450,7 @@ class _Listener { /// associated with this event. If `passive` is false, the browser will wait /// for the handler to finish execution before performing the respective /// action. - factory _Listener.register({ + factory Listener.register({ required String event, required DomEventTarget target, required DartDomEventListener handler, @@ -465,12 +467,12 @@ class _Listener { target.addEventListenerWithOptions(event, jsHandler, eventOptions); } - final _Listener listener = _Listener._( + final Listener listener = Listener._( event: event, target: target, handler: jsHandler, ); - target.addEventListener(event, jsHandler); + return listener; } @@ -496,12 +498,12 @@ abstract class _BaseAdapter { PointerDataConverter get _pointerDataConverter => _owner._pointerDataConverter; KeyboardConverter? get _keyboardConverter => _owner._keyboardConverter; - final List<_Listener> _listeners = <_Listener>[]; + final List _listeners = []; DomWheelEvent? _lastWheelEvent; bool _lastWheelEventWasTrackpad = false; bool _lastWheelEventAllowedDefault = false; - DomEventTarget get _viewTarget => _view.dom.rootElement; + DomElement get _viewTarget => _view.dom.rootElement; DomEventTarget get _globalTarget => _view.embeddingStrategy.globalEventTarget; /// Each subclass is expected to override this method to attach its own event @@ -510,7 +512,7 @@ abstract class _BaseAdapter { /// Cleans up all event listeners attached by this adapter. void dispose() { - for (final _Listener listener in _listeners) { + for (final Listener listener in _listeners) { listener.unregister(); } _listeners.clear(); @@ -546,7 +548,7 @@ abstract class _BaseAdapter { handler(event); } } - _listeners.add(_Listener.register( + _listeners.add(Listener.register( event: eventName, target: target, handler: loggedHandler, @@ -719,7 +721,7 @@ mixin _WheelEventListenerMixin on _BaseAdapter { } void _addWheelEventListener(DartDomEventListener handler) { - _listeners.add(_Listener.register( + _listeners.add(Listener.register( event: 'wheel', target: _viewTarget, handler: handler, @@ -966,6 +968,20 @@ class _PointerAdapter extends _BaseAdapter with _WheelEventListenerMixin { @override void setup() { + // Prevents the browser auto-canceling pointer events. + // Register into `_listener` directly so the event isn't forwarded to semantics. + _listeners.add(Listener.register( + event: 'touchstart', + target: _viewTarget, + handler: (DomEvent event) { + // This is one of the ways I've seen this done. PixiJS does a similar thing. + // ThreeJS seems to subscribe move/leave in the pointerdown handler instead? + if (event.cancelable) { + event.preventDefault(); + } + }, + )); + _addPointerEventListener(_viewTarget, 'pointerdown', (DomPointerEvent event) { final int device = _getPointerId(event); final List pointerData = []; diff --git a/engine/src/flutter/lib/web_ui/test/engine/pointer_binding_test.dart b/engine/src/flutter/lib/web_ui/test/engine/pointer_binding_test.dart index 04cbaeba92..e8d90c4658 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/pointer_binding_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/pointer_binding_test.dart @@ -305,6 +305,18 @@ void testMain() { }, ); + test('prevents default on touchstart events', () async { + final event = createDomEvent('Event', 'touchstart'); + + rootElement.dispatchEvent(event); + + expect( + event.defaultPrevented, + isTrue, + reason: 'touchstart events should be prevented so pointer events are not cancelled later.', + ); + }); + test( 'can receive pointer events on the app root', () { @@ -2670,6 +2682,60 @@ void testMain() { ); }); + group('Listener', () { + late DomElement eventTarget; + late DomEvent expected; + late bool handled; + + setUp(() { + eventTarget = createDomElement('div'); + expected = createDomEvent('Event', 'custom-event'); + handled = false; + }); + + test('listeners can be registered', () { + Listener.register( + event: 'custom-event', + target: eventTarget, + handler: (event) { + expect(event, expected); + handled = true; + }, + ); + + // Trigger the event... + eventTarget.dispatchEvent(expected); + expect(handled, isTrue); + }); + + test('listeners can be unregistered', () { + final Listener listener = Listener.register( + event: 'custom-event', + target: eventTarget, + handler: (event) { + handled = true; + }, + ); + listener.unregister(); + + eventTarget.dispatchEvent(expected); + expect(handled, isFalse); + }); + + test('listeners are registered only once', () { + int timesHandled = 0; + Listener.register( + event: 'custom-event', + target: eventTarget, + handler: (event) { + timesHandled++; + }, + ); + eventTarget.dispatchEvent(expected); + expect(timesHandled, 1, reason: 'The handler ran multiple times for a single event.'); + }); + }); + group('ClickDebouncer', () { _testClickDebouncer(getBinding: () => instance); });