From d3e16eaa3b7861b8f993e35fe8823e0e8a17ac76 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Thu, 9 May 2024 13:16:16 -0400 Subject: [PATCH 01/22] [url_launcher][web] Better support for semantics in the Link widget --- .../url_launcher_web/lib/src/link.dart | 430 +++++++++++++----- 1 file changed, 306 insertions(+), 124 deletions(-) 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 3a286fb40749..f8dd3c5ad466 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -42,12 +42,39 @@ class WebLinkDelegate extends StatefulWidget { WebLinkDelegateState createState() => WebLinkDelegateState(); } +extension on Uri { + String getHref() { + if (hasScheme) { + // External URIs are not modified. + return toString(); + } + + if (ui_web.urlStrategy == null) { + // If there's no UrlStrategy, we leave the URI as is. + return toString(); + } + + // In case an internal uri is given, the uri must be properly encoded + // using the currently used UrlStrategy. + return ui_web.urlStrategy!.prepareExternalUrl(toString()); + } +} + +int _nextSemanticsIdentifier = 0; + /// The link delegate used on the web platform. /// /// For external URIs, it lets the browser do its thing. For app route names, it /// pushes the route name to the framework. class WebLinkDelegateState extends State { late LinkViewController _controller; + late final String _semanticIdentifier; + + @override + void initState() { + super.initState(); + _semanticIdentifier = 'sem-id-${_nextSemanticsIdentifier++}'; + } @override void didUpdateWidget(WebLinkDelegate oldWidget) { @@ -61,7 +88,7 @@ class WebLinkDelegateState extends State { } Future _followLink() { - LinkViewController.registerHitTest(_controller); + LinkViewController.onFollowLink(_controller.viewId); return Future.value(); } @@ -70,28 +97,37 @@ class WebLinkDelegateState extends State { return Stack( fit: StackFit.passthrough, children: [ - widget.link.builder( - context, - widget.link.isDisabled ? null : _followLink, + Semantics( + link: true, + identifier: _semanticIdentifier, + value: widget.link.uri?.getHref(), + child: widget.link.builder( + context, + widget.link.isDisabled ? null : _followLink, + ), ), Positioned.fill( - child: PlatformViewLink( - viewType: linkViewType, - onCreatePlatformView: (PlatformViewCreationParams params) { - _controller = LinkViewController.fromParams(params); - return _controller - ..setUri(widget.link.uri) - ..setTarget(widget.link.target); - }, - surfaceFactory: - (BuildContext context, PlatformViewController controller) { - return PlatformViewSurface( - controller: controller, - gestureRecognizers: const >{}, - hitTestBehavior: PlatformViewHitTestBehavior.transparent, - ); - }, + child: ExcludeFocus( + child: ExcludeSemantics( + child: PlatformViewLink( + viewType: linkViewType, + onCreatePlatformView: (PlatformViewCreationParams params) { + _controller = LinkViewController.fromParams(params, _semanticIdentifier); + return _controller + ..setUri(widget.link.uri) + ..setTarget(widget.link.target); + }, + surfaceFactory: + (BuildContext context, PlatformViewController controller) { + return PlatformViewSurface( + controller: controller, + gestureRecognizers: const >{}, + hitTestBehavior: PlatformViewHitTestBehavior.transparent, + ); + }, + ), + ), ), ), ], @@ -101,56 +137,161 @@ class WebLinkDelegateState extends State { final JSAny _useCapture = {'capture': true}.jsify()!; +/// Keeps track of the signals required to trigger a link. +/// +/// Automatically resets the signals after a certain delay. This is to prevent +/// the signals from getting stale. +class LinkTriggerSignals { + LinkTriggerSignals({required this.staleTimeout}); + + /// Specifies the duration after which the signals are considered stale. + /// + /// Signals have to arrive within [staleTimeout] duration between them to be + /// considered valid. If they don't, the signals are reset. + final Duration staleTimeout; + + /// Whether the we got all the signals required to trigger the link. + bool get isReadyToTrigger => _hasFollowLink && _hasDomEvent; + + int? get viewId { + assert(_isValid); + + return _viewIdFromFollowLink; + } + + void registerFollowLink({required int viewId}) { + _hasFollowLink = true; + _viewIdFromFollowLink = viewId; + _update(); + } + + void registerDomEvent({ + required int? viewId, + required html.MouseEvent? mouseEvent, + }) { + if (mouseEvent != null && viewId == null) { + throw AssertionError('`viewId` must be provided for mouse events'); + } + _hasDomEvent = true; + _viewIdFromDomEvent = viewId; + this.mouseEvent = mouseEvent; + _update(); + } + + bool _hasFollowLink = false; + bool _hasDomEvent = false; + + int? _viewIdFromFollowLink; + int? _viewIdFromDomEvent; + + html.MouseEvent? mouseEvent; + + // The signals state is considered invalid if the view IDs from the follow + // link and the DOM event don't match. + bool get _isValid { + if (_viewIdFromFollowLink == null || _viewIdFromDomEvent == null) { + // We haven't received all view IDs yet, so we can't determine if the + // signals are valid. + return true; + } + + return _viewIdFromFollowLink == _viewIdFromDomEvent; + } + + Timer? _resetTimer; + + void _update() { + // When the state of signals is invalid, we reset the signals immediately. + if (_isValid) { + _resetTimer?.cancel(); + _resetTimer = Timer(staleTimeout, reset); + } else { + reset(); + } + } + + /// Reset all signals to their initial state. + void reset() { + _resetTimer?.cancel(); + _resetTimer = null; + + _hasFollowLink = false; + _hasDomEvent = false; + + _viewIdFromFollowLink = null; + _viewIdFromDomEvent = null; + + mouseEvent = null; + } +} + /// Controls link views. class LinkViewController extends PlatformViewController { /// Creates a [LinkViewController] instance with the unique [viewId]. - LinkViewController(this.viewId) { - if (_instances.isEmpty) { + LinkViewController(this.viewId, this._semanticIdentifier) { + if (_instancesByViewId.isEmpty) { // This is the first controller being created, attach the global click // listener. - - // 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); + _attachGlobalListeners(); } - _instances[viewId] = this; + _instancesByViewId[viewId] = this; + _instancesBySemanticIdentifier[_semanticIdentifier] = this; } /// Creates and initializes a [LinkViewController] instance with the given /// platform view [params]. factory LinkViewController.fromParams( PlatformViewCreationParams params, + String semanticIdentifier, ) { final int viewId = params.id; - final LinkViewController controller = LinkViewController(viewId); + final LinkViewController controller = LinkViewController(viewId, semanticIdentifier); controller._initialize().then((_) { /// Because _initialize is async, it can happen that [LinkViewController.dispose] /// may get called before this `then` callback. /// Check that the `controller` that was created by this factory is not /// disposed before calling `onPlatformViewCreated`. - if (_instances[viewId] == controller) { + if (_instancesByViewId[viewId] == controller) { params.onPlatformViewCreated(viewId); } }); return controller; } - static final Map _instances = + static final Map _instancesByViewId = {}; + static final Map _instancesBySemanticIdentifier = + {}; static html.Element _viewFactory(int viewId) { - return _instances[viewId]!._element; + return _instancesByViewId[viewId]!._element; } - static int? _hitTestedViewId; + static final LinkTriggerSignals _triggerSignals = + LinkTriggerSignals(staleTimeout: const Duration(milliseconds: 500)); static final JSFunction _jsGlobalKeydownListener = _onGlobalKeydown.toJS; static final JSFunction _jsGlobalClickListener = _onGlobalClick.toJS; + static void _attachGlobalListeners() { + // 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) + ..addEventListener('click', _jsGlobalClickListener, _useCapture); + + // TODO(mdebbar): Cleanup the global listeners on hot restart. + // https://github.com/flutter/flutter/issues/148133 + } + + static void _detachGlobalListeners() { + html.window + ..removeEventListener('keydown', _jsGlobalKeydownListener, _useCapture) + ..removeEventListener('click', _jsGlobalClickListener, _useCapture); + } + static void _onGlobalKeydown(html.KeyboardEvent event) { // Why not use `event.target`? // @@ -197,22 +338,54 @@ class LinkViewController extends PlatformViewController { // - If `_hitTestedViewId` is set, it means the app triggered the link. // - We navigate to the Link's URI. + if (_isModifierKey(event)) { + // Modifier keys (i.e. Shift, Ctrl, Alt, Meta) cannot trigger a Link. + return; + } + // 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(); + // we can't find the `viewId` from the event. + _triggerSignals.registerDomEvent(viewId: null, mouseEvent: null); + + if (_triggerSignals.isReadyToTrigger) { + _triggerLink(); } - // After the keyboard event has been received, clean up the hit test state - // so we can start fresh on the next event. - unregisterHitTest(); } + /// Global click handler that triggers on the `capture` phase. We use `capture` + /// because some events may be consumed and prevent further propagation at the + /// target. This may lead to issues (see: https://github.com/flutter/flutter/issues/143164) + /// where a followLink was executed but the event never bubbles back up to the + /// window (e.g. when button semantics obscure the platform view). We make sure + /// to only trigger the link if a hit test was registered and remains valid at + /// the time the click handler executes. 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 event. - unregisterHitTest(); + final html.Element? targetElement = event.target as html.Element?; + + // We only want to handle clicks that land on *our* links, whether that's a + // platform view link or a semantics link. + final int? viewIdFromTarget = _getViewIdFromLink(targetElement) ?? + _getViewIdFromSemanticLink(targetElement); + + if (viewIdFromTarget == null) { + // The click target was not one of our links, so we don't want to + // interfere with it. + // + // We also want to reset the signals in this case. + _triggerSignals.reset(); + return; + } + + // TODO: preventDefault if there's a mismatch in view IDs. + + _triggerSignals.registerDomEvent( + viewId: viewIdFromTarget, + mouseEvent: event, + ); + + if (_triggerSignals.isReadyToTrigger) { + _triggerLink(); + } } /// Call this method to indicate that a hit test has been registered for the @@ -220,18 +393,20 @@ class LinkViewController extends PlatformViewController { /// /// The [onClick] callback is invoked when the anchor element receives a /// `click` from the browser. - static void registerHitTest(LinkViewController controller) { - _hitTestedViewId = controller.viewId; - } + static void onFollowLink(int viewId) { + // TODO: preventDefault on mouseEvent if there's a mismatch in view IDs. + _triggerSignals.registerFollowLink(viewId: viewId); - /// Removes all information about previously registered hit tests. - static void unregisterHitTest() { - _hitTestedViewId = null; + if (_triggerSignals.isReadyToTrigger) { + _triggerLink(); + } } @override final int viewId; + final String _semanticIdentifier; + late html.HTMLElement _element; Future _initialize() async { @@ -255,48 +430,41 @@ 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', - ); + /// Triggers the Link that has already received all the required signals. + /// + /// It also handles logic for external vs internal links, triggered by a mouse + /// vs keyboard event. + static void _triggerLink() { + assert(_triggerSignals.isReadyToTrigger); - 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; - } + final LinkViewController controller = _instancesByViewId[_triggerSignals.viewId!]!; + final html.MouseEvent? mouseEvent = _triggerSignals.mouseEvent; - // 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); - } + // Make sure to reset no matter what code path we end up taking. + _triggerSignals.reset(); - void _onDomClick(html.MouseEvent event) { - final bool isHitTested = _hitTestedViewId == viewId; - if (!isHitTested) { - // There was no hit test registered for this click. This means the click - // landed on the anchor element but not on the underlying widget. In this - // case, we prevent the browser from following the click. - event.preventDefault(); + if (mouseEvent != null && _isModifierKey(mouseEvent)) { return; } - if (_isExternalLink) { - // External links will be handled by the browser, so we don't have to do - // anything. + if (controller._isExternalLink) { + if (mouseEvent == null) { + // When external links are trigger 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()); + } + + // When triggerd by a mouse event, external links will be handled by the + // browser, so we don't have to do anything. 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 letting the // browser handle it. - event.preventDefault(); - final String routeName = _uri.toString(); + mouseEvent?.preventDefault(); + final String routeName = controller._uri.toString(); pushRouteNameToFramework(null, routeName); } @@ -311,13 +479,7 @@ class LinkViewController extends PlatformViewController { if (uri == null) { _element.removeAttribute('href'); } else { - String href = uri.toString(); - // in case an internal uri is given, the url mus be properly encoded - // using the currently used [UrlStrategy] - if (!uri.hasScheme) { - href = ui_web.urlStrategy?.prepareExternalUrl(href) ?? href; - } - _element.setAttribute('href', href); + _element.setAttribute('href', uri.getHref()); } } @@ -342,6 +504,24 @@ class LinkViewController extends PlatformViewController { return '_self'; } + /// Finds the view ID in the Link's semantic element. + /// + /// Returns null if [target] is not a semantics element for one of our Links. + static int? _getViewIdFromSemanticLink(html.Element? target) { + // TODO: what if `target` IS the semantic element? + if (target != null && _isWithinSemanticTree(target)) { + final html.Element? semanticLink = _getClosestSemanticLink(target); + if (semanticLink != null) { + // TODO: Find out the view ID of semantic link. + final String? semanticIdentifier = semanticLink.getAttribute('semantic-identifier'); + if (semanticIdentifier != null) { + return _instancesBySemanticIdentifier[semanticIdentifier]?.viewId; + } + } + } + return null; + } + @override Future clearFocus() async { // Currently this does nothing on Flutter Web. @@ -356,49 +536,51 @@ class LinkViewController extends PlatformViewController { @override Future dispose() async { - assert(_instances[viewId] == this); - _instances.remove(viewId); - if (_instances.isEmpty) { - html.window.removeEventListener('click', _jsGlobalClickListener); - html.window.removeEventListener( - 'keydown', _jsGlobalKeydownListener, _useCapture); + assert(_instancesByViewId[viewId] == this); + assert(_instancesBySemanticIdentifier[_semanticIdentifier] == this); + + _instancesByViewId.remove(viewId); + _instancesBySemanticIdentifier.remove(_semanticIdentifier); + + if (_instancesByViewId.isEmpty) { + _detachGlobalListeners(); } await SystemChannels.platform_views.invokeMethod('dispose', viewId); } } -/// Finds the view id of the DOM element targeted by the [event]. -int? getViewIdFromTarget(html.Event event) { - final html.Element? linkElement = getLinkElementFromTarget(event); - if (linkElement != null) { - return linkElement.getProperty(linkViewIdProperty.toJS).toDartInt; +/// Finds the view ID in the Link's platform view element. +/// +/// Returns null if [target] is not a platform view of one of our Links. +int? _getViewIdFromLink(html.Element? target) { + final JSString linkViewIdPropertyJS = linkViewIdProperty.toJS; + if (target != null && target.tagName.toLowerCase() == 'a') { + return target.getProperty(linkViewIdPropertyJS)?.toDartInt; } return null; } -/// Finds the targeted DOM element by the [event]. +/// Whether [element] is within the semantic tree of a Flutter View. +bool _isWithinSemanticTree(html.Element element) { + return element.closest('flt-semantics-host') != null; +} + +/// Returns the closest semantic link ancestor of the given [element]. /// -/// It handles the case where the target element is inside a shadow DOM too. -html.Element? getLinkElementFromTarget(html.Event event) { - final html.EventTarget? target = event.target; - if (target != null && target is html.Element) { - if (isLinkElement(target)) { - return target; - } - if (target.shadowRoot != null) { - final html.Node? child = target.shadowRoot!.lastChild; - if (child != null && child is html.Element && isLinkElement(child)) { - return child; - } - } - } - return null; +/// If [element] itself is a link, it is returned. +html.Element? _getClosestSemanticLink(html.Element element) { + assert(_isWithinSemanticTree(element)); + return element.closest('a[id^="flt-semantic-node-"]'); } -/// Checks if the given [element] is a link that was created by -/// [LinkViewController]. -bool isLinkElement(html.Element? element) { - return element != null && - element.tagName == 'A' && - element.hasProperty(linkViewIdProperty.toJS).toDart; +bool _isModifierKey(html.Event event) { + // 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 + // `MouseEvent` to access these properties. + // + // It's safe to cast both event types to `KeyboardEvent` because it's just + // JS-interop and has no concrete runtime type. + event as html.KeyboardEvent; + return event.ctrlKey || event.altKey || event.metaKey || event.shiftKey; } From cce04c4b3ed887f0bfd2fb533e99c8db11d2e7db Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Mon, 20 May 2024 12:52:48 -0400 Subject: [PATCH 02/22] tests --- .../integration_test/link_widget_test.dart | 346 +++++++++++++----- .../url_launcher_web/lib/src/link.dart | 38 +- 2 files changed, 289 insertions(+), 95 deletions(-) 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 9c700c87d0eb..936780dfe3b9 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 @@ -4,6 +4,7 @@ import 'dart:js_interop'; import 'dart:js_interop_unsafe'; +import 'dart:typed_data'; import 'dart:ui_web' as ui_web; import 'package:flutter/material.dart'; @@ -18,6 +19,24 @@ import 'package:web/web.dart' as html; void main() { IntegrationTestWidgetsFlutterBinding.ensureInitialized(); + final List pushedRouteNames = []; + late Future Function(String) originalPushFunction; + + setUp(() { + pushedRouteNames.clear(); + originalPushFunction = pushRouteToFrameworkFunction; + pushRouteToFrameworkFunction = (String routeName) { + pushedRouteNames.add(routeName); + return Future.value(ByteData(0)); + }; + }); + + tearDown(() { + pushRouteToFrameworkFunction = originalPushFunction; + pushedRouteNames.clear(); + LinkViewController.debugReset(); + }); + group('Link Widget', () { testWidgets('creates anchor with correct attributes', (WidgetTester tester) async { @@ -190,43 +209,38 @@ void main() { 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); - }, - )), - ), + home: 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(pushedRouteNames, isEmpty); expect(testPlugin.launches, isEmpty); final html.Element anchor = _findSingleAnchor(); await followLinkCallback!(); - _simulateClick(anchor); - await tester.pumpAndSettle(); + final html.Event event = _simulateClick(anchor); // Internal links should navigate the app to the specified route. There // should be no calls to `launchUrl`. - expect(observer.currentRouteName, '/foobar'); + expect(pushedRouteNames, ['/foobar']); expect(testPlugin.launches, isEmpty); + expect(event.defaultPrevented, isTrue); // Needed when testing on on Chrome98 headless in CI. // See https://github.com/flutter/flutter/issues/121161 @@ -235,43 +249,38 @@ void main() { 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); - }, - )), - ), + home: 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(pushedRouteNames, isEmpty); expect(testPlugin.launches, isEmpty); final html.Element anchor = _findSingleAnchor(); await followLinkCallback!(); - _simulateKeydown(anchor); - await tester.pumpAndSettle(); + final html.KeyboardEvent event = _simulateKeydown(anchor); // Internal links should navigate the app to the specified route. There // should be no calls to `launchUrl`. - expect(observer.currentRouteName, '/foobar'); + expect(pushedRouteNames, ['/foobar']); expect(testPlugin.launches, isEmpty); + expect(event.defaultPrevented, isFalse); // Needed when testing on on Chrome98 headless in CI. // See https://github.com/flutter/flutter/issues/121161 @@ -280,41 +289,36 @@ void main() { 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); - }, - )), - ), + home: 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(pushedRouteNames, isEmpty); expect(testPlugin.launches, isEmpty); final html.Element anchor = _findSingleAnchor(); await followLinkCallback!(); - _simulateClick(anchor); - await tester.pumpAndSettle(); + final html.Event event = _simulateClick(anchor); // 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(pushedRouteNames, isEmpty); expect(testPlugin.launches, isEmpty); + expect(event.defaultPrevented, isFalse); // Needed when testing on on Chrome98 headless in CI. // See https://github.com/flutter/flutter/issues/121161 @@ -323,40 +327,205 @@ void main() { 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); - }, - )), - ), + home: 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(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + + final html.Element anchor = _findSingleAnchor(); + + await followLinkCallback!(); + final html.KeyboardEvent event = _simulateKeydown(anchor); + + // External links that are triggered by keyboard are handled by calling + // `launchUrl`, and there's no change to the app's route name. + expect(pushedRouteNames, isEmpty); + expect(testPlugin.launches, ['https://google.com']); + expect(event.defaultPrevented, isFalse); + + // Needed when testing on on Chrome98 headless in CI. + // See https://github.com/flutter/flutter/issues/121161 + await tester.pumpAndSettle(); + }); + }); + + group('Follows links (reversed order)', () { + 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 Uri uri = Uri.parse('/foobar'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + routes: { + '/foobar': (BuildContext context) => const Text('Internal route'), + }, + home: 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(pushedRouteNames, isEmpty); expect(testPlugin.launches, isEmpty); final html.Element anchor = _findSingleAnchor(); + final html.Event event = _simulateClick(anchor); await followLinkCallback!(); - _simulateKeydown(anchor); + + // Internal links should navigate the app to the specified route. There + // should be no calls to `launchUrl`. + expect(pushedRouteNames, ['/foobar']); + expect(testPlugin.launches, isEmpty); + expect(event.defaultPrevented, isTrue); + + // 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 Uri uri = Uri.parse('/foobar'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + routes: { + '/foobar': (BuildContext context) => const Text('Internal route'), + }, + home: 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(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + + final html.Element anchor = _findSingleAnchor(); + + final html.KeyboardEvent event = _simulateKeydown(anchor); + await followLinkCallback!(); + + // Internal links should navigate the app to the specified route. There + // should be no calls to `launchUrl`. + expect(pushedRouteNames, ['/foobar']); + expect(testPlugin.launches, isEmpty); + expect(event.defaultPrevented, isFalse); + + // 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 Uri uri = Uri.parse('https://google.com'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + home: 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(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + + final html.Element anchor = _findSingleAnchor(); + + final html.Event event = _simulateClick(anchor); + await followLinkCallback!(); + + // 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(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + expect(event.defaultPrevented, isFalse); + + // 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 Uri uri = Uri.parse('https://google.com'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + home: 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(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + + final html.Element anchor = _findSingleAnchor(); + + final html.KeyboardEvent event = _simulateKeydown(anchor); + await followLinkCallback!(); // 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(pushedRouteNames, isEmpty); expect(testPlugin.launches, ['https://google.com']); + expect(event.defaultPrevented, isFalse); // Needed when testing on on Chrome98 headless in CI. // See https://github.com/flutter/flutter/issues/121161 @@ -365,7 +534,7 @@ void main() { }); } -html.Element _findSingleAnchor() { +List _findAllAnchors() { final List foundAnchors = []; html.NodeList anchors = html.document.querySelectorAll('a'); for (int i = 0; i < anchors.length; i++) { @@ -388,34 +557,33 @@ html.Element _findSingleAnchor() { } } - return foundAnchors.single; + return foundAnchors; } -void _simulateClick(html.Element target) { - target.dispatchEvent( - html.MouseEvent( - 'click', - html.MouseEventInit()..bubbles = true, - ), - ); +html.Element _findSingleAnchor() { + return _findAllAnchors().single; } -void _simulateKeydown(html.Element target) { - target.dispatchEvent( - html.KeyboardEvent( - 'keydown', - html.KeyboardEventInit()..bubbles = true, - ), +html.MouseEvent _simulateClick(html.Element target) { + final html.MouseEvent mouseEvent = html.MouseEvent( + 'click', + html.MouseEventInit() + ..bubbles = true + ..cancelable = true, ); + LinkViewController.handleGlobalClick(event: mouseEvent, target: target); + return mouseEvent; } -class TestNavigatorObserver extends NavigatorObserver { - String? currentRouteName; - - @override - void didPush(Route route, Route? previousRoute) { - currentRouteName = route.settings.name; - } +html.KeyboardEvent _simulateKeydown(html.Element target) { + final html.KeyboardEvent keydownEvent = html.KeyboardEvent( + 'keydown', + html.KeyboardEventInit() + ..bubbles = true + ..cancelable = true, + ); + LinkViewController.handleGlobalKeydown(event: keydownEvent); + return keydownEvent; } class TestLinkInfo extends LinkInfo { 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 f8dd3c5ad466..07b0553e5195 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -28,6 +28,11 @@ typedef HtmlViewFactory = html.Element Function(int viewId); /// Factory that returns the link DOM element for each unique view id. HtmlViewFactory get linkViewFactory => LinkViewController._viewFactory; +/// The function used to push routes to the Flutter framework. +@visibleForTesting +Future Function(String) pushRouteToFrameworkFunction = + (String routeName) => pushRouteNameToFramework(null, routeName); + /// The delegate for building the [Link] widget on the web. /// /// It uses a platform view to render an anchor element in the DOM. @@ -293,6 +298,10 @@ class LinkViewController extends PlatformViewController { } static void _onGlobalKeydown(html.KeyboardEvent event) { + handleGlobalKeydown(event: event); + } + + static void handleGlobalKeydown({required html.KeyboardEvent event}) { // Why not use `event.target`? // // Because the target is usually and not the element, so @@ -352,6 +361,13 @@ class LinkViewController extends PlatformViewController { } } + static void _onGlobalClick(html.MouseEvent event) { + handleGlobalClick( + event: event, + target: event.target as html.Element?, + ); + } + /// Global click handler that triggers on the `capture` phase. We use `capture` /// because some events may be consumed and prevent further propagation at the /// target. This may lead to issues (see: https://github.com/flutter/flutter/issues/143164) @@ -359,13 +375,15 @@ class LinkViewController extends PlatformViewController { /// window (e.g. when button semantics obscure the platform view). We make sure /// to only trigger the link if a hit test was registered and remains valid at /// the time the click handler executes. - static void _onGlobalClick(html.MouseEvent event) { - final html.Element? targetElement = event.target as html.Element?; - + @visibleForTesting + static void handleGlobalClick({ + required html.MouseEvent event, + required html.Element? target, + }) { // We only want to handle clicks that land on *our* links, whether that's a // platform view link or a semantics link. - final int? viewIdFromTarget = _getViewIdFromLink(targetElement) ?? - _getViewIdFromSemanticLink(targetElement); + final int? viewIdFromTarget = _getViewIdFromLink(target) ?? + _getViewIdFromSemanticLink(target); if (viewIdFromTarget == null) { // The click target was not one of our links, so we don't want to @@ -465,7 +483,7 @@ class LinkViewController extends PlatformViewController { // browser handle it. mouseEvent?.preventDefault(); final String routeName = controller._uri.toString(); - pushRouteNameToFramework(null, routeName); + pushRouteToFrameworkFunction(routeName); } Uri? _uri; @@ -547,6 +565,14 @@ class LinkViewController extends PlatformViewController { } await SystemChannels.platform_views.invokeMethod('dispose', viewId); } + + @visibleForTesting + static Future debugReset() async { + _triggerSignals.reset(); + for (final LinkViewController instance in _instancesByViewId.values) { + await instance.dispose(); + } + } } /// Finds the view ID in the Link's platform view element. From b3d16c778b796aa33b4e68c6c5ac47ab07be64cb Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Wed, 22 May 2024 12:01:24 -0400 Subject: [PATCH 03/22] click on mismatched link --- .../integration_test/link_widget_test.dart | 77 +++++++++++++++++++ .../url_launcher_web/lib/src/link.dart | 41 ++++++---- 2 files changed, 102 insertions(+), 16 deletions(-) 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 936780dfe3b9..c247fc930618 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 @@ -361,6 +361,83 @@ void main() { // See https://github.com/flutter/flutter/issues/121161 await tester.pumpAndSettle(); }); + + testWidgets('click on mismatched link', (WidgetTester tester) async { + final Uri uri1 = Uri.parse('/foobar1'); + final Uri uri2 = Uri.parse('/foobar2'); + FollowLink? followLinkCallback1; + FollowLink? followLinkCallback2; + + await tester.pumpWidget(MaterialApp( + routes: { + '/foobar1': (BuildContext context) => const Text('Internal route 1'), + '/foobar2': (BuildContext context) => const Text('Internal route 2'), + }, + home: Column( + children: [ + WebLinkDelegate(TestLinkInfo( + uri: uri1, + target: LinkTarget.blank, + builder: (BuildContext context, FollowLink? followLink) { + followLinkCallback1 = followLink; + return const SizedBox(width: 100, height: 100); + }, + )), + WebLinkDelegate(TestLinkInfo( + uri: uri2, + target: LinkTarget.blank, + builder: (BuildContext context, FollowLink? followLink) { + followLinkCallback2 = followLink; + return const SizedBox(width: 100, height: 100); + }, + )), + ], + ), + )); + // Platform view creation happens asynchronously. + await tester.pumpAndSettle(); + + expect(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + + final [ + html.Element anchor1, + html.Element anchor2, + ...List rest, + ] = _findAllAnchors(); + expect(rest, isEmpty); + + await followLinkCallback2!(); + // Click on mismatched link. + final html.Event event1 = _simulateClick(anchor1); + + // The link shouldn't have been triggered. + expect(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + expect(event1.defaultPrevented, isTrue); + + // Click on mismatched link (in reverse order). + final html.Event event2 = _simulateClick(anchor2); + await followLinkCallback1!(); + + // The link shouldn't have been triggered. + expect(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + expect(event2.defaultPrevented, isTrue); + + await followLinkCallback2!(); + // Click on the correct link. + final html.Event event = _simulateClick(anchor2); + + // The link should've been triggered now. + expect(pushedRouteNames, ['/foobar2']); + expect(testPlugin.launches, isEmpty); + expect(event.defaultPrevented, isTrue); + + // Needed when testing on on Chrome98 headless in CI. + // See https://github.com/flutter/flutter/issues/121161 + await tester.pumpAndSettle(); + }); }); group('Follows links (reversed order)', () { 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 07b0553e5195..7ddc7e9c76fc 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -159,15 +159,14 @@ class LinkTriggerSignals { bool get isReadyToTrigger => _hasFollowLink && _hasDomEvent; int? get viewId { - assert(_isValid); - + assert(isValid); return _viewIdFromFollowLink; } void registerFollowLink({required int viewId}) { _hasFollowLink = true; _viewIdFromFollowLink = viewId; - _update(); + _didUpdate(); } void registerDomEvent({ @@ -180,7 +179,7 @@ class LinkTriggerSignals { _hasDomEvent = true; _viewIdFromDomEvent = viewId; this.mouseEvent = mouseEvent; - _update(); + _didUpdate(); } bool _hasFollowLink = false; @@ -193,7 +192,7 @@ class LinkTriggerSignals { // The signals state is considered invalid if the view IDs from the follow // link and the DOM event don't match. - bool get _isValid { + bool get isValid { if (_viewIdFromFollowLink == null || _viewIdFromDomEvent == null) { // We haven't received all view IDs yet, so we can't determine if the // signals are valid. @@ -205,14 +204,9 @@ class LinkTriggerSignals { Timer? _resetTimer; - void _update() { - // When the state of signals is invalid, we reset the signals immediately. - if (_isValid) { - _resetTimer?.cancel(); - _resetTimer = Timer(staleTimeout, reset); - } else { - reset(); - } + void _didUpdate() { + _resetTimer?.cancel(); + _resetTimer = Timer(staleTimeout, reset); } /// Reset all signals to their initial state. @@ -394,13 +388,20 @@ class LinkViewController extends PlatformViewController { return; } - // TODO: preventDefault if there's a mismatch in view IDs. - _triggerSignals.registerDomEvent( viewId: viewIdFromTarget, mouseEvent: event, ); + if (!_triggerSignals.isValid) { + // The view ID from the target doesn't match the view ID from the follow + // link. Let's prevent the browser from navigating, and let's reset the + // signals so we start fresh on the next event. + event.preventDefault(); + _triggerSignals.reset(); + return; + } + if (_triggerSignals.isReadyToTrigger) { _triggerLink(); } @@ -412,9 +413,17 @@ class LinkViewController extends PlatformViewController { /// The [onClick] callback is invoked when the anchor element receives a /// `click` from the browser. static void onFollowLink(int viewId) { - // TODO: preventDefault on mouseEvent if there's a mismatch in view IDs. _triggerSignals.registerFollowLink(viewId: viewId); + if (!_triggerSignals.isValid) { + // The view ID from follow link doesn't match the view ID from the event + // target. Let's prevent the browser from navigating, and let's reset the + // signals so we start fresh on the next event. + _triggerSignals.mouseEvent?.preventDefault(); + _triggerSignals.reset(); + return; + } + if (_triggerSignals.isReadyToTrigger) { _triggerLink(); } From f6840659f3d5cba0b622238d348be7e7d0c30fe5 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Wed, 22 May 2024 14:51:10 -0400 Subject: [PATCH 04/22] more tests --- .../integration_test/link_widget_test.dart | 213 +++++++++++++++++- 1 file changed, 203 insertions(+), 10 deletions(-) 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 c247fc930618..c3b26fa88a43 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 @@ -289,7 +289,7 @@ void main() { testWidgets('click to navigate to external link', (WidgetTester tester) async { - final Uri uri = Uri.parse('https://google.com'); + final Uri uri = Uri.parse('https://flutter.dev'); FollowLink? followLinkCallback; await tester.pumpWidget(MaterialApp( @@ -327,7 +327,7 @@ void main() { testWidgets('keydown to navigate to external link', (WidgetTester tester) async { - final Uri uri = Uri.parse('https://google.com'); + final Uri uri = Uri.parse('https://flutter.dev'); FollowLink? followLinkCallback; await tester.pumpWidget(MaterialApp( @@ -354,7 +354,7 @@ void main() { // External links that are triggered by keyboard are handled by calling // `launchUrl`, and there's no change to the app's route name. expect(pushedRouteNames, isEmpty); - expect(testPlugin.launches, ['https://google.com']); + expect(testPlugin.launches, ['https://flutter.dev']); expect(event.defaultPrevented, isFalse); // Needed when testing on on Chrome98 headless in CI. @@ -438,6 +438,197 @@ void main() { // See https://github.com/flutter/flutter/issues/121161 await tester.pumpAndSettle(); }); + + testWidgets('trigger signals are reset after a delay', + (WidgetTester tester) async { + final Uri uri = Uri.parse('/foobar'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + routes: { + '/foobar': (BuildContext context) => const Text('Internal route'), + }, + home: 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(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + + final html.Element anchor = _findSingleAnchor(); + + // A large delay between signals should reset the previous signal. + await followLinkCallback!(); + await Future.delayed(const Duration(seconds: 1)); + final html.Event event1 = _simulateClick(anchor); + + // 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)); + + // Signals with large delay (in reverse order). + final html.Event event2 = _simulateClick(anchor); + await Future.delayed(const Duration(seconds: 1)); + await followLinkCallback!(); + + // 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)); + + // A small delay is okay. + await followLinkCallback!(); + await Future.delayed(const Duration(milliseconds: 100)); + final html.Event event3 = _simulateClick(anchor); + + // The link should've been triggered now. + expect(pushedRouteNames, ['/foobar']); + expect(testPlugin.launches, isEmpty); + expect(event3.defaultPrevented, isTrue); + + // Needed when testing on on Chrome98 headless in CI. + // See https://github.com/flutter/flutter/issues/121161 + await tester.pumpAndSettle(); + }); + + testWidgets('ignores clicks on non-Flutter link', + (WidgetTester tester) async { + final Uri uri = Uri.parse('/foobar'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + routes: { + '/foobar': (BuildContext context) => const Text('Internal route'), + }, + home: 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(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + + final html.Element nonFlutterAnchor = html.document.createElement('a'); + nonFlutterAnchor.setAttribute('href', '/non-flutter'); + + await followLinkCallback!(); + final html.Event event = _simulateClick(nonFlutterAnchor); + + // The link shouldn't have been triggered. + expect(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + expect(event.defaultPrevented, isFalse); + + // Needed when testing on on Chrome98 headless in CI. + // See https://github.com/flutter/flutter/issues/121161 + await tester.pumpAndSettle(); + }); + + testWidgets('handles cmd+click correctly', (WidgetTester tester) async { + final Uri uri = Uri.parse('/foobar'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + routes: { + '/foobar': (BuildContext context) => const Text('Internal route'), + }, + home: 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(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + + final html.Element anchor = _findSingleAnchor(); + + await followLinkCallback!(); + final html.Event event = _simulateClick(anchor, metaKey: true); + + // When a modifier key is present, we should let the browser handle the + // navigation. That means we do nothing on our side. + expect(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + expect(event.defaultPrevented, isFalse); + + // Needed when testing on on Chrome98 headless in CI. + // See https://github.com/flutter/flutter/issues/121161 + await tester.pumpAndSettle(); + }); + + testWidgets('ignores keydown when it is a modifier key', + (WidgetTester tester) async { + final Uri uri = Uri.parse('/foobar'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + routes: { + '/foobar': (BuildContext context) => const Text('Internal route'), + }, + home: 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(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + + final html.Element anchor = _findSingleAnchor(); + + final html.KeyboardEvent event1 = _simulateKeydown(anchor, metaKey: true); + await followLinkCallback!(); + + // When the pressed key is a modifier key, we should ignore it. + expect(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + expect(event1.defaultPrevented, isFalse); + + // If later we receive another trigger, it should work. + final html.KeyboardEvent event2 = _simulateKeydown(anchor); + + // Now the link should be triggered. + expect(pushedRouteNames, ['/foobar']); + expect(testPlugin.launches, isEmpty); + expect(event2.defaultPrevented, isFalse); + + // Needed when testing on on Chrome98 headless in CI. + // See https://github.com/flutter/flutter/issues/121161 + await tester.pumpAndSettle(); + }); }); group('Follows links (reversed order)', () { @@ -536,7 +727,7 @@ void main() { testWidgets('click to navigate to external link', (WidgetTester tester) async { - final Uri uri = Uri.parse('https://google.com'); + final Uri uri = Uri.parse('https://flutter.dev'); FollowLink? followLinkCallback; await tester.pumpWidget(MaterialApp( @@ -574,7 +765,7 @@ void main() { testWidgets('keydown to navigate to external link', (WidgetTester tester) async { - final Uri uri = Uri.parse('https://google.com'); + final Uri uri = Uri.parse('https://flutter.dev'); FollowLink? followLinkCallback; await tester.pumpWidget(MaterialApp( @@ -601,7 +792,7 @@ void main() { // External links that are triggered by keyboard are handled by calling // `launchUrl`, and there's no change to the app's route name. expect(pushedRouteNames, isEmpty); - expect(testPlugin.launches, ['https://google.com']); + expect(testPlugin.launches, ['https://flutter.dev']); expect(event.defaultPrevented, isFalse); // Needed when testing on on Chrome98 headless in CI. @@ -641,23 +832,25 @@ html.Element _findSingleAnchor() { return _findAllAnchors().single; } -html.MouseEvent _simulateClick(html.Element target) { +html.MouseEvent _simulateClick(html.Element target, {bool? metaKey}) { final html.MouseEvent mouseEvent = html.MouseEvent( 'click', html.MouseEventInit() ..bubbles = true - ..cancelable = true, + ..cancelable = true + ..metaKey = metaKey ?? false, ); LinkViewController.handleGlobalClick(event: mouseEvent, target: target); return mouseEvent; } -html.KeyboardEvent _simulateKeydown(html.Element target) { +html.KeyboardEvent _simulateKeydown(html.Element target, {bool? metaKey}) { final html.KeyboardEvent keydownEvent = html.KeyboardEvent( 'keydown', html.KeyboardEventInit() ..bubbles = true - ..cancelable = true, + ..cancelable = true + ..metaKey = metaKey ?? false, ); LinkViewController.handleGlobalKeydown(event: keydownEvent); return keydownEvent; From c1556cc6eaf5bfa7f088626f894611f8ac5b9de6 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Fri, 31 May 2024 14:04:08 -0400 Subject: [PATCH 05/22] semantics tests --- .../integration_test/link_widget_test.dart | 332 ++++++++++++++++++ .../url_launcher_web/lib/src/link.dart | 10 +- 2 files changed, 338 insertions(+), 4 deletions(-) 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 c3b26fa88a43..9e6395923e85 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 @@ -800,6 +800,338 @@ void main() { await tester.pumpAndSettle(); }); }); + + group('Link semantics', () { + late TestUrlLauncherPlugin testPlugin; + late UrlLauncherPlatform originalPlugin; + + setUp(() { + originalPlugin = UrlLauncherPlatform.instance; + testPlugin = TestUrlLauncherPlugin(); + UrlLauncherPlatform.instance = testPlugin; + }); + + tearDown(() { + UrlLauncherPlatform.instance = originalPlugin; + }); + + testWidgets('produces the correct semantics tree with a button', + (WidgetTester tester) async { + final SemanticsHandle semanticsHandle = tester.ensureSemantics(); + final Key linkKey = UniqueKey(); + + await tester.pumpWidget(Directionality( + textDirection: TextDirection.ltr, + child: WebLinkDelegate( + key: linkKey, + TestLinkInfo( + uri: Uri.parse('https://foobar/example?q=1'), + target: LinkTarget.blank, + builder: (BuildContext context, FollowLink? followLink) { + return ElevatedButton( + onPressed: followLink, + child: const Text('Button Link Text'), + ); + }, + ), + ), + )); + + final Finder linkFinder = find.byKey(linkKey); + final WebLinkDelegateState linkState = tester.state(linkFinder); + final String semanticIdentifier = linkState.semanticIdentifier; + expect( + tester.getSemantics(find.descendant( + of: linkFinder, + matching: find.byType(Semantics).first, + )), + matchesSemantics( + isLink: true, + identifier: 'sem-id-$semanticIdentifier', + value: 'https://foobar/example?q=1', + children: [ + matchesSemantics( + hasTapAction: true, + hasEnabledState: true, + isEnabled: true, + isButton: true, + isFocusable: true, + label: 'Button Link Text', + ), + ], + ), + ); + + semanticsHandle.dispose(); + }); + + testWidgets('produces the correct semantics tree with text', + (WidgetTester tester) async { + final SemanticsHandle semanticsHandle = tester.ensureSemantics(); + final Key linkKey = UniqueKey(); + + await tester.pumpWidget(Directionality( + textDirection: TextDirection.ltr, + child: WebLinkDelegate( + key: linkKey, + TestLinkInfo( + uri: Uri.parse('https://foobar/example?q=1'), + target: LinkTarget.blank, + builder: (BuildContext context, FollowLink? followLink) { + return GestureDetector( + onTap: followLink, + child: const Text('Link Text'), + ); + }, + ), + ), + )); + + final Finder linkFinder = find.byKey(linkKey); + final WebLinkDelegateState linkState = tester.state(linkFinder); + final String semanticIdentifier = linkState.semanticIdentifier; + expect( + tester.getSemantics(find.descendant( + of: linkFinder, + matching: find.byType(Semantics), + )), + matchesSemantics( + isLink: true, + hasTapAction: true, + identifier: 'sem-id-$semanticIdentifier', + value: 'https://foobar/example?q=1', + label: 'Link Text', + ), + ); + + semanticsHandle.dispose(); + }); + + testWidgets('handles clicks on semantic link with a button', + (WidgetTester tester) async { + final Uri uri = Uri.parse('/foobar'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + routes: { + '/foobar': (BuildContext context) => const Text('Internal route'), + }, + home: WebLinkDelegate(TestLinkInfo( + uri: uri, + target: LinkTarget.blank, + builder: (BuildContext context, FollowLink? followLink) { + followLinkCallback = followLink; + return ElevatedButton( + onPressed: () {}, + child: const Text('My Button Link'), + ); + }, + )), + )); + // Platform view creation happens asynchronously. + await tester.pumpAndSettle(); + + final WebLinkDelegateState linkState = tester.state( + find.byType(WebLinkDelegate), + ); + final String semanticIdentifier = linkState.semanticIdentifier; + + 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('semantic-identifier', semanticIdentifier) + ..setAttribute('href', '/foobar'); + semanticsHost.append(semanticsAnchor); + final html.Element semanticsContainer = html.document.createElement('flt-semantics-container'); + semanticsAnchor.append(semanticsContainer); + final html.Element semanticsButton = html.document.createElement('flt-semantics') + ..setAttribute('role', 'button') + ..textContent = 'My Button Link'; + semanticsContainer.append(semanticsButton); + + expect(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + + await followLinkCallback!(); + // Click on the button (child of the anchor). + final html.Event event1 = _simulateClick(semanticsButton); + + expect(pushedRouteNames, ['/foobar']); + expect(testPlugin.launches, isEmpty); + expect(event1.defaultPrevented, isTrue); + pushedRouteNames.clear(); + + await followLinkCallback!(); + // Click on the anchor itself. + final html.Event event2 = _simulateClick(semanticsAnchor); + + expect(pushedRouteNames, ['/foobar']); + expect(testPlugin.launches, isEmpty); + expect(event2.defaultPrevented, isTrue); + + // Needed when testing on on Chrome98 headless in CI. + // See https://github.com/flutter/flutter/issues/121161 + await tester.pumpAndSettle(); + }); + + testWidgets('handles clicks on semantic link with text', + (WidgetTester tester) async { + final Uri uri = Uri.parse('/foobar'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + routes: { + '/foobar': (BuildContext context) => const Text('Internal route'), + }, + home: WebLinkDelegate(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(); + + final WebLinkDelegateState linkState = tester.state( + find.byType(WebLinkDelegate), + ); + final String semanticIdentifier = linkState.semanticIdentifier; + + 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('semantic-identifier', semanticIdentifier) + ..setAttribute('href', '/foobar') + ..textContent = 'My Text Link'; + semanticsHost.append(semanticsAnchor); + + expect(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + + await followLinkCallback!(); + final html.Event event = _simulateClick(semanticsAnchor); + + expect(pushedRouteNames, ['/foobar']); + expect(testPlugin.launches, isEmpty); + expect(event.defaultPrevented, isTrue); + + // Needed when testing on on Chrome98 headless in CI. + // See https://github.com/flutter/flutter/issues/121161 + await tester.pumpAndSettle(); + }); + + // 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', + (WidgetTester tester) async { + final Uri uri = Uri.parse('/foobar'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + routes: { + '/foobar': (BuildContext context) => const Text('Internal route'), + }, + home: 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(); + + 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('href', '#'); + semanticsHost.append(semanticsAnchor); + final html.Element semanticsContainer = html.document.createElement('flt-semantics-container'); + semanticsAnchor.append(semanticsContainer); + final html.Element semanticsButton = html.document.createElement('flt-semantics') + ..setAttribute('role', 'button') + ..textContent = 'My Button'; + semanticsContainer.append(semanticsButton); + + expect(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + + await followLinkCallback!(); + final html.Event event1 = _simulateClick(semanticsButton); + + // Before the changes land in the web engine, this will not trigger the + // link properly. + expect(pushedRouteNames, []); + expect(testPlugin.launches, isEmpty); + expect(event1.defaultPrevented, isFalse); + + // Needed when testing on on Chrome98 headless in CI. + // See https://github.com/flutter/flutter/issues/121161 + await tester.pumpAndSettle(); + }); + + // 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 text', + (WidgetTester tester) async { + final Uri uri = Uri.parse('/foobar'); + FollowLink? followLinkCallback; + + await tester.pumpWidget(MaterialApp( + routes: { + '/foobar': (BuildContext context) => const Text('Internal route'), + }, + home: WebLinkDelegate(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(); + + 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('href', '#') + ..textContent = 'My Text Link'; + semanticsHost.append(semanticsAnchor); + + expect(pushedRouteNames, isEmpty); + expect(testPlugin.launches, isEmpty); + + await followLinkCallback!(); + final html.Event event = _simulateClick(semanticsAnchor); + + // Before the changes land in the web engine, this will not trigger the + // link properly. + expect(pushedRouteNames, []); + expect(testPlugin.launches, isEmpty); + expect(event.defaultPrevented, isFalse); + + // Needed when testing on on Chrome98 headless in CI. + // See https://github.com/flutter/flutter/issues/121161 + await tester.pumpAndSettle(); + }); + }); } List _findAllAnchors() { 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 7ddc7e9c76fc..57c0b7340929 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -73,12 +73,14 @@ int _nextSemanticsIdentifier = 0; /// pushes the route name to the framework. class WebLinkDelegateState extends State { late LinkViewController _controller; - late final String _semanticIdentifier; + + @visibleForTesting + late final String semanticIdentifier; @override void initState() { super.initState(); - _semanticIdentifier = 'sem-id-${_nextSemanticsIdentifier++}'; + semanticIdentifier = 'sem-id-${_nextSemanticsIdentifier++}'; } @override @@ -104,7 +106,7 @@ class WebLinkDelegateState extends State { children: [ Semantics( link: true, - identifier: _semanticIdentifier, + identifier: semanticIdentifier, value: widget.link.uri?.getHref(), child: widget.link.builder( context, @@ -117,7 +119,7 @@ class WebLinkDelegateState extends State { child: PlatformViewLink( viewType: linkViewType, onCreatePlatformView: (PlatformViewCreationParams params) { - _controller = LinkViewController.fromParams(params, _semanticIdentifier); + _controller = LinkViewController.fromParams(params, semanticIdentifier); return _controller ..setUri(widget.link.uri) ..setTarget(widget.link.target); From d98eab2de6fa619ef54e8028b94840b8927c27cc Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Fri, 31 May 2024 14:04:33 -0400 Subject: [PATCH 06/22] apply the target attribute on semantic links --- .../url_launcher_web/lib/src/link.dart | 42 +++++++++++++------ 1 file changed, 29 insertions(+), 13 deletions(-) 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 57c0b7340929..ea4bf1fb2d3b 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -512,12 +512,16 @@ class LinkViewController extends PlatformViewController { } } + late LinkTarget _target; + String get _htmlTargetAttribute => _getHtmlTargetAttribute(_target); + /// Set the [LinkTarget] value for this link. void setTarget(LinkTarget target) { - _element.setAttribute('target', _getHtmlTarget(target)); + _target = target; + _element.setAttribute('target', _htmlTargetAttribute); } - String _getHtmlTarget(LinkTarget target) { + String _getHtmlTargetAttribute(LinkTarget target) { switch (target) { case LinkTarget.defaultTarget: case LinkTarget.self: @@ -537,18 +541,30 @@ class LinkViewController extends PlatformViewController { /// /// Returns null if [target] is not a semantics element for one of our Links. static int? _getViewIdFromSemanticLink(html.Element? target) { - // TODO: what if `target` IS the semantic element? - if (target != null && _isWithinSemanticTree(target)) { - final html.Element? semanticLink = _getClosestSemanticLink(target); - if (semanticLink != null) { - // TODO: Find out the view ID of semantic link. - final String? semanticIdentifier = semanticLink.getAttribute('semantic-identifier'); - if (semanticIdentifier != null) { - return _instancesBySemanticIdentifier[semanticIdentifier]?.viewId; - } - } + if (target == null) { + return null; + } + if (!_isWithinSemanticTree(target)) { + return null; + } + + final html.Element? semanticLink = _getClosestSemanticLink(target); + if (semanticLink == null) { + return null; } - return null; + + final String? semanticIdentifier = semanticLink.getAttribute('semantic-identifier'); + if (semanticIdentifier == null) { + return null; + } + + final LinkViewController? controller = _instancesBySemanticIdentifier[semanticIdentifier]; + if (controller == null) { + return null; + } + + semanticLink.setAttribute('target', controller._htmlTargetAttribute); + return controller.viewId; } @override From 66df2e25ab69fac1d59223b5cec78329a6c35679 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Mon, 3 Jun 2024 10:56:51 -0400 Subject: [PATCH 07/22] always use plural 'semantics' --- .../integration_test/link_widget_test.dart | 16 +++--- .../url_launcher_web/lib/src/link.dart | 52 +++++++++---------- 2 files changed, 34 insertions(+), 34 deletions(-) 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 9e6395923e85..b602e2448f85 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 @@ -839,7 +839,7 @@ void main() { final Finder linkFinder = find.byKey(linkKey); final WebLinkDelegateState linkState = tester.state(linkFinder); - final String semanticIdentifier = linkState.semanticIdentifier; + final String semanticsIdentifier = linkState.semanticsIdentifier; expect( tester.getSemantics(find.descendant( of: linkFinder, @@ -847,7 +847,7 @@ void main() { )), matchesSemantics( isLink: true, - identifier: 'sem-id-$semanticIdentifier', + identifier: 'sem-id-$semanticsIdentifier', value: 'https://foobar/example?q=1', children: [ matchesSemantics( @@ -889,7 +889,7 @@ void main() { final Finder linkFinder = find.byKey(linkKey); final WebLinkDelegateState linkState = tester.state(linkFinder); - final String semanticIdentifier = linkState.semanticIdentifier; + final String semanticsIdentifier = linkState.semanticsIdentifier; expect( tester.getSemantics(find.descendant( of: linkFinder, @@ -898,7 +898,7 @@ void main() { matchesSemantics( isLink: true, hasTapAction: true, - identifier: 'sem-id-$semanticIdentifier', + identifier: 'sem-id-$semanticsIdentifier', value: 'https://foobar/example?q=1', label: 'Link Text', ), @@ -934,13 +934,13 @@ void main() { final WebLinkDelegateState linkState = tester.state( find.byType(WebLinkDelegate), ); - final String semanticIdentifier = linkState.semanticIdentifier; + final String semanticsIdentifier = linkState.semanticsIdentifier; 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('semantic-identifier', semanticIdentifier) + ..setAttribute('semantic-identifier', semanticsIdentifier) ..setAttribute('href', '/foobar'); semanticsHost.append(semanticsAnchor); final html.Element semanticsContainer = html.document.createElement('flt-semantics-container'); @@ -1002,13 +1002,13 @@ void main() { final WebLinkDelegateState linkState = tester.state( find.byType(WebLinkDelegate), ); - final String semanticIdentifier = linkState.semanticIdentifier; + final String semanticsIdentifier = linkState.semanticsIdentifier; 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('semantic-identifier', semanticIdentifier) + ..setAttribute('semantic-identifier', semanticsIdentifier) ..setAttribute('href', '/foobar') ..textContent = 'My Text Link'; semanticsHost.append(semanticsAnchor); 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 ea4bf1fb2d3b..804fd8505d0c 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -75,12 +75,12 @@ class WebLinkDelegateState extends State { late LinkViewController _controller; @visibleForTesting - late final String semanticIdentifier; + late final String semanticsIdentifier; @override void initState() { super.initState(); - semanticIdentifier = 'sem-id-${_nextSemanticsIdentifier++}'; + semanticsIdentifier = 'sem-id-${_nextSemanticsIdentifier++}'; } @override @@ -106,7 +106,7 @@ class WebLinkDelegateState extends State { children: [ Semantics( link: true, - identifier: semanticIdentifier, + identifier: semanticsIdentifier, value: widget.link.uri?.getHref(), child: widget.link.builder( context, @@ -119,7 +119,7 @@ class WebLinkDelegateState extends State { child: PlatformViewLink( viewType: linkViewType, onCreatePlatformView: (PlatformViewCreationParams params) { - _controller = LinkViewController.fromParams(params, semanticIdentifier); + _controller = LinkViewController.fromParams(params, semanticsIdentifier); return _controller ..setUri(widget.link.uri) ..setTarget(widget.link.target); @@ -229,24 +229,24 @@ class LinkTriggerSignals { /// Controls link views. class LinkViewController extends PlatformViewController { /// Creates a [LinkViewController] instance with the unique [viewId]. - LinkViewController(this.viewId, this._semanticIdentifier) { + LinkViewController(this.viewId, this._semanticsIdentifier) { if (_instancesByViewId.isEmpty) { // This is the first controller being created, attach the global click // listener. _attachGlobalListeners(); } _instancesByViewId[viewId] = this; - _instancesBySemanticIdentifier[_semanticIdentifier] = this; + _instancesBySemanticsIdentifier[_semanticsIdentifier] = this; } /// Creates and initializes a [LinkViewController] instance with the given /// platform view [params]. factory LinkViewController.fromParams( PlatformViewCreationParams params, - String semanticIdentifier, + String semanticsIdentifier, ) { final int viewId = params.id; - final LinkViewController controller = LinkViewController(viewId, semanticIdentifier); + final LinkViewController controller = LinkViewController(viewId, semanticsIdentifier); controller._initialize().then((_) { /// Because _initialize is async, it can happen that [LinkViewController.dispose] /// may get called before this `then` callback. @@ -261,7 +261,7 @@ class LinkViewController extends PlatformViewController { static final Map _instancesByViewId = {}; - static final Map _instancesBySemanticIdentifier = + static final Map _instancesBySemanticsIdentifier = {}; static html.Element _viewFactory(int viewId) { @@ -379,7 +379,7 @@ class LinkViewController extends PlatformViewController { // We only want to handle clicks that land on *our* links, whether that's a // platform view link or a semantics link. final int? viewIdFromTarget = _getViewIdFromLink(target) ?? - _getViewIdFromSemanticLink(target); + _getViewIdFromSemanticsLink(target); if (viewIdFromTarget == null) { // The click target was not one of our links, so we don't want to @@ -434,7 +434,7 @@ class LinkViewController extends PlatformViewController { @override final int viewId; - final String _semanticIdentifier; + final String _semanticsIdentifier; late html.HTMLElement _element; @@ -540,30 +540,30 @@ class LinkViewController extends PlatformViewController { /// Finds the view ID in the Link's semantic element. /// /// Returns null if [target] is not a semantics element for one of our Links. - static int? _getViewIdFromSemanticLink(html.Element? target) { + static int? _getViewIdFromSemanticsLink(html.Element? target) { if (target == null) { return null; } - if (!_isWithinSemanticTree(target)) { + if (!_isWithinSemanticsTree(target)) { return null; } - final html.Element? semanticLink = _getClosestSemanticLink(target); - if (semanticLink == null) { + final html.Element? semanticsLink = _getClosestSemanticsLink(target); + if (semanticsLink == null) { return null; } - final String? semanticIdentifier = semanticLink.getAttribute('semantic-identifier'); - if (semanticIdentifier == null) { + final String? semanticsIdentifier = semanticsLink.getAttribute('semantic-identifier'); + if (semanticsIdentifier == null) { return null; } - final LinkViewController? controller = _instancesBySemanticIdentifier[semanticIdentifier]; + final LinkViewController? controller = _instancesBySemanticsIdentifier[semanticsIdentifier]; if (controller == null) { return null; } - semanticLink.setAttribute('target', controller._htmlTargetAttribute); + semanticsLink.setAttribute('target', controller._htmlTargetAttribute); return controller.viewId; } @@ -582,10 +582,10 @@ class LinkViewController extends PlatformViewController { @override Future dispose() async { assert(_instancesByViewId[viewId] == this); - assert(_instancesBySemanticIdentifier[_semanticIdentifier] == this); + assert(_instancesBySemanticsIdentifier[_semanticsIdentifier] == this); _instancesByViewId.remove(viewId); - _instancesBySemanticIdentifier.remove(_semanticIdentifier); + _instancesBySemanticsIdentifier.remove(_semanticsIdentifier); if (_instancesByViewId.isEmpty) { _detachGlobalListeners(); @@ -613,16 +613,16 @@ int? _getViewIdFromLink(html.Element? target) { return null; } -/// Whether [element] is within the semantic tree of a Flutter View. -bool _isWithinSemanticTree(html.Element element) { +/// Whether [element] is within the semantics tree of a Flutter View. +bool _isWithinSemanticsTree(html.Element element) { return element.closest('flt-semantics-host') != null; } -/// Returns the closest semantic link ancestor of the given [element]. +/// Returns the closest semantics link ancestor of the given [element]. /// /// If [element] itself is a link, it is returned. -html.Element? _getClosestSemanticLink(html.Element element) { - assert(_isWithinSemanticTree(element)); +html.Element? _getClosestSemanticsLink(html.Element element) { + assert(_isWithinSemanticsTree(element)); return element.closest('a[id^="flt-semantic-node-"]'); } From c359fd48944100e995110cd6aa8c190d5828cfea Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Mon, 3 Jun 2024 11:04:39 -0400 Subject: [PATCH 08/22] changes to semanticsIdentifier --- .../integration_test/link_widget_test.dart | 74 +++++++++---------- .../url_launcher_web/lib/src/link.dart | 14 ++-- 2 files changed, 42 insertions(+), 46 deletions(-) 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 b602e2448f85..bae6f6e6ba5a 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 @@ -824,6 +824,7 @@ void main() { textDirection: TextDirection.ltr, child: WebLinkDelegate( key: linkKey, + semanticsIdentifier: 'test-link-12', TestLinkInfo( uri: Uri.parse('https://foobar/example?q=1'), target: LinkTarget.blank, @@ -838,8 +839,6 @@ void main() { )); final Finder linkFinder = find.byKey(linkKey); - final WebLinkDelegateState linkState = tester.state(linkFinder); - final String semanticsIdentifier = linkState.semanticsIdentifier; expect( tester.getSemantics(find.descendant( of: linkFinder, @@ -847,7 +846,7 @@ void main() { )), matchesSemantics( isLink: true, - identifier: 'sem-id-$semanticsIdentifier', + identifier: 'test-link-12', value: 'https://foobar/example?q=1', children: [ matchesSemantics( @@ -874,6 +873,7 @@ void main() { textDirection: TextDirection.ltr, child: WebLinkDelegate( key: linkKey, + semanticsIdentifier: 'test-link-43', TestLinkInfo( uri: Uri.parse('https://foobar/example?q=1'), target: LinkTarget.blank, @@ -888,8 +888,6 @@ void main() { )); final Finder linkFinder = find.byKey(linkKey); - final WebLinkDelegateState linkState = tester.state(linkFinder); - final String semanticsIdentifier = linkState.semanticsIdentifier; expect( tester.getSemantics(find.descendant( of: linkFinder, @@ -898,7 +896,7 @@ void main() { matchesSemantics( isLink: true, hasTapAction: true, - identifier: 'sem-id-$semanticsIdentifier', + identifier: 'test-link-43', value: 'https://foobar/example?q=1', label: 'Link Text', ), @@ -916,31 +914,29 @@ void main() { routes: { '/foobar': (BuildContext context) => const Text('Internal route'), }, - home: WebLinkDelegate(TestLinkInfo( - uri: uri, - target: LinkTarget.blank, - builder: (BuildContext context, FollowLink? followLink) { - followLinkCallback = followLink; - return ElevatedButton( - onPressed: () {}, - child: const Text('My Button Link'), - ); - }, - )), + home: WebLinkDelegate( + semanticsIdentifier: 'test-link-27', + TestLinkInfo( + uri: uri, + target: LinkTarget.blank, + builder: (BuildContext context, FollowLink? followLink) { + followLinkCallback = followLink; + return ElevatedButton( + onPressed: () {}, + child: const Text('My Button Link'), + ); + }, + ), + ), )); // Platform view creation happens asynchronously. await tester.pumpAndSettle(); - final WebLinkDelegateState linkState = tester.state( - find.byType(WebLinkDelegate), - ); - final String semanticsIdentifier = linkState.semanticsIdentifier; - 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('semantic-identifier', semanticsIdentifier) + ..setAttribute('semantic-identifier', 'test-link-27') ..setAttribute('href', '/foobar'); semanticsHost.append(semanticsAnchor); final html.Element semanticsContainer = html.document.createElement('flt-semantics-container'); @@ -984,31 +980,29 @@ void main() { routes: { '/foobar': (BuildContext context) => const Text('Internal route'), }, - home: WebLinkDelegate(TestLinkInfo( - uri: uri, - target: LinkTarget.blank, - builder: (BuildContext context, FollowLink? followLink) { - followLinkCallback = followLink; - return GestureDetector( - onTap: () {}, - child: const Text('My Link'), - ); - }, - )), + 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(); - final WebLinkDelegateState linkState = tester.state( - find.byType(WebLinkDelegate), - ); - final String semanticsIdentifier = linkState.semanticsIdentifier; - 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('semantic-identifier', semanticsIdentifier) + ..setAttribute('semantic-identifier', 'test-link-71') ..setAttribute('href', '/foobar') ..textContent = 'My Text Link'; semanticsHost.append(semanticsAnchor); 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 804fd8505d0c..98555c85c3b7 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -38,11 +38,13 @@ Future Function(String) pushRouteToFrameworkFunction = /// It uses a platform view to render an anchor element in the DOM. class WebLinkDelegate extends StatefulWidget { /// Creates a delegate for the given [link]. - const WebLinkDelegate(this.link, {super.key}); + const WebLinkDelegate(this.link, {super.key, this.semanticsIdentifier}); /// Information about the link built by the app. final LinkInfo link; + final String? semanticsIdentifier; + @override WebLinkDelegateState createState() => WebLinkDelegateState(); } @@ -74,13 +76,13 @@ int _nextSemanticsIdentifier = 0; class WebLinkDelegateState extends State { late LinkViewController _controller; - @visibleForTesting - late final String semanticsIdentifier; + late final String _semanticsIdentifier; @override void initState() { super.initState(); - semanticsIdentifier = 'sem-id-${_nextSemanticsIdentifier++}'; + _semanticsIdentifier = + widget.semanticsIdentifier ?? 'link-${_nextSemanticsIdentifier++}'; } @override @@ -106,7 +108,7 @@ class WebLinkDelegateState extends State { children: [ Semantics( link: true, - identifier: semanticsIdentifier, + identifier: _semanticsIdentifier, value: widget.link.uri?.getHref(), child: widget.link.builder( context, @@ -119,7 +121,7 @@ class WebLinkDelegateState extends State { child: PlatformViewLink( viewType: linkViewType, onCreatePlatformView: (PlatformViewCreationParams params) { - _controller = LinkViewController.fromParams(params, semanticsIdentifier); + _controller = LinkViewController.fromParams(params, _semanticsIdentifier); return _controller ..setUri(widget.link.uri) ..setTarget(widget.link.target); From 7fd5b7b06ea35842d908e5dea64f2d1c3c2c1290 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 4 Jun 2024 10:48:29 -0400 Subject: [PATCH 09/22] more attributes on --- packages/url_launcher/url_launcher_web/lib/src/link.dart | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 98555c85c3b7..b42630972f67 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -337,7 +337,7 @@ class LinkViewController extends PlatformViewController { // 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`. + // - The button widget calls `onPressed` and therefore `followLink`. // - `followLink` calls `LinkViewController.registerHitTest`. // - `LinkViewController.registerHitTest` sets `_hitTestedViewId`. // 4. The `LinkViewController` also receives the keydown event: @@ -454,6 +454,9 @@ class LinkViewController extends PlatformViewController { // - https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-target _element.setAttribute('rel', 'noreferrer noopener'); + _element.setAttribute('aria-hidden', 'true'); + _element.setAttribute('tabIndex', '-1'); + final Map args = { 'id': viewId, 'viewType': linkViewType, From 80e7158fb3c1219eea97d715355e4d71655cbf37 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 4 Jun 2024 13:26:25 -0400 Subject: [PATCH 10/22] more readable --- .../url_launcher_web/lib/src/link.dart | 71 +++++++++++-------- 1 file changed, 40 insertions(+), 31 deletions(-) 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 b42630972f67..b319fffa8344 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -106,42 +106,51 @@ class WebLinkDelegateState extends State { return Stack( fit: StackFit.passthrough, children: [ - Semantics( - link: true, - identifier: _semanticsIdentifier, - value: widget.link.uri?.getHref(), - child: widget.link.builder( - context, - widget.link.isDisabled ? null : _followLink, - ), - ), + _buildChild(context), Positioned.fill( - child: ExcludeFocus( - child: ExcludeSemantics( - child: PlatformViewLink( - viewType: linkViewType, - onCreatePlatformView: (PlatformViewCreationParams params) { - _controller = LinkViewController.fromParams(params, _semanticsIdentifier); - return _controller - ..setUri(widget.link.uri) - ..setTarget(widget.link.target); - }, - surfaceFactory: - (BuildContext context, PlatformViewController controller) { - return PlatformViewSurface( - controller: controller, - gestureRecognizers: const >{}, - hitTestBehavior: PlatformViewHitTestBehavior.transparent, - ); - }, - ), - ), - ), + child: _buildPlatformView(context), ), ], ); } + + Widget _buildChild(BuildContext context) { + return Semantics( + link: true, + identifier: _semanticsIdentifier, + value: widget.link.uri?.getHref(), + child: widget.link.builder( + context, + widget.link.isDisabled ? null : _followLink, + ), + ); + } + + Widget _buildPlatformView(BuildContext context) { + return ExcludeFocus( + child: ExcludeSemantics( + child: PlatformViewLink( + viewType: linkViewType, + onCreatePlatformView: (PlatformViewCreationParams params) { + _controller = + LinkViewController.fromParams(params, _semanticsIdentifier); + return _controller + ..setUri(widget.link.uri) + ..setTarget(widget.link.target); + }, + surfaceFactory: + (BuildContext context, PlatformViewController controller) { + return PlatformViewSurface( + controller: controller, + gestureRecognizers: const >{}, + hitTestBehavior: PlatformViewHitTestBehavior.transparent, + ); + }, + ), + ), + ); + } } final JSAny _useCapture = {'capture': true}.jsify()!; From 9d021019e91defb517a35c27c5c90dfd212004cb Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 13 Aug 2024 10:02:25 -0400 Subject: [PATCH 11/22] correct attribute name 'flt-semantics-identifier' --- .../example/integration_test/link_widget_test.dart | 4 ++-- packages/url_launcher/url_launcher_web/lib/src/link.dart | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) 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 bae6f6e6ba5a..158016a29934 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 @@ -936,7 +936,7 @@ void main() { html.document.body!.append(semanticsHost); final html.Element semanticsAnchor = html.document.createElement('a') ..setAttribute('id', 'flt-semantic-node-99') - ..setAttribute('semantic-identifier', 'test-link-27') + ..setAttribute('flt-semantics-identifier', 'test-link-27') ..setAttribute('href', '/foobar'); semanticsHost.append(semanticsAnchor); final html.Element semanticsContainer = html.document.createElement('flt-semantics-container'); @@ -1002,7 +1002,7 @@ void main() { html.document.body!.append(semanticsHost); final html.Element semanticsAnchor = html.document.createElement('a') ..setAttribute('id', 'flt-semantic-node-99') - ..setAttribute('semantic-identifier', 'test-link-71') + ..setAttribute('flt-semantics-identifier', 'test-link-71') ..setAttribute('href', '/foobar') ..textContent = 'My Text Link'; semanticsHost.append(semanticsAnchor); 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 b319fffa8344..37fa4ecf934a 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -555,6 +555,8 @@ class LinkViewController extends PlatformViewController { /// /// Returns null if [target] is not a semantics element for one of our Links. static int? _getViewIdFromSemanticsLink(html.Element? target) { + // TODO: The whole could be inside a shadow root. In that case, + // the target is always the shadow root (because we are listening on window). if (target == null) { return null; } @@ -567,7 +569,7 @@ class LinkViewController extends PlatformViewController { return null; } - final String? semanticsIdentifier = semanticsLink.getAttribute('semantic-identifier'); + final String? semanticsIdentifier = semanticsLink.getAttribute('flt-semantics-identifier'); if (semanticsIdentifier == null) { return null; } From 53349224340b696c0ef67efd7f1c8a7c0001a395 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 13 Aug 2024 12:52:35 -0400 Subject: [PATCH 12/22] use the new linkUrl semantics property --- .../example/integration_test/link_widget_test.dart | 5 +++-- packages/url_launcher/url_launcher_web/lib/src/link.dart | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) 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 158016a29934..47498f1ea6fa 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 @@ -847,11 +847,12 @@ void main() { matchesSemantics( isLink: true, identifier: 'test-link-12', - value: 'https://foobar/example?q=1', + // linkUrl: 'https://foobar/example?q=1', children: [ matchesSemantics( hasTapAction: true, hasEnabledState: true, + hasFocusAction: true, isEnabled: true, isButton: true, isFocusable: true, @@ -897,7 +898,7 @@ void main() { isLink: true, hasTapAction: true, identifier: 'test-link-43', - value: 'https://foobar/example?q=1', + // linkUrl: 'https://foobar/example?q=1', label: 'Link Text', ), ); 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 37fa4ecf934a..8bcb89e18c4f 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -118,7 +118,7 @@ class WebLinkDelegateState extends State { return Semantics( link: true, identifier: _semanticsIdentifier, - value: widget.link.uri?.getHref(), + linkUrl: widget.link.uri, child: widget.link.builder( context, widget.link.isDisabled ? null : _followLink, From 99553602ff36c796b6aa824ef2b6cfb687c660f3 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 13 Aug 2024 13:15:47 -0400 Subject: [PATCH 13/22] triggerIfReady --- .../url_launcher_web/lib/src/link.dart | 103 +++++++++--------- 1 file changed, 49 insertions(+), 54 deletions(-) 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 8bcb89e18c4f..9d704121f793 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -155,12 +155,21 @@ class WebLinkDelegateState extends State { final JSAny _useCapture = {'capture': true}.jsify()!; +typedef _TriggerLinkCallback = void Function(int viewId, html.MouseEvent? mouseEvent); + /// Keeps track of the signals required to trigger a link. /// /// Automatically resets the signals after a certain delay. This is to prevent /// the signals from getting stale. class LinkTriggerSignals { - LinkTriggerSignals({required this.staleTimeout}); + LinkTriggerSignals({ + required this.triggerLink, + required this.staleTimeout, + }); + + /// The function to be called when all signals have been received and the link + /// is ready to be triggered. + final _TriggerLinkCallback triggerLink; /// Specifies the duration after which the signals are considered stale. /// @@ -168,12 +177,27 @@ class LinkTriggerSignals { /// considered valid. If they don't, the signals are reset. final Duration staleTimeout; - /// Whether the we got all the signals required to trigger the link. - bool get isReadyToTrigger => _hasFollowLink && _hasDomEvent; + /// Whether we got all the signals required to trigger the link. + bool get _isReadyToTrigger => _hasFollowLink && _hasDomEvent; - int? get viewId { - assert(isValid); - return _viewIdFromFollowLink; + int? get _viewId { + assert(!isViewIdMismatched); + return _viewIdFromFollowLink ?? _viewIdFromDomEvent; + } + + void triggerLinkIfReady() { + if (isViewIdMismatched) { + // When view IDs from signals are mismatched, let's reset the signals and + // prevent the browser from navigating. + _mouseEvent?.preventDefault(); + reset(); + return; + } + + if (_isReadyToTrigger) { + triggerLink(_viewId!, _mouseEvent); + reset(); + } } void registerFollowLink({required int viewId}) { @@ -191,7 +215,7 @@ class LinkTriggerSignals { } _hasDomEvent = true; _viewIdFromDomEvent = viewId; - this.mouseEvent = mouseEvent; + _mouseEvent = mouseEvent; _didUpdate(); } @@ -201,18 +225,18 @@ class LinkTriggerSignals { int? _viewIdFromFollowLink; int? _viewIdFromDomEvent; - html.MouseEvent? mouseEvent; + html.MouseEvent? _mouseEvent; - // The signals state is considered invalid if the view IDs from the follow - // link and the DOM event don't match. - bool get isValid { + /// Whether the view ID from various signals have a mismatch. + /// + /// When a signal's view ID is missing, it's not considered a mismatch. + bool get isViewIdMismatched { if (_viewIdFromFollowLink == null || _viewIdFromDomEvent == null) { - // We haven't received all view IDs yet, so we can't determine if the - // signals are valid. - return true; + // A missing view ID is not considered a mismatch. + return false; } - return _viewIdFromFollowLink == _viewIdFromDomEvent; + return _viewIdFromFollowLink != _viewIdFromDomEvent; } Timer? _resetTimer; @@ -233,7 +257,7 @@ class LinkTriggerSignals { _viewIdFromFollowLink = null; _viewIdFromDomEvent = null; - mouseEvent = null; + _mouseEvent = null; } } @@ -280,7 +304,10 @@ class LinkViewController extends PlatformViewController { } static final LinkTriggerSignals _triggerSignals = - LinkTriggerSignals(staleTimeout: const Duration(milliseconds: 500)); + LinkTriggerSignals( + triggerLink: _triggerLink, + staleTimeout: const Duration(milliseconds: 500), + ); static final JSFunction _jsGlobalKeydownListener = _onGlobalKeydown.toJS; static final JSFunction _jsGlobalClickListener = _onGlobalClick.toJS; @@ -362,10 +389,7 @@ class LinkViewController extends PlatformViewController { // The keydown event is not directly associated with the target Link, so // we can't find the `viewId` from the event. _triggerSignals.registerDomEvent(viewId: null, mouseEvent: null); - - if (_triggerSignals.isReadyToTrigger) { - _triggerLink(); - } + _triggerSignals.triggerLinkIfReady(); } static void _onGlobalClick(html.MouseEvent event) { @@ -406,18 +430,7 @@ class LinkViewController extends PlatformViewController { mouseEvent: event, ); - if (!_triggerSignals.isValid) { - // The view ID from the target doesn't match the view ID from the follow - // link. Let's prevent the browser from navigating, and let's reset the - // signals so we start fresh on the next event. - event.preventDefault(); - _triggerSignals.reset(); - return; - } - - if (_triggerSignals.isReadyToTrigger) { - _triggerLink(); - } + _triggerSignals.triggerLinkIfReady(); } /// Call this method to indicate that a hit test has been registered for the @@ -427,19 +440,7 @@ class LinkViewController extends PlatformViewController { /// `click` from the browser. static void onFollowLink(int viewId) { _triggerSignals.registerFollowLink(viewId: viewId); - - if (!_triggerSignals.isValid) { - // The view ID from follow link doesn't match the view ID from the event - // target. Let's prevent the browser from navigating, and let's reset the - // signals so we start fresh on the next event. - _triggerSignals.mouseEvent?.preventDefault(); - _triggerSignals.reset(); - return; - } - - if (_triggerSignals.isReadyToTrigger) { - _triggerLink(); - } + _triggerSignals.triggerLinkIfReady(); } @override @@ -477,14 +478,8 @@ class LinkViewController extends PlatformViewController { /// /// It also handles logic for external vs internal links, triggered by a mouse /// vs keyboard event. - static void _triggerLink() { - assert(_triggerSignals.isReadyToTrigger); - - final LinkViewController controller = _instancesByViewId[_triggerSignals.viewId!]!; - final html.MouseEvent? mouseEvent = _triggerSignals.mouseEvent; - - // Make sure to reset no matter what code path we end up taking. - _triggerSignals.reset(); + static void _triggerLink(int viewId, html.MouseEvent? mouseEvent) { + final LinkViewController controller = _instancesByViewId[viewId]!; if (mouseEvent != null && _isModifierKey(mouseEvent)) { return; From af67dfcb3c3ad51b6b138aa71d6636ae905df75f Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 13 Aug 2024 13:30:17 -0400 Subject: [PATCH 14/22] docs --- .../url_launcher_web/lib/src/link.dart | 81 +++++++++++++------ 1 file changed, 57 insertions(+), 24 deletions(-) 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 9d704121f793..56800fcd4f27 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -43,6 +43,10 @@ class WebLinkDelegate extends StatefulWidget { /// Information about the link built by the app. final LinkInfo link; + /// A user-provided identifier to be as the [SemanticsProperties.identifier] + /// for the link. + /// + /// This identifier is optional and is only useful for testing purposes. final String? semanticsIdentifier; @override @@ -155,13 +159,30 @@ class WebLinkDelegateState extends State { final JSAny _useCapture = {'capture': true}.jsify()!; -typedef _TriggerLinkCallback = void Function(int viewId, html.MouseEvent? mouseEvent); +/// Signature for the function that triggers a link. +typedef TriggerLinkCallback = void Function(int viewId, html.MouseEvent? mouseEvent); /// Keeps track of the signals required to trigger a link. /// -/// Automatically resets the signals after a certain delay. This is to prevent -/// the signals from getting stale. +/// Currently, the signals required are: +/// +/// 1. A FollowLink signal. This signal indicates that a hit test for the link +/// was registered on the app/framework side. +/// +/// 2. A DOM event signal. This signal indicates that a click or keyboard event +/// was received on the link's corresponding DOM element. +/// +/// These signals can arrive at any order depending on how the user triggers +/// the link. +/// +/// Each signal may be accompanied by a view ID. The view IDs, when present, +/// must match in order for the trigger to be considered valid. +/// +/// The signals are reset after a certain delay to prevent them from getting +/// stale. The delay is specified by [staleTimeout]. class LinkTriggerSignals { + /// Creates a [LinkTriggerSignals] instance that calls [triggerLink] when all + /// the signals are received within a [staleTimeout] duration. LinkTriggerSignals({ required this.triggerLink, required this.staleTimeout, @@ -169,7 +190,7 @@ class LinkTriggerSignals { /// The function to be called when all signals have been received and the link /// is ready to be triggered. - final _TriggerLinkCallback triggerLink; + final TriggerLinkCallback triggerLink; /// Specifies the duration after which the signals are considered stale. /// @@ -177,14 +198,20 @@ class LinkTriggerSignals { /// considered valid. If they don't, the signals are reset. final Duration staleTimeout; - /// Whether we got all the signals required to trigger the link. - bool get _isReadyToTrigger => _hasFollowLink && _hasDomEvent; + bool get _hasAllSignals => _hasFollowLink && _hasDomEvent; int? get _viewId { assert(!isViewIdMismatched); return _viewIdFromFollowLink ?? _viewIdFromDomEvent; } + /// Triggers the link if all signals are ready and there's no view ID + /// mismatch. + /// + /// If the view IDs from the signals are mismatched, the signals are reset and + /// the browser is prevented from navigating. + /// + /// If the signals are ready, the link is triggered and the signals are reset. void triggerLinkIfReady() { if (isViewIdMismatched) { // When view IDs from signals are mismatched, let's reset the signals and @@ -194,18 +221,26 @@ class LinkTriggerSignals { return; } - if (_isReadyToTrigger) { + if (_hasAllSignals) { triggerLink(_viewId!, _mouseEvent); reset(); } } + /// Registers a FollowLink signal with the given [viewId]. void registerFollowLink({required int viewId}) { _hasFollowLink = true; _viewIdFromFollowLink = viewId; _didUpdate(); } + /// Registers a DOM event signal with the given [viewId] and [mouseEvent]. + /// + /// The [viewId] is optional because it cannot be determined from some DOM + /// events. + /// + /// The [mouseEvent] is optional because the signal may be from a keyboard + /// event. void registerDomEvent({ required int? viewId, required html.MouseEvent? mouseEvent, @@ -335,6 +370,9 @@ class LinkViewController extends PlatformViewController { handleGlobalKeydown(event: event); } + /// Global keydown handler that's called for every keydown event on the + /// window. + @visibleForTesting static void handleGlobalKeydown({required html.KeyboardEvent event}) { // Why not use `event.target`? // @@ -399,19 +437,13 @@ class LinkViewController extends PlatformViewController { ); } - /// Global click handler that triggers on the `capture` phase. We use `capture` - /// because some events may be consumed and prevent further propagation at the - /// target. This may lead to issues (see: https://github.com/flutter/flutter/issues/143164) - /// where a followLink was executed but the event never bubbles back up to the - /// window (e.g. when button semantics obscure the platform view). We make sure - /// to only trigger the link if a hit test was registered and remains valid at - /// the time the click handler executes. + /// Global click handler that's called for every click event on the window. @visibleForTesting static void handleGlobalClick({ required html.MouseEvent event, required html.Element? target, }) { - // We only want to handle clicks that land on *our* links, whether that's a + // We only want to handle clicks that land on *our* links. That could be a // platform view link or a semantics link. final int? viewIdFromTarget = _getViewIdFromLink(target) ?? _getViewIdFromSemanticsLink(target); @@ -420,7 +452,8 @@ class LinkViewController extends PlatformViewController { // The click target was not one of our links, so we don't want to // interfere with it. // - // We also want to reset the signals in this case. + // We also want to reset the signals to make sure we start fresh on the + // next click. _triggerSignals.reset(); return; } @@ -434,10 +467,7 @@ class LinkViewController extends PlatformViewController { } /// Call this method to indicate that a hit test has been registered for the - /// given [controller]. - /// - /// The [onClick] callback is invoked when the anchor element receives a - /// `click` from the browser. + /// given [viewId]. static void onFollowLink(int viewId) { _triggerSignals.registerFollowLink(viewId: viewId); _triggerSignals.triggerLinkIfReady(); @@ -482,12 +512,15 @@ class LinkViewController extends PlatformViewController { final LinkViewController controller = _instancesByViewId[viewId]!; if (mouseEvent != null && _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 trigger by keyboard, they are not handled by + // 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()); @@ -498,9 +531,8 @@ class LinkViewController extends PlatformViewController { 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 letting the - // browser handle it. + // Internal links are pushed through Flutter's navigation system instead of + // letting the browser handle it. mouseEvent?.preventDefault(); final String routeName = controller._uri.toString(); pushRouteToFrameworkFunction(routeName); @@ -604,6 +636,7 @@ class LinkViewController extends PlatformViewController { await SystemChannels.platform_views.invokeMethod('dispose', viewId); } + /// Resets all link-related state. @visibleForTesting static Future debugReset() async { _triggerSignals.reset(); From aa0c3d784e2658edf7af50f41efe24b5b8f65061 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Mon, 23 Dec 2024 10:52:09 -0500 Subject: [PATCH 15/22] bump minimum sdk version --- packages/url_launcher/url_launcher_web/example/pubspec.yaml | 2 +- packages/url_launcher/url_launcher_web/pubspec.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/url_launcher/url_launcher_web/example/pubspec.yaml b/packages/url_launcher/url_launcher_web/example/pubspec.yaml index a70f27951b9e..1b851677ca60 100644 --- a/packages/url_launcher/url_launcher_web/example/pubspec.yaml +++ b/packages/url_launcher/url_launcher_web/example/pubspec.yaml @@ -3,7 +3,7 @@ publish_to: none environment: sdk: ^3.4.0 - flutter: ">=3.22.0" + flutter: ">=3.27.0" dependencies: flutter: diff --git a/packages/url_launcher/url_launcher_web/pubspec.yaml b/packages/url_launcher/url_launcher_web/pubspec.yaml index 7b9068d19153..8ccff7f23349 100644 --- a/packages/url_launcher/url_launcher_web/pubspec.yaml +++ b/packages/url_launcher/url_launcher_web/pubspec.yaml @@ -6,7 +6,7 @@ version: 2.3.3 environment: sdk: ^3.4.0 - flutter: ">=3.22.0" + flutter: ">=3.27.0" flutter: plugin: From f296c493d62940cdc1eecf9472d04b34da1c13a2 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Fri, 27 Dec 2024 15:11:54 -0500 Subject: [PATCH 16/22] dart format --- .../integration_test/link_widget_test.dart | 32 ++++++++++++------- .../url_launcher_web/lib/src/link.dart | 25 ++++++++------- 2 files changed, 34 insertions(+), 23 deletions(-) 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 2d5a63cb5832..9b0004cc9191 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 @@ -933,18 +933,21 @@ void main() { // Platform view creation happens asynchronously. await tester.pumpAndSettle(); - final html.Element semanticsHost = html.document.createElement('flt-semantics-host'); + 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-27') ..setAttribute('href', '/foobar'); semanticsHost.append(semanticsAnchor); - final html.Element semanticsContainer = html.document.createElement('flt-semantics-container'); + final html.Element semanticsContainer = + html.document.createElement('flt-semantics-container'); semanticsAnchor.append(semanticsContainer); - final html.Element semanticsButton = html.document.createElement('flt-semantics') - ..setAttribute('role', 'button') - ..textContent = 'My Button Link'; + final html.Element semanticsButton = + html.document.createElement('flt-semantics') + ..setAttribute('role', 'button') + ..textContent = 'My Button Link'; semanticsContainer.append(semanticsButton); expect(pushedRouteNames, isEmpty); @@ -999,7 +1002,8 @@ void main() { // Platform view creation happens asynchronously. await tester.pumpAndSettle(); - final html.Element semanticsHost = html.document.createElement('flt-semantics-host'); + 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') @@ -1046,17 +1050,20 @@ void main() { // Platform view creation happens asynchronously. await tester.pumpAndSettle(); - final html.Element semanticsHost = html.document.createElement('flt-semantics-host'); + 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('href', '#'); semanticsHost.append(semanticsAnchor); - final html.Element semanticsContainer = html.document.createElement('flt-semantics-container'); + final html.Element semanticsContainer = + html.document.createElement('flt-semantics-container'); semanticsAnchor.append(semanticsContainer); - final html.Element semanticsButton = html.document.createElement('flt-semantics') - ..setAttribute('role', 'button') - ..textContent = 'My Button'; + final html.Element semanticsButton = + html.document.createElement('flt-semantics') + ..setAttribute('role', 'button') + ..textContent = 'My Button'; semanticsContainer.append(semanticsButton); expect(pushedRouteNames, isEmpty); @@ -1102,7 +1109,8 @@ void main() { // Platform view creation happens asynchronously. await tester.pumpAndSettle(); - final html.Element semanticsHost = html.document.createElement('flt-semantics-host'); + 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') 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 6a0a0aa935eb..7f9c2fa13af9 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -160,7 +160,8 @@ class WebLinkDelegateState extends State { final JSAny _useCapture = {'capture': true}.jsify()!; /// Signature for the function that triggers a link. -typedef TriggerLinkCallback = void Function(int viewId, html.MouseEvent? mouseEvent); +typedef TriggerLinkCallback = void Function( + int viewId, html.MouseEvent? mouseEvent); /// Keeps track of the signals required to trigger a link. /// @@ -316,7 +317,8 @@ class LinkViewController extends PlatformViewController { String semanticsIdentifier, ) { final int viewId = params.id; - final LinkViewController controller = LinkViewController(viewId, semanticsIdentifier); + final LinkViewController controller = + LinkViewController(viewId, semanticsIdentifier); controller._initialize().then((_) { /// Because _initialize is async, it can happen that [LinkViewController.dispose] /// may get called before this `then` callback. @@ -338,11 +340,10 @@ class LinkViewController extends PlatformViewController { return _instancesByViewId[viewId]!._element; } - static final LinkTriggerSignals _triggerSignals = - LinkTriggerSignals( - triggerLink: _triggerLink, - staleTimeout: const Duration(milliseconds: 500), - ); + static final LinkTriggerSignals _triggerSignals = LinkTriggerSignals( + triggerLink: _triggerLink, + staleTimeout: const Duration(milliseconds: 500), + ); static final JSFunction _jsGlobalKeydownListener = _onGlobalKeydown.toJS; static final JSFunction _jsGlobalClickListener = _onGlobalClick.toJS; @@ -445,8 +446,8 @@ class LinkViewController extends PlatformViewController { }) { // We only want to handle clicks that land on *our* links. That could be a // platform view link or a semantics link. - final int? viewIdFromTarget = _getViewIdFromLink(target) ?? - _getViewIdFromSemanticsLink(target); + final int? viewIdFromTarget = + _getViewIdFromLink(target) ?? _getViewIdFromSemanticsLink(target); if (viewIdFromTarget == null) { // The click target was not one of our links, so we don't want to @@ -596,12 +597,14 @@ class LinkViewController extends PlatformViewController { return null; } - final String? semanticsIdentifier = semanticsLink.getAttribute('flt-semantics-identifier'); + final String? semanticsIdentifier = + semanticsLink.getAttribute('flt-semantics-identifier'); if (semanticsIdentifier == null) { return null; } - final LinkViewController? controller = _instancesBySemanticsIdentifier[semanticsIdentifier]; + final LinkViewController? controller = + _instancesBySemanticsIdentifier[semanticsIdentifier]; if (controller == null) { return null; } From 114731d22374e2ce2d2f030ef8e643ad7003c6be Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Wed, 8 Jan 2025 15:48:55 -0800 Subject: [PATCH 17/22] Help test pass in wasm --- .../example/integration_test/link_widget_test.dart | 13 +++++++++++++ 1 file changed, 13 insertions(+) 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 d46e8b6786bb..b23f3558b789 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 @@ -374,6 +374,7 @@ void main() { )); // Platform view creation happens asynchronously. await tester.pumpAndSettle(); + await tester.pump(); expect(pushedRouteNames, isEmpty); expect(testPlugin.launches, isEmpty); @@ -433,6 +434,7 @@ void main() { )); // Platform view creation happens asynchronously. await tester.pumpAndSettle(); + await tester.pump(); expect(pushedRouteNames, isEmpty); expect(testPlugin.launches, isEmpty); @@ -494,6 +496,7 @@ void main() { )); // Platform view creation happens asynchronously. await tester.pumpAndSettle(); + await tester.pump(); expect(pushedRouteNames, isEmpty); expect(testPlugin.launches, isEmpty); @@ -529,6 +532,7 @@ void main() { )); // Platform view creation happens asynchronously. await tester.pumpAndSettle(); + await tester.pump(); expect(pushedRouteNames, isEmpty); expect(testPlugin.launches, isEmpty); @@ -565,6 +569,7 @@ void main() { )); // Platform view creation happens asynchronously. await tester.pumpAndSettle(); + await tester.pump(); expect(pushedRouteNames, isEmpty); expect(testPlugin.launches, isEmpty); @@ -623,6 +628,7 @@ void main() { )); // Platform view creation happens asynchronously. await tester.pumpAndSettle(); + await tester.pump(); expect(pushedRouteNames, isEmpty); expect(testPlugin.launches, isEmpty); @@ -659,6 +665,7 @@ void main() { )); // Platform view creation happens asynchronously. await tester.pumpAndSettle(); + await tester.pump(); expect(pushedRouteNames, isEmpty); expect(testPlugin.launches, isEmpty); @@ -692,6 +699,7 @@ void main() { )); // Platform view creation happens asynchronously. await tester.pumpAndSettle(); + await tester.pump(); expect(pushedRouteNames, isEmpty); expect(testPlugin.launches, isEmpty); @@ -726,6 +734,7 @@ void main() { )); // Platform view creation happens asynchronously. await tester.pumpAndSettle(); + await tester.pump(); expect(pushedRouteNames, isEmpty); expect(testPlugin.launches, isEmpty); @@ -874,6 +883,7 @@ void main() { )); // Platform view creation happens asynchronously. await tester.pumpAndSettle(); + await tester.pump(); final html.Element semanticsHost = html.document.createElement('flt-semantics-host'); @@ -939,6 +949,7 @@ void main() { )); // Platform view creation happens asynchronously. await tester.pumpAndSettle(); + await tester.pump(); final html.Element semanticsHost = html.document.createElement('flt-semantics-host'); @@ -983,6 +994,7 @@ void main() { )); // Platform view creation happens asynchronously. await tester.pumpAndSettle(); + await tester.pump(); final html.Element semanticsHost = html.document.createElement('flt-semantics-host'); @@ -1038,6 +1050,7 @@ void main() { )); // Platform view creation happens asynchronously. await tester.pumpAndSettle(); + await tester.pump(); final html.Element semanticsHost = html.document.createElement('flt-semantics-host'); From 4cd3f68df652e4db52f9efeb2427e892bfc15b48 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Wed, 8 Jan 2025 15:53:56 -0800 Subject: [PATCH 18/22] dart format . --- .../integration_test/link_widget_test.dart | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) 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 b23f3558b789..b8c11fa32da1 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 @@ -1112,16 +1112,16 @@ html.MouseEvent _simulateClick(html.Element target, {bool metaKey = false}) { return mouseEvent; } -html.KeyboardEvent _simulateKeydown(html.Element target, {bool metaKey = false}) { +html.KeyboardEvent _simulateKeydown(html.Element target, + {bool metaKey = false}) { final html.KeyboardEvent keydownEvent = html.KeyboardEvent( - 'keydown', - html.KeyboardEventInit( - bubbles: true, - cancelable: true, - metaKey: metaKey, - // code: 'Space', - ) - ); + 'keydown', + html.KeyboardEventInit( + bubbles: true, + cancelable: true, + metaKey: metaKey, + // code: 'Space', + )); LinkViewController.handleGlobalKeydown(event: keydownEvent); return keydownEvent; } From c2035421960d03487ffc2ea338ae3941076ef956 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Wed, 8 Jan 2025 16:59:44 -0800 Subject: [PATCH 19/22] Tag as v2.4.0, update CHANGELOG. --- packages/url_launcher/url_launcher_web/CHANGELOG.md | 8 ++++++-- packages/url_launcher/url_launcher_web/pubspec.yaml | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/url_launcher/url_launcher_web/CHANGELOG.md b/packages/url_launcher/url_launcher_web/CHANGELOG.md index 0baf6389c8b9..f923d118fca3 100644 --- a/packages/url_launcher/url_launcher_web/CHANGELOG.md +++ b/packages/url_launcher/url_launcher_web/CHANGELOG.md @@ -1,6 +1,10 @@ -## NEXT +## 2.4.0 -* Updates minimum supported SDK version to Flutter 3.22/Dart 3.4. +* Enhances handling of out-of-order events. +* Adds support for clicks with a modifier key (e.g. cmd+click). +* Improves support for semantics. +* Applies the `target` attribute to semantic links. +* Updates minimum supported SDK version to Flutter 3.27/Dart 3.6. ## 2.3.3 diff --git a/packages/url_launcher/url_launcher_web/pubspec.yaml b/packages/url_launcher/url_launcher_web/pubspec.yaml index 8ccff7f23349..ee13f34f7523 100644 --- a/packages/url_launcher/url_launcher_web/pubspec.yaml +++ b/packages/url_launcher/url_launcher_web/pubspec.yaml @@ -2,10 +2,10 @@ 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.3 +version: 2.4.0 environment: - sdk: ^3.4.0 + sdk: ^3.6.0 flutter: ">=3.27.0" flutter: From c12a920f677cd4b19167060974bce40666b1caaf Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Wed, 8 Jan 2025 17:13:34 -0800 Subject: [PATCH 20/22] Update sdk lower bound in example. --- packages/url_launcher/url_launcher_web/example/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/url_launcher/url_launcher_web/example/pubspec.yaml b/packages/url_launcher/url_launcher_web/example/pubspec.yaml index 1b851677ca60..b734fade0f7e 100644 --- a/packages/url_launcher/url_launcher_web/example/pubspec.yaml +++ b/packages/url_launcher/url_launcher_web/example/pubspec.yaml @@ -2,7 +2,7 @@ name: regular_integration_tests publish_to: none environment: - sdk: ^3.4.0 + sdk: ^3.6.0 flutter: ">=3.27.0" dependencies: From 4e703d5af181532c2a049d5bbb5824b434b07a30 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Thu, 9 Jan 2025 16:54:18 -0800 Subject: [PATCH 21/22] Address PR comments. --- .../url_launcher_web/lib/src/link.dart | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) 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 7f9c2fa13af9..b6034ddabbec 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -228,26 +228,28 @@ class LinkTriggerSignals { } } - /// Registers a FollowLink signal with the given [viewId]. - void registerFollowLink({required int viewId}) { + /// Acknowledges a FollowLink signal from [viewId]. + void acknowledgeFollowLink({required int viewId}) { _hasFollowLink = true; _viewIdFromFollowLink = viewId; _didUpdate(); } - /// Registers a DOM event signal with the given [viewId] and [mouseEvent]. + /// Acknowledges an incoming [mouseEvent] from a specific [viewId]. /// - /// The [viewId] is optional because it cannot be determined from some DOM - /// events. + /// [viewId] is nullable because it cannot be determined for some DOM events. /// - /// The [mouseEvent] is optional because the signal may be from a keyboard + /// [mouseEvent] is nullable because some signals may come from a keyboard /// event. - void registerDomEvent({ + /// + /// If `mouseEvent` is not null, `viewId` becomes mandatory. If `viewId` is + /// not present in this case, a [StateError] is thrown. + void acknowledgeMouseEvent({ required int? viewId, required html.MouseEvent? mouseEvent, }) { if (mouseEvent != null && viewId == null) { - throw AssertionError('`viewId` must be provided for mouse events'); + throw StateError('`viewId` must be provided for mouse events'); } _hasDomEvent = true; _viewIdFromDomEvent = viewId; @@ -427,7 +429,7 @@ class LinkViewController extends PlatformViewController { // The keydown event is not directly associated with the target Link, so // we can't find the `viewId` from the event. - _triggerSignals.registerDomEvent(viewId: null, mouseEvent: null); + _triggerSignals.acknowledgeMouseEvent(viewId: null, mouseEvent: null); _triggerSignals.triggerLinkIfReady(); } @@ -459,7 +461,7 @@ class LinkViewController extends PlatformViewController { return; } - _triggerSignals.registerDomEvent( + _triggerSignals.acknowledgeMouseEvent( viewId: viewIdFromTarget, mouseEvent: event, ); @@ -470,7 +472,7 @@ class LinkViewController extends PlatformViewController { /// Call this method to indicate that a hit test has been registered for the /// given [viewId]. static void onFollowLink(int viewId) { - _triggerSignals.registerFollowLink(viewId: viewId); + _triggerSignals.acknowledgeFollowLink(viewId: viewId); _triggerSignals.triggerLinkIfReady(); } From 286d33d2fa3e160d4c9378938973c272b00d4e15 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Thu, 9 Jan 2025 17:45:11 -0800 Subject: [PATCH 22/22] Rename signal handlers to onSignalName --- .../url_launcher_web/lib/src/link.dart | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) 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 b6034ddabbec..3540ac97f518 100644 --- a/packages/url_launcher/url_launcher_web/lib/src/link.dart +++ b/packages/url_launcher/url_launcher_web/lib/src/link.dart @@ -228,23 +228,23 @@ class LinkTriggerSignals { } } - /// Acknowledges a FollowLink signal from [viewId]. - void acknowledgeFollowLink({required int viewId}) { + /// Handles a FollowLink signal from [viewId]. + void onFollowLink({required int viewId}) { _hasFollowLink = true; _viewIdFromFollowLink = viewId; _didUpdate(); } - /// Acknowledges an incoming [mouseEvent] from a specific [viewId]. + /// Handles a [mouseEvent] signal from a specific [viewId]. /// - /// [viewId] is nullable because it cannot be determined for some DOM events. - /// - /// [mouseEvent] is nullable because some signals may come from a keyboard - /// event. + /// * [viewId] identifies the view where the Link widget was rendered to. It + /// is nullable because it cannot be determined for some DOM events. + /// * [mouseEvent] is the DOM MouseEvent that triggered this signal. It is + /// nullable because some signals may come from a keyboard event. /// /// If `mouseEvent` is not null, `viewId` becomes mandatory. If `viewId` is /// not present in this case, a [StateError] is thrown. - void acknowledgeMouseEvent({ + void onMouseEvent({ required int? viewId, required html.MouseEvent? mouseEvent, }) { @@ -429,7 +429,7 @@ class LinkViewController extends PlatformViewController { // The keydown event is not directly associated with the target Link, so // we can't find the `viewId` from the event. - _triggerSignals.acknowledgeMouseEvent(viewId: null, mouseEvent: null); + _triggerSignals.onMouseEvent(viewId: null, mouseEvent: null); _triggerSignals.triggerLinkIfReady(); } @@ -461,7 +461,7 @@ class LinkViewController extends PlatformViewController { return; } - _triggerSignals.acknowledgeMouseEvent( + _triggerSignals.onMouseEvent( viewId: viewIdFromTarget, mouseEvent: event, ); @@ -472,7 +472,7 @@ class LinkViewController extends PlatformViewController { /// Call this method to indicate that a hit test has been registered for the /// given [viewId]. static void onFollowLink(int viewId) { - _triggerSignals.acknowledgeFollowLink(viewId: viewId); + _triggerSignals.onFollowLink(viewId: viewId); _triggerSignals.triggerLinkIfReady(); }