From aff4047665c98355193d92077771b447c95ed9bc Mon Sep 17 00:00:00 2001 From: tugorez Date: Wed, 6 May 2020 23:04:56 -0700 Subject: [PATCH 1/8] Launches mailto urls in same window on iOS devices --- .../url_launcher_web/lib/src/navigator.dart | 15 ------ .../lib/url_launcher_web.dart | 12 +++-- .../url_launcher_web/pubspec.yaml | 1 - .../test/url_launcher_web_test.dart | 52 +++++++++++++------ 4 files changed, 44 insertions(+), 36 deletions(-) delete mode 100644 packages/url_launcher/url_launcher_web/lib/src/navigator.dart diff --git a/packages/url_launcher/url_launcher_web/lib/src/navigator.dart b/packages/url_launcher/url_launcher_web/lib/src/navigator.dart deleted file mode 100644 index 4c7a99d4c486..000000000000 --- a/packages/url_launcher/url_launcher_web/lib/src/navigator.dart +++ /dev/null @@ -1,15 +0,0 @@ -@JS() -library navigator; - -import 'package:js/js.dart'; -import 'package:meta/meta.dart'; - -@JS('window.navigator.standalone') -external bool get _standalone; - -/// The window.navigator.standalone DOM property. -bool get standalone => _standalone ?? false; - -@visibleForTesting -@JS('window.navigator.standalone') -external set standalone(bool enabled); diff --git a/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart b/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart index cce2bbf635ee..0f9f21d87bff 100644 --- a/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart +++ b/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart @@ -4,12 +4,12 @@ import 'dart:html' as html; import 'package:flutter_web_plugins/flutter_web_plugins.dart'; import 'package:meta/meta.dart'; import 'package:url_launcher_platform_interface/url_launcher_platform_interface.dart'; -import 'src/navigator.dart' as navigator; /// The web implementation of [UrlLauncherPlatform]. /// /// This class implements the `package:url_launcher` functionality for the web. class UrlLauncherPlugin extends UrlLauncherPlatform { + static final _iosPlatforms = ['iPhone', 'iPad', 'iPod']; html.Window _window; /// A constructor that allows tests to override the window object used by the plugin. @@ -21,14 +21,18 @@ class UrlLauncherPlugin extends UrlLauncherPlatform { UrlLauncherPlatform.instance = UrlLauncherPlugin(); } + bool get _isIos => _iosPlatforms.contains(_window.navigator.platform); + + bool _isMailTo(String url) { + return Uri.tryParse(url)?.isScheme('mailto') ?? false; + } + /// Opens the given [url] in a new window. /// /// Returns the newly created window. @visibleForTesting html.WindowBase openNewWindow(String url) { - // We need to open on _top in ios browsers in standalone mode. - // See https://github.com/flutter/flutter/issues/51461 for reference. - final target = navigator.standalone ? '_top' : ''; + final target = _isIos && _isMailTo(url) ? '_top' : ''; return _window.open(url, target); } diff --git a/packages/url_launcher/url_launcher_web/pubspec.yaml b/packages/url_launcher/url_launcher_web/pubspec.yaml index 08b57aaa7e1a..64610e35fa6c 100644 --- a/packages/url_launcher/url_launcher_web/pubspec.yaml +++ b/packages/url_launcher/url_launcher_web/pubspec.yaml @@ -20,7 +20,6 @@ dependencies: flutter_web_plugins: sdk: flutter meta: ^1.1.7 - js: ^0.6.0 dev_dependencies: flutter_test: diff --git a/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart b/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart index b6cf8b70460e..624e8fff7a57 100644 --- a/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart +++ b/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart @@ -7,10 +7,19 @@ import 'dart:html' as html; import 'package:flutter_test/flutter_test.dart'; import 'package:url_launcher_web/url_launcher_web.dart'; -import 'package:url_launcher_web/src/navigator.dart' as navigator; import 'package:mockito/mockito.dart'; -class MockWindow extends Mock implements html.Window {} +class MockNavigator extends Mock implements html.Navigator { + final String platform; + + MockNavigator(this.platform); +} + +class MockWindow extends Mock implements html.Window { + final MockNavigator navigator; + + MockWindow({String platform = ''}) : navigator = MockNavigator(platform); +} void main() { group('$UrlLauncherPlugin', () { @@ -75,28 +84,39 @@ void main() { }); group('openNewWindow', () { - bool _standalone; - - setUp(() { - _standalone = navigator.standalone; - }); - - tearDown(() { - navigator.standalone = _standalone; - }); - test('the window that is launched is a new window', () { plugin.openNewWindow('https://www.google.com'); verify(mockWindow.open('https://www.google.com', '')); }); - test('the window that is launched is in the same window', () { - navigator.standalone = true; + group('iosDevices', () { + test('mailto urls should be launched on the same window on Iphone', () { + final mockIosWindow = MockWindow(platform: 'iPhone'); + UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); - plugin.openNewWindow('https://www.google.com'); + plugin.openNewWindow('mailto:name@mydomain.com'); + + verify(mockIosWindow.open('mailto:name@mydomain.com', '_top')); + }); + + test('mailto urls should be launched on the same window on Ipad', () { + final mockIosWindow = MockWindow(platform: 'iPad'); + UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); + + plugin.openNewWindow('mailto:name@mydomain.com'); + + verify(mockIosWindow.open('mailto:name@mydomain.com', '_top')); + }); + + test('mailto urls should be launched on the same window on Iphone', () { + final mockIosWindow = MockWindow(platform: 'iPod'); + UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); + + plugin.openNewWindow('mailto:name@mydomain.com'); - verify(mockWindow.open('https://www.google.com', '_top')); + verify(mockIosWindow.open('mailto:name@mydomain.com', '_top')); + }); }); }); }); From fe4aac79b0c68156b0b04c795b7d70e0ee683b07 Mon Sep 17 00:00:00 2001 From: tugorez Date: Sun, 10 May 2020 22:09:38 -0700 Subject: [PATCH 2/8] Launches mailto urls in same windows in emulated iOS devices --- .../lib/url_launcher_web.dart | 4 +-- .../test/url_launcher_web_test.dart | 33 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart b/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart index 0f9f21d87bff..1f510902c446 100644 --- a/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart +++ b/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart @@ -9,7 +9,7 @@ import 'package:url_launcher_platform_interface/url_launcher_platform_interface. /// /// This class implements the `package:url_launcher` functionality for the web. class UrlLauncherPlugin extends UrlLauncherPlatform { - static final _iosPlatforms = ['iPhone', 'iPad', 'iPod']; + static final _iosPlatforms = RegExp(r'iPad|iPhone|iPod'); html.Window _window; /// A constructor that allows tests to override the window object used by the plugin. @@ -21,7 +21,7 @@ class UrlLauncherPlugin extends UrlLauncherPlatform { UrlLauncherPlatform.instance = UrlLauncherPlugin(); } - bool get _isIos => _iosPlatforms.contains(_window.navigator.platform); + bool get _isIos => _iosPlatforms.hasMatch(_window.navigator.platform); bool _isMailTo(String url) { return Uri.tryParse(url)?.isScheme('mailto') ?? false; diff --git a/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart b/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart index 624e8fff7a57..b85327001a18 100644 --- a/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart +++ b/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart @@ -117,6 +117,39 @@ void main() { verify(mockIosWindow.open('mailto:name@mydomain.com', '_top')); }); + + test( + 'mailto urls should be launched on the same window on an simulated Iphone', + () { + final mockIosWindow = MockWindow(platform: 'iPhone Simulator'); + UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); + + plugin.openNewWindow('mailto:name@mydomain.com'); + + verify(mockIosWindow.open('mailto:name@mydomain.com', '_top')); + }); + + test( + 'mailto urls should be launched on the same window on an simulated Ipad', + () { + final mockIosWindow = MockWindow(platform: 'iPad Simulator'); + UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); + + plugin.openNewWindow('mailto:name@mydomain.com'); + + verify(mockIosWindow.open('mailto:name@mydomain.com', '_top')); + }); + + test( + 'mailto urls should be launched on the same window on an simulated Iphone', + () { + final mockIosWindow = MockWindow(platform: 'iPod Simulator'); + UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); + + plugin.openNewWindow('mailto:name@mydomain.com'); + + verify(mockIosWindow.open('mailto:name@mydomain.com', '_top')); + }); }); }); }); From 1f88bc39a61a58e2ce7ab0e63b9d7a2a21504826 Mon Sep 17 00:00:00 2001 From: tugorez Date: Sun, 10 May 2020 22:16:28 -0700 Subject: [PATCH 3/8] Increase version and update CHANGELOG --- packages/url_launcher/url_launcher_web/CHANGELOG.md | 8 ++++++-- .../url_launcher_web/lib/url_launcher_web.dart | 2 ++ packages/url_launcher/url_launcher_web/pubspec.yaml | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/url_launcher/url_launcher_web/CHANGELOG.md b/packages/url_launcher/url_launcher_web/CHANGELOG.md index e6702bde392a..ec9651e2985b 100644 --- a/packages/url_launcher/url_launcher_web/CHANGELOG.md +++ b/packages/url_launcher/url_launcher_web/CHANGELOG.md @@ -1,6 +1,10 @@ +# 0.1.1+5 + +- Open only "mailto" urls with target "\_top" on iOS. + # 0.1.1+4 -* Declare API stability and compatibility with `1.0.0` (more details at: https://github.com/flutter/flutter/wiki/Package-migration-to-1.0.0). +- Declare API stability and compatibility with `1.0.0` (more details at: https://github.com/flutter/flutter/wiki/Package-migration-to-1.0.0). # 0.1.1+3 @@ -8,7 +12,7 @@ # 0.1.1+2 -- Open urls with target "_top" on iOS PWAs. +- Open urls with target "\_top" on iOS PWAs. # 0.1.1+1 diff --git a/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart b/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart index 1f510902c446..10d4136f1d34 100644 --- a/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart +++ b/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart @@ -32,6 +32,8 @@ class UrlLauncherPlugin extends UrlLauncherPlatform { /// Returns the newly created window. @visibleForTesting html.WindowBase openNewWindow(String url) { + // We need to open mailto urls on the _top window context on iOS devices. + // See https://github.com/flutter/flutter/issues/51461 for reference. final target = _isIos && _isMailTo(url) ? '_top' : ''; return _window.open(url, target); } diff --git a/packages/url_launcher/url_launcher_web/pubspec.yaml b/packages/url_launcher/url_launcher_web/pubspec.yaml index 64610e35fa6c..f412483cb147 100644 --- a/packages/url_launcher/url_launcher_web/pubspec.yaml +++ b/packages/url_launcher/url_launcher_web/pubspec.yaml @@ -4,7 +4,7 @@ homepage: https://github.com/flutter/plugins/tree/master/packages/url_launcher/u # 0.1.y+z is compatible with 1.0.0, if you land a breaking change bump # the version to 2.0.0. # See more details: https://github.com/flutter/flutter/wiki/Package-migration-to-1.0.0 -version: 0.1.1+4 +version: 0.1.1+5 flutter: plugin: From db52d788218383735eab41c07441b4106bc60423 Mon Sep 17 00:00:00 2001 From: tugorez Date: Mon, 11 May 2020 08:58:48 -0700 Subject: [PATCH 4/8] Add tests to make sure http and https are still opened in new windows --- .../url_launcher_web/lib/url_launcher_web.dart | 4 +--- .../test/url_launcher_web_test.dart | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart b/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart index 10d4136f1d34..0f213a5748f9 100644 --- a/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart +++ b/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart @@ -23,9 +23,7 @@ class UrlLauncherPlugin extends UrlLauncherPlatform { bool get _isIos => _iosPlatforms.hasMatch(_window.navigator.platform); - bool _isMailTo(String url) { - return Uri.tryParse(url)?.isScheme('mailto') ?? false; - } + bool _isMailTo(String url) => Uri.tryParse(url)?.isScheme('mailto') ?? false; /// Opens the given [url] in a new window. /// diff --git a/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart b/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart index b85327001a18..3692cca64178 100644 --- a/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart +++ b/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart @@ -91,6 +91,24 @@ void main() { }); group('iosDevices', () { + test('http urls should be launched in a new window', () { + final mockIosWindow = MockWindow(platform: 'iPhone'); + UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); + + plugin.openNewWindow('http://www.google.com'); + + verify(mockIosWindow.open('http://www.google.com', '')); + }); + + test('https urls should be launched in a new window', () { + final mockIosWindow = MockWindow(platform: 'iPhone'); + UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); + + plugin.openNewWindow('https://www.google.com'); + + verify(mockIosWindow.open('https://www.google.com', '')); + }); + test('mailto urls should be launched on the same window on Iphone', () { final mockIosWindow = MockWindow(platform: 'iPhone'); UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); From da5487df956dcbe9698681f407f15173ef0173a5 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Mon, 11 May 2020 17:10:25 -0700 Subject: [PATCH 5/8] Use package:platform_detect to detect Safari to open mailto links in _top. --- .../lib/url_launcher_web.dart | 30 +++-- .../url_launcher_web/pubspec.yaml | 3 +- .../test/url_launcher_web_test.dart | 104 ++++-------------- 3 files changed, 44 insertions(+), 93 deletions(-) diff --git a/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart b/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart index 0f213a5748f9..936a6b8f392b 100644 --- a/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart +++ b/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart @@ -5,11 +5,24 @@ import 'package:flutter_web_plugins/flutter_web_plugins.dart'; import 'package:meta/meta.dart'; import 'package:url_launcher_platform_interface/url_launcher_platform_interface.dart'; +import 'package:platform_detect/platform_detect.dart' show browser; + +final _httpSchemes = { + 'http', 'https', +}; + +// See https://github.com/flutter/flutter/issues/51461 +final _safariTargetTopSchemes = { + 'mailto', 'tel', +}; + +/// The set of schemes that can be handled by the plugin +final _validSchemes = _httpSchemes.union(_safariTargetTopSchemes); + /// The web implementation of [UrlLauncherPlatform]. /// /// This class implements the `package:url_launcher` functionality for the web. class UrlLauncherPlugin extends UrlLauncherPlatform { - static final _iosPlatforms = RegExp(r'iPad|iPhone|iPod'); html.Window _window; /// A constructor that allows tests to override the window object used by the plugin. @@ -21,9 +34,7 @@ class UrlLauncherPlugin extends UrlLauncherPlatform { UrlLauncherPlatform.instance = UrlLauncherPlugin(); } - bool get _isIos => _iosPlatforms.hasMatch(_window.navigator.platform); - - bool _isMailTo(String url) => Uri.tryParse(url)?.isScheme('mailto') ?? false; + String _getUrlScheme(String url) => Uri.tryParse(url)?.scheme; /// Opens the given [url] in a new window. /// @@ -32,18 +43,15 @@ class UrlLauncherPlugin extends UrlLauncherPlatform { html.WindowBase openNewWindow(String url) { // We need to open mailto urls on the _top window context on iOS devices. // See https://github.com/flutter/flutter/issues/51461 for reference. - final target = _isIos && _isMailTo(url) ? '_top' : ''; + + final target = browser.isSafari && _safariTargetTopSchemes.contains(_getUrlScheme(url)) ? '_top' : ''; + return _window.open(url, target); } @override Future canLaunch(String url) { - final Uri parsedUrl = Uri.tryParse(url); - if (parsedUrl == null) return Future.value(false); - - return Future.value(parsedUrl.isScheme('http') || - parsedUrl.isScheme('https') || - parsedUrl.isScheme('mailto')); + return Future.value(_validSchemes.contains(_getUrlScheme(url))); } @override diff --git a/packages/url_launcher/url_launcher_web/pubspec.yaml b/packages/url_launcher/url_launcher_web/pubspec.yaml index f412483cb147..b2935263645c 100644 --- a/packages/url_launcher/url_launcher_web/pubspec.yaml +++ b/packages/url_launcher/url_launcher_web/pubspec.yaml @@ -15,6 +15,7 @@ flutter: dependencies: url_launcher_platform_interface: ^1.0.1 + platform_detect: ^1.4.0 flutter: sdk: flutter flutter_web_plugins: @@ -29,5 +30,5 @@ dev_dependencies: mockito: ^4.1.1 environment: - sdk: ">=2.0.0-dev.28.0 <3.0.0" + sdk: ">=2.2.0 <3.0.0" flutter: ">=1.10.0 <2.0.0" diff --git a/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart b/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart index 3692cca64178..1c37f7c0677b 100644 --- a/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart +++ b/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart @@ -9,23 +9,19 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:url_launcher_web/url_launcher_web.dart'; import 'package:mockito/mockito.dart'; -class MockNavigator extends Mock implements html.Navigator { - final String platform; +import 'package:platform_detect/test_utils.dart' as platform; - MockNavigator(this.platform); -} - -class MockWindow extends Mock implements html.Window { - final MockNavigator navigator; - - MockWindow({String platform = ''}) : navigator = MockNavigator(platform); -} +class MockWindow extends Mock implements html.Window {} void main() { group('$UrlLauncherPlugin', () { MockWindow mockWindow = MockWindow(); UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockWindow); + setUp(() { + platform.configurePlatformForTesting(browser: platform.chrome); + }); + group('canLaunch', () { test('"http" URLs -> true', () { expect(plugin.canLaunch('http://google.com'), completion(isTrue)); @@ -40,8 +36,12 @@ void main() { plugin.canLaunch('mailto:name@mydomain.com'), completion(isTrue)); }); - test('"tel" URLs -> false', () { - expect(plugin.canLaunch('tel:5551234567'), completion(isFalse)); + test('"tel" URLs -> true', () { + expect(plugin.canLaunch('tel:5551234567'), completion(isTrue)); + }); + + test('"ftp" URLs -> false', () { + expect(plugin.canLaunch('ftp://some.example.com'), completion(isFalse)); }); }); @@ -90,83 +90,25 @@ void main() { verify(mockWindow.open('https://www.google.com', '')); }); - group('iosDevices', () { - test('http urls should be launched in a new window', () { - final mockIosWindow = MockWindow(platform: 'iPhone'); - UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); - - plugin.openNewWindow('http://www.google.com'); - - verify(mockIosWindow.open('http://www.google.com', '')); + group('Safari', () { + setUp(() { + platform.configurePlatformForTesting(browser: platform.safari); }); - test('https urls should be launched in a new window', () { - final mockIosWindow = MockWindow(platform: 'iPhone'); - UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); - + test('http(s) urls should be launched in a new window', () { + plugin.openNewWindow('http://www.google.com'); plugin.openNewWindow('https://www.google.com'); - verify(mockIosWindow.open('https://www.google.com', '')); - }); - - test('mailto urls should be launched on the same window on Iphone', () { - final mockIosWindow = MockWindow(platform: 'iPhone'); - UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); - - plugin.openNewWindow('mailto:name@mydomain.com'); - - verify(mockIosWindow.open('mailto:name@mydomain.com', '_top')); + verify(mockWindow.open('http://www.google.com', '')); + verify(mockWindow.open('https://www.google.com', '')); }); - test('mailto urls should be launched on the same window on Ipad', () { - final mockIosWindow = MockWindow(platform: 'iPad'); - UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); - - plugin.openNewWindow('mailto:name@mydomain.com'); - - verify(mockIosWindow.open('mailto:name@mydomain.com', '_top')); - }); - - test('mailto urls should be launched on the same window on Iphone', () { - final mockIosWindow = MockWindow(platform: 'iPod'); - UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); - - plugin.openNewWindow('mailto:name@mydomain.com'); - - verify(mockIosWindow.open('mailto:name@mydomain.com', '_top')); - }); - - test( - 'mailto urls should be launched on the same window on an simulated Iphone', - () { - final mockIosWindow = MockWindow(platform: 'iPhone Simulator'); - UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); - - plugin.openNewWindow('mailto:name@mydomain.com'); - - verify(mockIosWindow.open('mailto:name@mydomain.com', '_top')); - }); - - test( - 'mailto urls should be launched on the same window on an simulated Ipad', - () { - final mockIosWindow = MockWindow(platform: 'iPad Simulator'); - UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); - - plugin.openNewWindow('mailto:name@mydomain.com'); - - verify(mockIosWindow.open('mailto:name@mydomain.com', '_top')); - }); - - test( - 'mailto urls should be launched on the same window on an simulated Iphone', - () { - final mockIosWindow = MockWindow(platform: 'iPod Simulator'); - UrlLauncherPlugin plugin = UrlLauncherPlugin(window: mockIosWindow); - + test('mailto and tel urls should be launched on the same window', () { plugin.openNewWindow('mailto:name@mydomain.com'); + plugin.openNewWindow('tel:+1-555-555-5555'); - verify(mockIosWindow.open('mailto:name@mydomain.com', '_top')); + verify(mockWindow.open('mailto:name@mydomain.com', '_top')); + verify(mockWindow.open('tel:+1-555-555-5555', '_top')); }); }); }); From 8ef00a0bfd5f0710fd9e602a3a349e05d86639bf Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Mon, 11 May 2020 17:41:55 -0700 Subject: [PATCH 6/8] Remove tel + dartfmt --- .../lib/url_launcher_web.dart | 10 ++++++--- .../test/url_launcher_web_test.dart | 22 ++++++++++--------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart b/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart index 936a6b8f392b..3922de1125c6 100644 --- a/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart +++ b/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart @@ -8,12 +8,13 @@ import 'package:url_launcher_platform_interface/url_launcher_platform_interface. import 'package:platform_detect/platform_detect.dart' show browser; final _httpSchemes = { - 'http', 'https', + 'http', + 'https', }; // See https://github.com/flutter/flutter/issues/51461 final _safariTargetTopSchemes = { - 'mailto', 'tel', + 'mailto', }; /// The set of schemes that can be handled by the plugin @@ -44,7 +45,10 @@ class UrlLauncherPlugin extends UrlLauncherPlatform { // We need to open mailto urls on the _top window context on iOS devices. // See https://github.com/flutter/flutter/issues/51461 for reference. - final target = browser.isSafari && _safariTargetTopSchemes.contains(_getUrlScheme(url)) ? '_top' : ''; + final target = + browser.isSafari && _safariTargetTopSchemes.contains(_getUrlScheme(url)) + ? '_top' + : ''; return _window.open(url, target); } diff --git a/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart b/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart index 1c37f7c0677b..51458f6ea4eb 100644 --- a/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart +++ b/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart @@ -36,12 +36,8 @@ void main() { plugin.canLaunch('mailto:name@mydomain.com'), completion(isTrue)); }); - test('"tel" URLs -> true', () { - expect(plugin.canLaunch('tel:5551234567'), completion(isTrue)); - }); - - test('"ftp" URLs -> false', () { - expect(plugin.canLaunch('ftp://some.example.com'), completion(isFalse)); + test('"tel" URLs -> false', () { + expect(plugin.canLaunch('tel:5551234567'), completion(isFalse)); }); }); @@ -84,12 +80,20 @@ void main() { }); group('openNewWindow', () { - test('the window that is launched is a new window', () { + test('http(s) urls should be launched in a new window', () { + plugin.openNewWindow('http://www.google.com'); plugin.openNewWindow('https://www.google.com'); + verify(mockWindow.open('http://www.google.com', '')); verify(mockWindow.open('https://www.google.com', '')); }); + test('mailto urls should be launched on a new window', () { + plugin.openNewWindow('mailto:name@mydomain.com'); + + verify(mockWindow.open('mailto:name@mydomain.com', '')); + }); + group('Safari', () { setUp(() { platform.configurePlatformForTesting(browser: platform.safari); @@ -103,12 +107,10 @@ void main() { verify(mockWindow.open('https://www.google.com', '')); }); - test('mailto and tel urls should be launched on the same window', () { + test('mailto urls should be launched on the same window', () { plugin.openNewWindow('mailto:name@mydomain.com'); - plugin.openNewWindow('tel:+1-555-555-5555'); verify(mockWindow.open('mailto:name@mydomain.com', '_top')); - verify(mockWindow.open('tel:+1-555-555-5555', '_top')); }); }); }); From 70373c3608cd47f0377f8377c6007f52c9870bd9 Mon Sep 17 00:00:00 2001 From: tugorez Date: Fri, 15 May 2020 10:26:37 -0700 Subject: [PATCH 7/8] Add one test for each scheme instead of grouping them. --- .../lib/url_launcher_web.dart | 29 ++++++------------- .../test/url_launcher_web_test.dart | 16 +++++++--- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart b/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart index 3922de1125c6..e55ceb2269bc 100644 --- a/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart +++ b/packages/url_launcher/url_launcher_web/lib/url_launcher_web.dart @@ -7,18 +7,7 @@ import 'package:url_launcher_platform_interface/url_launcher_platform_interface. import 'package:platform_detect/platform_detect.dart' show browser; -final _httpSchemes = { - 'http', - 'https', -}; - -// See https://github.com/flutter/flutter/issues/51461 -final _safariTargetTopSchemes = { - 'mailto', -}; - -/// The set of schemes that can be handled by the plugin -final _validSchemes = _httpSchemes.union(_safariTargetTopSchemes); +const _mailtoScheme = 'mailto'; /// The web implementation of [UrlLauncherPlatform]. /// @@ -26,6 +15,9 @@ final _validSchemes = _httpSchemes.union(_safariTargetTopSchemes); class UrlLauncherPlugin extends UrlLauncherPlatform { html.Window _window; + // The set of schemes that can be handled by the plugin + static final _supportedSchemes = {'http', 'https', _mailtoScheme}; + /// A constructor that allows tests to override the window object used by the plugin. UrlLauncherPlugin({@visibleForTesting html.Window window}) : _window = window ?? html.window; @@ -37,25 +29,22 @@ class UrlLauncherPlugin extends UrlLauncherPlatform { String _getUrlScheme(String url) => Uri.tryParse(url)?.scheme; + bool _isMailtoScheme(String url) => _getUrlScheme(url) == _mailtoScheme; + /// Opens the given [url] in a new window. /// /// Returns the newly created window. @visibleForTesting html.WindowBase openNewWindow(String url) { - // We need to open mailto urls on the _top window context on iOS devices. + // We need to open mailto urls on the _top window context on safari browsers. // See https://github.com/flutter/flutter/issues/51461 for reference. - - final target = - browser.isSafari && _safariTargetTopSchemes.contains(_getUrlScheme(url)) - ? '_top' - : ''; - + final target = browser.isSafari && _isMailtoScheme(url) ? '_top' : ''; return _window.open(url, target); } @override Future canLaunch(String url) { - return Future.value(_validSchemes.contains(_getUrlScheme(url))); + return Future.value(_supportedSchemes.contains(_getUrlScheme(url))); } @override diff --git a/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart b/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart index 51458f6ea4eb..4cf92062e10f 100644 --- a/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart +++ b/packages/url_launcher/url_launcher_web/test/url_launcher_web_test.dart @@ -80,11 +80,15 @@ void main() { }); group('openNewWindow', () { - test('http(s) urls should be launched in a new window', () { + test('http urls should be launched in a new window', () { plugin.openNewWindow('http://www.google.com'); - plugin.openNewWindow('https://www.google.com'); verify(mockWindow.open('http://www.google.com', '')); + }); + + test('https urls should be launched in a new window', () { + plugin.openNewWindow('https://www.google.com'); + verify(mockWindow.open('https://www.google.com', '')); }); @@ -99,11 +103,15 @@ void main() { platform.configurePlatformForTesting(browser: platform.safari); }); - test('http(s) urls should be launched in a new window', () { + test('http urls should be launched in a new window', () { plugin.openNewWindow('http://www.google.com'); - plugin.openNewWindow('https://www.google.com'); verify(mockWindow.open('http://www.google.com', '')); + }); + + test('https urls should be launched in a new window', () { + plugin.openNewWindow('https://www.google.com'); + verify(mockWindow.open('https://www.google.com', '')); }); From 735fc2a1e154259648f69a3b6a66534bb8e39e28 Mon Sep 17 00:00:00 2001 From: tugorez Date: Fri, 15 May 2020 10:28:57 -0700 Subject: [PATCH 8/8] Update changelog message to reflect changes. --- packages/url_launcher/url_launcher_web/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/url_launcher/url_launcher_web/CHANGELOG.md b/packages/url_launcher/url_launcher_web/CHANGELOG.md index ec9651e2985b..0f6c93cb431a 100644 --- a/packages/url_launcher/url_launcher_web/CHANGELOG.md +++ b/packages/url_launcher/url_launcher_web/CHANGELOG.md @@ -1,6 +1,6 @@ # 0.1.1+5 -- Open only "mailto" urls with target "\_top" on iOS. +- Open "mailto" urls with target set as "\_top" on Safari browsers. # 0.1.1+4