[url_launcher][web] Prevent browser from navigating when followLink isn't called (flutter#8675)

When a DOM click event is received, we need to make a decision before the end of the event loop whether to allow the browser to navigate or not. At the end of the event loop, the browser will perform the navigation if we don't prevent it.

The problem occurs when the `followLink` signal arrives AFTER the event loop (this can happen in semantics mode when the web engine uses a debouncer to queue events and send them to the framework after some delay). This leads to a situation where we can't make a definitive decision by the end of the event loop, so this PR does the following:

1. [best case] If the `followLink` signal is received before the end of the event loop, we let the browser do the navigation for a full web link experience.

2. [meh case] If no `followLink` signal is received before the end of the event loop, we PREVENT the browser from navigating. But we keep waiting for the `followLink` signal. If the signal arrives a bit later, we fallback to a programmatic navigation through the `launchUrl` API.

Fixes flutter#162927
Fixes flutter#162408

Author: mdebbar

packages/url_launcher/url_launcher_web/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 2.4.1
+
+* Fixes a bug that triggers Links when they are not supposed to.
+
 ## 2.4.0
 
 * Enhances handling of out-of-order events.

packages/url_launcher/url_launcher_web/example/integration_test/link_widget_test.dart

@@ -445,25 +445,23 @@ void main() {
       await followLinkCallback!();
       await Future<void>.delayed(const Duration(seconds: 1));
       final html.Event event1 = _simulateClick(anchor);
+      await Future<void>.delayed(const Duration(seconds: 1));
 
       // The link shouldn't have been triggered.
       expect(pushedRouteNames, isEmpty);
       expect(testPlugin.launches, isEmpty);
-      expect(event1.defaultPrevented, isFalse);
-
-      await Future<void>.delayed(const Duration(seconds: 1));
+      expect(event1.defaultPrevented, isTrue);
 
       // Signals with large delay (in reverse order).
       final html.Event event2 = _simulateClick(anchor);
       await Future<void>.delayed(const Duration(seconds: 1));
       await followLinkCallback!();
+      await Future<void>.delayed(const Duration(seconds: 1));
 
       // The link shouldn't have been triggered.
       expect(pushedRouteNames, isEmpty);
       expect(testPlugin.launches, isEmpty);
-      expect(event2.defaultPrevented, isFalse);
-
-      await Future<void>.delayed(const Duration(seconds: 1));
+      expect(event2.defaultPrevented, isTrue);
 
       // A small delay is okay.
       await followLinkCallback!();
@@ -473,6 +471,7 @@ void main() {
       // The link should've been triggered now.
       expect(pushedRouteNames, <String>['/foobar']);
       expect(testPlugin.launches, isEmpty);
+      // (default is prevented here because the link is internal)
       expect(event3.defaultPrevented, isTrue);
     });
 
@@ -972,6 +971,109 @@ void main() {
       expect(event.defaultPrevented, isTrue);
     });
 
+    testWidgets('handles debounced clicks on semantic link',
+        (WidgetTester tester) async {
+      final Uri uri = Uri.parse('https://flutter.dev');
+      FollowLink? followLinkCallback;
+
+      await tester.pumpWidget(MaterialApp(
+        home: WebLinkDelegate(
+          semanticsIdentifier: 'test-link-71',
+          TestLinkInfo(
+            uri: uri,
+            target: LinkTarget.blank,
+            builder: (BuildContext context, FollowLink? followLink) {
+              followLinkCallback = followLink;
+              return GestureDetector(
+                onTap: () {},
+                child: const Text('My Link'),
+              );
+            },
+          ),
+        ),
+      ));
+      // Platform view creation happens asynchronously.
+      await tester.pumpAndSettle();
+      await tester.pump();
+
+      final html.Element semanticsHost =
+          html.document.createElement('flt-semantics-host');
+      html.document.body!.append(semanticsHost);
+      final html.Element semanticsAnchor = html.document.createElement('a')
+        ..setAttribute('id', 'flt-semantic-node-99')
+        ..setAttribute('flt-semantics-identifier', 'test-link-71')
+        ..setAttribute('href', uri.toString())
+        ..textContent = 'My Text Link';
+      semanticsHost.append(semanticsAnchor);
+
+      expect(pushedRouteNames, isEmpty);
+      expect(testPlugin.launches, isEmpty);
+
+      // Receive the DOM click first.
+      final html.Event event = _simulateClick(semanticsAnchor);
+      // The browser will be prevented from navigating right at the end of the event loop because
+      // we are still not sure if the `followLink` will arrive or not.
+      await Future<void>.delayed(Duration.zero);
+      expect(event.defaultPrevented, isTrue);
+      // After some delay, the engine's click debouncer sends the events to the framework and we get
+      // the `followLink` signal.
+      await Future<void>.delayed(const Duration(milliseconds: 200));
+      await followLinkCallback!();
+
+      expect(pushedRouteNames, isEmpty);
+      expect(testPlugin.launches, <String>['https://flutter.dev']);
+    });
+
+    // Regression test for: https://github.com/flutter/flutter/issues/162927
+    testWidgets('prevents navigation with debounced clicks on semantic link',
+        (WidgetTester tester) async {
+      final Uri uri = Uri.parse('https://flutter.dev');
+
+      await tester.pumpWidget(MaterialApp(
+        home: WebLinkDelegate(
+          semanticsIdentifier: 'test-link-71',
+          TestLinkInfo(
+            uri: uri,
+            target: LinkTarget.blank,
+            builder: (BuildContext context, FollowLink? followLink) {
+              return GestureDetector(
+                onTap: () {},
+                child: const Text('My Link'),
+              );
+            },
+          ),
+        ),
+      ));
+      // Platform view creation happens asynchronously.
+      await tester.pumpAndSettle();
+      await tester.pump();
+
+      final html.Element semanticsHost =
+          html.document.createElement('flt-semantics-host');
+      html.document.body!.append(semanticsHost);
+      final html.Element semanticsAnchor = html.document.createElement('a')
+        ..setAttribute('id', 'flt-semantic-node-99')
+        ..setAttribute('flt-semantics-identifier', 'test-link-71')
+        ..setAttribute('href', uri.toString())
+        ..textContent = 'My Text Link';
+      semanticsHost.append(semanticsAnchor);
+
+      expect(pushedRouteNames, isEmpty);
+      expect(testPlugin.launches, isEmpty);
+
+      // Receive the DOM click first.
+      final html.Event event = _simulateClick(semanticsAnchor);
+      // The browser will be prevented from navigating right at the end of the event loop because
+      // we are still not sure if the `followLink` will arrive or not.
+      await Future<void>.delayed(Duration.zero);
+      expect(event.defaultPrevented, isTrue);
+
+      // No navigation should occur because no `followLink` signal was received.
+      await Future<void>.delayed(const Duration(seconds: 1));
+      expect(pushedRouteNames, isEmpty);
+      expect(testPlugin.launches, isEmpty);
+    });
+
     // TODO(mdebbar): Remove this test after the engine PR [1] makes it to stable.
     //                [1] https://github.com/flutter/engine/pull/52720
     testWidgets('handles clicks on (old) semantic link with a button',

packages/url_launcher/url_launcher_web/lib/src/link.dart

@@ -199,6 +199,16 @@ class LinkTriggerSignals {
   /// considered valid. If they don't, the signals are reset.
   final Duration staleTimeout;
 
+  /// Whether the browser can do the navigation itself.
+  ///
+  /// If the link is activated by a click event, then we know the browser can do the navigation by
+  /// itself.
+  ///
+  /// In some cases, we decide to prevent default on the click event, so [canBrowserNavigate] is
+  /// set to false to indicate that the browser is not able to perform the navigation anymore. In
+  /// such case, the navigation has to be performed programmatically (using `launchUrl`).
+  bool canBrowserNavigate = false;
+
   bool get _hasAllSignals => _hasFollowLink && _hasDomEvent;
 
   int? get _viewId {
@@ -232,7 +242,7 @@ class LinkTriggerSignals {
   void onFollowLink({required int viewId}) {
     _hasFollowLink = true;
     _viewIdFromFollowLink = viewId;
-    _didUpdate();
+    _scheduleReset();
   }
 
   /// Handles a [mouseEvent] signal from a specific [viewId].
@@ -254,7 +264,26 @@ class LinkTriggerSignals {
     _hasDomEvent = true;
     _viewIdFromDomEvent = viewId;
     _mouseEvent = mouseEvent;
-    _didUpdate();
+    _scheduleReset();
+
+    if (mouseEvent != null) {
+      canBrowserNavigate = true;
+      // We have to make a decision whether to allow the browser to navigate or not. The decision
+      // has to be made before the end of the event loop.
+      //
+      // If we don't hear back from the `followLink` signal by the end of the event loop, we should
+      // prevent the browser from navigating. If the `followLink` signal arrives later, we can
+      // navigate programmatically using `launchUrl`.
+      scheduleMicrotask(() {
+        if (!_hasDomEvent) {
+          // By the time the microtask runs, the link may have already been triggered. In that case,
+          // we want to do nothing here.
+          return;
+        }
+        canBrowserNavigate = false;
+        mouseEvent.preventDefault();
+      });
+    }
   }
 
   bool _hasFollowLink = false;
@@ -279,7 +308,7 @@ class LinkTriggerSignals {
 
   Timer? _resetTimer;
 
-  void _didUpdate() {
+  void _scheduleReset() {
     _resetTimer?.cancel();
     _resetTimer = Timer(staleTimeout, reset);
   }
@@ -296,6 +325,7 @@ class LinkTriggerSignals {
     _viewIdFromDomEvent = null;
 
     _mouseEvent = null;
+    canBrowserNavigate = false;
   }
 }
 
@@ -514,23 +544,23 @@ class LinkViewController extends PlatformViewController {
   static void _triggerLink(int viewId, html.MouseEvent? mouseEvent) {
     final LinkViewController controller = _instancesByViewId[viewId]!;
 
-    if (mouseEvent != null && _isModifierKey(mouseEvent)) {
+    if (_triggerSignals.canBrowserNavigate && _isModifierKey(mouseEvent)) {
       // When the click is accompanied by a modifier key (e.g. cmd+click or
       // shift+click), we want to let the browser do its thing (e.g. open a new
       // tab or a new window).
       return;
     }
 
     if (controller._isExternalLink) {
-      if (mouseEvent == null) {
-        // When external links are triggered by keyboard, they are not handled by
-        // the browser. So we have to launch the url manually.
-        UrlLauncherPlatform.instance
-            .launchUrl(controller._uri.toString(), const LaunchOptions());
+      if (!_triggerSignals.canBrowserNavigate) {
+        // When the browser can't do the navigation, we have to launch the url manually.
+        UrlLauncherPlatform.instance.launchUrl(
+          controller._uri.toString(),
+          const LaunchOptions(),
+        );
       }
 
-      // When triggerd by a mouse event, external links will be handled by the
-      // browser, so we don't have to do anything.
+      // Otherwise, let the browser handle it, so we don't have to do anything.
       return;
     }
 
@@ -675,7 +705,11 @@ html.Element? _getClosestSemanticsLink(html.Element element) {
   return element.closest('a[id^="flt-semantic-node-"]');
 }
 
-bool _isModifierKey(html.Event event) {
+bool _isModifierKey(html.Event? event) {
+  if (event == null) {
+    return false;
+  }
+
   // This method accepts both KeyboardEvent and MouseEvent but there's no common
   // interface that contains the `ctrlKey`, `altKey`, `metaKey`, and `shiftKey`
   // properties. So we have to cast the event to either `KeyboardEvent` or

packages/url_launcher/url_launcher_web/pubspec.yaml

@@ -2,7 +2,7 @@ name: url_launcher_web
 description: Web platform implementation of url_launcher
 repository: https://github.com/flutter/packages/tree/main/packages/url_launcher/url_launcher_web
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+url_launcher%22
-version: 2.4.0
+version: 2.4.1
 
 environment:
   sdk: ^3.6.0