diff --git a/packages/url_launcher/url_launcher_web/CHANGELOG.md b/packages/url_launcher/url_launcher_web/CHANGELOG.md index f923d118fca..115414ab252 100644 --- a/packages/url_launcher/url_launcher_web/CHANGELOG.md +++ b/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. diff --git a/packages/url_launcher/url_launcher_web/example/integration_test/link_widget_test.dart b/packages/url_launcher/url_launcher_web/example/integration_test/link_widget_test.dart index b8c11fa32da..678848b15a4 100644 --- a/packages/url_launcher/url_launcher_web/example/integration_test/link_widget_test.dart +++ b/packages/url_launcher/url_launcher_web/example/integration_test/link_widget_test.dart @@ -445,25 +445,23 @@ void main() { await followLinkCallback!(); await Future.delayed(const Duration(seconds: 1)); final html.Event event1 = _simulateClick(anchor); + await Future.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.delayed(const Duration(seconds: 1)); + expect(event1.defaultPrevented, isTrue); // Signals with large delay (in reverse order). final html.Event event2 = _simulateClick(anchor); await Future.delayed(const Duration(seconds: 1)); await followLinkCallback!(); + await Future.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.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, ['/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.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.delayed(const Duration(milliseconds: 200)); + await followLinkCallback!(); + + expect(pushedRouteNames, isEmpty); + expect(testPlugin.launches, ['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.delayed(Duration.zero); + expect(event.defaultPrevented, isTrue); + + // No navigation should occur because no `followLink` signal was received. + await Future.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', diff --git a/packages/url_launcher/url_launcher_web/lib/src/link.dart b/packages/url_launcher/url_launcher_web/lib/src/link.dart index 3540ac97f51..c24d47876ec 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/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,7 +544,7 @@ 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). @@ -522,15 +552,15 @@ class LinkViewController extends PlatformViewController { } 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 diff --git a/packages/url_launcher/url_launcher_web/pubspec.yaml b/packages/url_launcher/url_launcher_web/pubspec.yaml index ee13f34f752..0335a62f5ab 100644 --- a/packages/url_launcher/url_launcher_web/pubspec.yaml +++ b/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