diff --git a/packages/url_launcher/url_launcher_web/CHANGELOG.md b/packages/url_launcher/url_launcher_web/CHANGELOG.md index 5e3d6d42b35e..ca4d9d86cc66 100644 --- a/packages/url_launcher/url_launcher_web/CHANGELOG.md +++ b/packages/url_launcher/url_launcher_web/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.3.1 + +* Implements correct handling of keyboard events with Link. + ## 2.3.0 * Updates web code to package `web: ^0.5.0`. @@ -24,7 +28,7 @@ ## 2.1.0 * Adds `launchUrl` implementation. -* Prevents _Tabnabbing_ and disallows `javascript:` URLs on `launch` and `launchUrl`. +* Prevents _Tabnabbing_ and disallows `javascript:` URLs on `launch` and `launchUrl`. ## 2.0.20 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 9cd76d03c80c..9c700c87d0eb 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 @@ -6,11 +6,13 @@ import 'dart:js_interop'; import 'dart:js_interop_unsafe'; import 'dart:ui_web' as ui_web; -import 'package:flutter/widgets.dart'; +import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:integration_test/integration_test.dart'; import 'package:url_launcher_platform_interface/link.dart'; +import 'package:url_launcher_platform_interface/url_launcher_platform_interface.dart'; import 'package:url_launcher_web/src/link.dart'; +import 'package:url_launcher_web/url_launcher_web.dart'; import 'package:web/web.dart' as html; void main() { @@ -171,6 +173,196 @@ void main() { await tester.pumpAndSettle(); }); }); + + group('Follows links', () { + late TestUrlLauncherPlugin testPlugin; + late UrlLauncherPlatform originalPlugin; + + setUp(() { + originalPlugin = UrlLauncherPlatform.instance; + testPlugin = TestUrlLauncherPlugin(); + UrlLauncherPlatform.instance = testPlugin; + }); + + tearDown(() { + UrlLauncherPlatform.instance = originalPlugin; + }); + + testWidgets('click to navigate to internal link', + (WidgetTester tester) async { + final TestNavigatorObserver observer = TestNavigatorObserver(); + final Uri uri = Uri.parse('/foobar'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + navigatorObservers: [observer], + routes: { + '/foobar': (BuildContext context) => const Text('Internal route'), + }, + home: Directionality( + textDirection: TextDirection.ltr, + child: WebLinkDelegate(TestLinkInfo( + uri: uri, + target: LinkTarget.blank, + builder: (BuildContext context, FollowLink? followLink) { + followLinkCallback = followLink; + return const SizedBox(width: 100, height: 100); + }, + )), + ), + )); + // Platform view creation happens asynchronously. + await tester.pumpAndSettle(); + + expect(observer.currentRouteName, '/'); + expect(testPlugin.launches, isEmpty); + + final html.Element anchor = _findSingleAnchor(); + + await followLinkCallback!(); + _simulateClick(anchor); + await tester.pumpAndSettle(); + + // Internal links should navigate the app to the specified route. There + // should be no calls to `launchUrl`. + expect(observer.currentRouteName, '/foobar'); + expect(testPlugin.launches, isEmpty); + + // Needed when testing on on Chrome98 headless in CI. + // See https://github.com/flutter/flutter/issues/121161 + await tester.pumpAndSettle(); + }); + + testWidgets('keydown to navigate to internal link', + (WidgetTester tester) async { + final TestNavigatorObserver observer = TestNavigatorObserver(); + final Uri uri = Uri.parse('/foobar'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + navigatorObservers: [observer], + routes: { + '/foobar': (BuildContext context) => const Text('Internal route'), + }, + home: Directionality( + textDirection: TextDirection.ltr, + child: WebLinkDelegate(TestLinkInfo( + uri: uri, + target: LinkTarget.blank, + builder: (BuildContext context, FollowLink? followLink) { + followLinkCallback = followLink; + return const SizedBox(width: 100, height: 100); + }, + )), + ), + )); + // Platform view creation happens asynchronously. + await tester.pumpAndSettle(); + + expect(observer.currentRouteName, '/'); + expect(testPlugin.launches, isEmpty); + + final html.Element anchor = _findSingleAnchor(); + + await followLinkCallback!(); + _simulateKeydown(anchor); + await tester.pumpAndSettle(); + + // Internal links should navigate the app to the specified route. There + // should be no calls to `launchUrl`. + expect(observer.currentRouteName, '/foobar'); + expect(testPlugin.launches, isEmpty); + + // Needed when testing on on Chrome98 headless in CI. + // See https://github.com/flutter/flutter/issues/121161 + await tester.pumpAndSettle(); + }); + + testWidgets('click to navigate to external link', + (WidgetTester tester) async { + final TestNavigatorObserver observer = TestNavigatorObserver(); + final Uri uri = Uri.parse('https://google.com'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + navigatorObservers: [observer], + home: Directionality( + textDirection: TextDirection.ltr, + child: WebLinkDelegate(TestLinkInfo( + uri: uri, + target: LinkTarget.blank, + builder: (BuildContext context, FollowLink? followLink) { + followLinkCallback = followLink; + return const SizedBox(width: 100, height: 100); + }, + )), + ), + )); + // Platform view creation happens asynchronously. + await tester.pumpAndSettle(); + + expect(observer.currentRouteName, '/'); + expect(testPlugin.launches, isEmpty); + + final html.Element anchor = _findSingleAnchor(); + + await followLinkCallback!(); + _simulateClick(anchor); + await tester.pumpAndSettle(); + + // External links that are triggered by a click are left to be handled by + // the browser, so there should be no change to the app's route name, and + // no calls to `launchUrl`. + expect(observer.currentRouteName, '/'); + expect(testPlugin.launches, isEmpty); + + // Needed when testing on on Chrome98 headless in CI. + // See https://github.com/flutter/flutter/issues/121161 + await tester.pumpAndSettle(); + }); + + testWidgets('keydown to navigate to external link', + (WidgetTester tester) async { + final TestNavigatorObserver observer = TestNavigatorObserver(); + final Uri uri = Uri.parse('https://google.com'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + navigatorObservers: [observer], + home: Directionality( + textDirection: TextDirection.ltr, + child: WebLinkDelegate(TestLinkInfo( + uri: uri, + target: LinkTarget.blank, + builder: (BuildContext context, FollowLink? followLink) { + followLinkCallback = followLink; + return const SizedBox(width: 100, height: 100); + }, + )), + ), + )); + // Platform view creation happens asynchronously. + await tester.pumpAndSettle(); + + expect(observer.currentRouteName, '/'); + expect(testPlugin.launches, isEmpty); + + final html.Element anchor = _findSingleAnchor(); + + await followLinkCallback!(); + _simulateKeydown(anchor); + await tester.pumpAndSettle(); + + // External links that are triggered by keyboard are handled by calling + // `launchUrl`, and there's no change to the app's route name. + expect(observer.currentRouteName, '/'); + expect(testPlugin.launches, ['https://google.com']); + + // Needed when testing on on Chrome98 headless in CI. + // See https://github.com/flutter/flutter/issues/121161 + await tester.pumpAndSettle(); + }); + }); } html.Element _findSingleAnchor() { @@ -199,6 +391,33 @@ html.Element _findSingleAnchor() { return foundAnchors.single; } +void _simulateClick(html.Element target) { + target.dispatchEvent( + html.MouseEvent( + 'click', + html.MouseEventInit()..bubbles = true, + ), + ); +} + +void _simulateKeydown(html.Element target) { + target.dispatchEvent( + html.KeyboardEvent( + 'keydown', + html.KeyboardEventInit()..bubbles = true, + ), + ); +} + +class TestNavigatorObserver extends NavigatorObserver { + String? currentRouteName; + + @override + void didPush(Route route, Route? previousRoute) { + currentRouteName = route.settings.name; + } +} + class TestLinkInfo extends LinkInfo { TestLinkInfo({ required this.uri, @@ -218,3 +437,13 @@ class TestLinkInfo extends LinkInfo { @override bool get isDisabled => uri == null; } + +class TestUrlLauncherPlugin extends UrlLauncherPlugin { + final List launches = []; + + @override + Future launchUrl(String url, LaunchOptions options) async { + launches.add(url); + return true; + } +} 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 558f3a135a27..3a286fb40749 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -13,6 +13,7 @@ import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/services.dart'; import 'package:url_launcher_platform_interface/link.dart'; +import 'package:url_launcher_platform_interface/url_launcher_platform_interface.dart'; import 'package:web/web.dart' as html; /// The unique identifier for the view type to be used for link platform views. @@ -98,6 +99,8 @@ class WebLinkDelegateState extends State { } } +final JSAny _useCapture = {'capture': true}.jsify()!; + /// Controls link views. class LinkViewController extends PlatformViewController { /// Creates a [LinkViewController] instance with the unique [viewId]. @@ -106,10 +109,13 @@ class LinkViewController extends PlatformViewController { // This is the first controller being created, attach the global click // listener. - _clickSubscription = - const html.EventStreamProvider('click') - .forTarget(html.window) - .listen(_onGlobalClick); + // Why listen in the capture phase? + // + // To ensure we always receive the event even if the engine calls + // `stopPropagation`. + html.window + .addEventListener('keydown', _jsGlobalKeydownListener, _useCapture); + html.window.addEventListener('click', _jsGlobalClickListener); } _instances[viewId] = this; } @@ -142,13 +148,70 @@ class LinkViewController extends PlatformViewController { static int? _hitTestedViewId; - static late StreamSubscription _clickSubscription; + static final JSFunction _jsGlobalKeydownListener = _onGlobalKeydown.toJS; + static final JSFunction _jsGlobalClickListener = _onGlobalClick.toJS; + + static void _onGlobalKeydown(html.KeyboardEvent event) { + // Why not use `event.target`? + // + // Because the target is usually and not the element, so + // it's not very helpful. That's because focus management is handled by + // Flutter, and the browser doesn't always know which element is focused. In + // fact, in many cases, the focused widget is fully drawn on canvas and + // there's no corresponding HTML element to receive browser focus. + + // Why not check for "Enter" or "Space" keys? + // + // Because we don't know (nor do we want to assume) which keys the app + // considers to be "trigger" keys. So we let the app do its thing, and if it + // decides to "trigger" the link, it will call `followLink`, which will set + // `_hitTestedViewId` to the ID of the triggered Link. + + // Life of a keydown event: + // + // For simplicity, let's assume we are dealing with a Link widget setup with + // with a button widget like this: + // + // ```dart + // Link( + // uri: Uri.parse('...'), + // builder: (context, followLink) { + // return ElevatedButton( + // onPressed: followLink, + // child: const Text('Press me'), + // ); + // }, + // ); + // ``` + // + // 1. The user navigates through the UI using the Tab key until they reach + // the button in question. + // 2. The user presses the Enter key to trigger the link. + // 3. The framework receives the Enter keydown event: + // - The event is dispatched to the button widget. + // - The button widget calls `onPressed` and therefor `followLink`. + // - `followLink` calls `LinkViewController.registerHitTest`. + // - `LinkViewController.registerHitTest` sets `_hitTestedViewId`. + // 4. The `LinkViewController` also receives the keydown event: + // - We check the value of `_hitTestedViewId`. + // - If `_hitTestedViewId` is set, it means the app triggered the link. + // - We navigate to the Link's URI. + + // The keydown event is not directly associated with the target Link, so + // we need to look for the recently hit tested Link to handle the event. + if (_hitTestedViewId != null) { + _instances[_hitTestedViewId]?._onDomKeydown(); + } + // After the keyboard event has been received, clean up the hit test state + // so we can start fresh on the next event. + unregisterHitTest(); + } static void _onGlobalClick(html.MouseEvent event) { final int? viewId = getViewIdFromTarget(event); _instances[viewId]?._onDomClick(event); // After the DOM click event has been received, clean up the hit test state - // so we can start fresh on the next click. + // so we can start fresh on the next event. unregisterHitTest(); } @@ -192,6 +255,27 @@ class LinkViewController extends PlatformViewController { await SystemChannels.platform_views.invokeMethod('create', args); } + void _onDomKeydown() { + assert( + _hitTestedViewId == viewId, + 'Keydown event should only be handled by the hit tested Link', + ); + + if (_isExternalLink) { + // External links are not handled by the browser when triggered via a + // keydown, so we have to launch the url manually. + UrlLauncherPlatform.instance + .launchUrl(_uri.toString(), const LaunchOptions()); + return; + } + + // A uri that doesn't have a scheme is an internal route name. In this + // case, we push it via Flutter's navigation system instead of using + // `launchUrl`. + final String routeName = _uri.toString(); + pushRouteNameToFramework(null, routeName); + } + void _onDomClick(html.MouseEvent event) { final bool isHitTested = _hitTestedViewId == viewId; if (!isHitTested) { @@ -202,7 +286,7 @@ class LinkViewController extends PlatformViewController { return; } - if (_uri != null && _uri!.hasScheme) { + if (_isExternalLink) { // External links will be handled by the browser, so we don't have to do // anything. return; @@ -217,6 +301,7 @@ class LinkViewController extends PlatformViewController { } Uri? _uri; + bool get _isExternalLink => _uri != null && _uri!.hasScheme; /// Set the [Uri] value for this link. /// @@ -274,7 +359,9 @@ class LinkViewController extends PlatformViewController { assert(_instances[viewId] == this); _instances.remove(viewId); if (_instances.isEmpty) { - await _clickSubscription.cancel(); + html.window.removeEventListener('click', _jsGlobalClickListener); + html.window.removeEventListener( + 'keydown', _jsGlobalKeydownListener, _useCapture); } await SystemChannels.platform_views.invokeMethod('dispose', viewId); } diff --git a/packages/url_launcher/url_launcher_web/pubspec.yaml b/packages/url_launcher/url_launcher_web/pubspec.yaml index 35154b5a6d2b..51f320a32a5e 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.3.0 +version: 2.3.1 environment: sdk: ^3.3.0