[CP-stable][web:semantics] fix double click due to long-press

lib/web_ui/lib/src/engine/pointer_binding.dart

@@ -205,12 +205,13 @@ class ClickDebouncer {
   @visibleForTesting
   DebounceState? get debugState => _state;
 
-  // The timestamp of the last "pointerup" DOM event that was flushed.
+  // The timestamp of the last "pointerup" DOM event that was sent to the
+  // framework.
   //
   // Not to be confused with the time when it was flushed. The two may be far
   // apart because the flushing can happen after a delay due to timer, or events
   // that happen after the said "pointerup".
-  Duration? _lastFlushedPointerUpTimeStamp;
+  Duration? _lastSentPointerUpTimeStamp;
 
   /// Returns true if the debouncer has a non-empty queue of pointer events that
   /// were withheld from the framework.
@@ -244,6 +245,14 @@ class ClickDebouncer {
     } else if (event.type == 'pointerdown') {
       _startDebouncing(event, data);
     } else {
+      if (event.type == 'pointerup') {
+        // Record the last pointerup event even if not debouncing. This is
+        // because the sequence of pointerdown-pointerup could indicate a
+        // long-press, and the debounce timer is not long enough to capture it.
+        // If a "click" is observed after a long-press it should be
+        // discarded.
+        _lastSentPointerUpTimeStamp = _BaseAdapter._eventTimeStampToDuration(event.timeStamp!);
+      }
       _sendToFramework(event, data);
     }
   }
@@ -295,6 +304,7 @@ class ClickDebouncer {
 
     EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
         semanticsNodeId, ui.SemanticsAction.tap, null);
+    reset();
   }
 
   void _startDebouncing(DomEvent event, List<ui.PointerData> data) {
@@ -351,10 +361,7 @@ class ClickDebouncer {
     // It's only interesting to debounce clicks when both `pointerdown` and
     // `pointerup` land on the same element.
     if (event.type == 'pointerup') {
-      // TODO(yjbanov): this is a bit mouthful, but see https://github.com/dart-lang/sdk/issues/53070
-      final DomEventTarget? eventTarget = event.target;
-      final DomElement stateTarget = state.target;
-      final bool targetChanged = eventTarget != stateTarget;
+      final bool targetChanged = event.target != state.target;
       if (targetChanged) {
         _flush();
       }
@@ -372,15 +379,15 @@ class ClickDebouncer {
   // already flushed to the framework, the click event is dropped to avoid
   // double click.
   bool _shouldSendClickEventToFramework(DomEvent click) {
-    final Duration? lastFlushedPointerUpTimeStamp = _lastFlushedPointerUpTimeStamp;
+    final Duration? lastSentPointerUpTimeStamp = _lastSentPointerUpTimeStamp;
 
-    if (lastFlushedPointerUpTimeStamp == null) {
+    if (lastSentPointerUpTimeStamp == null) {
       // We haven't seen a pointerup. It's standalone click event. Let it through.
       return true;
     }
 
     final Duration clickTimeStamp = _BaseAdapter._eventTimeStampToDuration(click.timeStamp!);
-    final Duration delta = clickTimeStamp - lastFlushedPointerUpTimeStamp;
+    final Duration delta = clickTimeStamp - lastSentPointerUpTimeStamp;
     return delta >= const Duration(milliseconds: 50);
   }
 
@@ -393,7 +400,7 @@ class ClickDebouncer {
     final List<ui.PointerData> aggregateData = <ui.PointerData>[];
     for (final QueuedEvent queuedEvent in state.queue) {
       if (queuedEvent.event.type == 'pointerup') {
-        _lastFlushedPointerUpTimeStamp = queuedEvent.timeStamp;
+        _lastSentPointerUpTimeStamp = queuedEvent.timeStamp;
       }
       aggregateData.addAll(queuedEvent.data);
     }
@@ -419,7 +426,7 @@ class ClickDebouncer {
   void reset() {
     _state?.timer.cancel();
     _state = null;
-    _lastFlushedPointerUpTimeStamp = null;
+    _lastSentPointerUpTimeStamp = null;
   }
 }
 

lib/web_ui/test/engine/pointer_binding_test.dart

@@ -4,6 +4,7 @@
 
 import 'dart:js_util' as js_util;
 
+import 'package:meta/meta.dart';
 import 'package:test/bootstrap/browser.dart';
 import 'package:test/test.dart';
 import 'package:ui/src/engine.dart';
@@ -2757,6 +2758,7 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
     String description,
     Future<void> Function() body, {
     Object? skip,
+    @doNotSubmit bool solo = false,
   }) {
     test(
       description,
@@ -2768,6 +2770,7 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
         EngineSemantics.instance.semanticsEnabled = false;
       },
       skip: skip,
+      solo: solo, // ignore: invalid_use_of_do_not_submit_member
     );
   }
 
@@ -3053,6 +3056,67 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
     // TODO(yjbanov): https://github.com/flutter/flutter/issues/142991.
   }, skip: ui_web.browser.operatingSystem == ui_web.OperatingSystem.windows);
 
+  // Regression test for https://github.com/flutter/flutter/issues/147050
+  //
+  // This test emulates a long-press. Start with a "pointerdown" followed by no
+  // activity long enough that the debounce timer expires and the state of the
+  // ClickDebouncer is reset back to idle. Then a "pointerup" arrives seemingly
+  // standalone. Since no gesture is being debounced, the debouncer simply
+  // forwards it to the framework. However, "pointerup" will be immediately
+  // followed by a "click". Since we sent the "pointerdown" and "pointerup" to
+  // the framework already, the framework registered a tap. Forwarding the
+  // "click" would lead to a double-tap. This was the bug.
+  testWithSemantics('Dedupes click if pointer up happened recently without debouncing', () async {
+    expect(EnginePlatformDispatcher.instance.semanticsEnabled, true);
+    expect(PointerBinding.clickDebouncer.isDebouncing, false);
+
+    final DomElement testElement = createDomElement('flt-semantics');
+    testElement.setAttribute('flt-tappable', '');
+    view.dom.semanticsHost.appendChild(testElement);
+
+    // Begin a long-press with a "pointerdown".
+    testElement.dispatchEvent(context.primaryDown());
+
+    // Expire the timer causing the debouncer to reset itself.
+    await Future<void>.delayed(const Duration(milliseconds: 250));
+    expect(
+      reason: '"pointerdown" should be flushed when the timer expires.',
+      pointerPackets,
+      <ui.PointerChange>[ui.PointerChange.add, ui.PointerChange.down],
+    );
+    pointerPackets.clear();
+
+    // Send a "pointerup" while the debouncer is not debouncing anything.
+    testElement.dispatchEvent(context.primaryUp());
+
+    // A standalone "pointerup" should not start debouncing anything.
+    expect(PointerBinding.clickDebouncer.isDebouncing, isFalse);
+    expect(
+      reason: 'The "pointerup" should be forwarded to the framework immediately',
+      pointerPackets,
+      <ui.PointerChange>[ui.PointerChange.up],
+    );
+
+    // Use a delay that's short enough for the click to be deduped.
+    await Future<void>.delayed(const Duration(milliseconds: 10));
+
+    final DomEvent click = createDomMouseEvent(
+      'click',
+      <Object?, Object?>{
+        'clientX': testElement.getBoundingClientRect().x,
+        'clientY': testElement.getBoundingClientRect().y,
+      }
+    );
+    PointerBinding.clickDebouncer.onClick(click, 42, true);
+
+    expect(
+      reason: 'Because the DOM click event was deduped.',
+      semanticsActions,
+      isEmpty,
+    );
+    // TODO(yjbanov): https://github.com/flutter/flutter/issues/142991.
+  }, skip: ui_web.browser.operatingSystem == ui_web.OperatingSystem.windows);
+
   testWithSemantics('Forwards click if enough time passed after the last flushed pointerup', () async {
     expect(EnginePlatformDispatcher.instance.semanticsEnabled, true);
     expect(PointerBinding.clickDebouncer.isDebouncing, false);