[url_launcher][web] Prevent browser from navigating when followLink isn't called

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