Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/url_launcher/url_launcher_android/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 6.3.22

* Adds support for `externalNonBrowserApplication` on API 30+.

## 6.3.21

* Updates minimum supported SDK version to Flutter 3.35.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Autogenerated from Pigeon (v22.4.1), do not edit directly.
// Autogenerated from Pigeon (v22.7.4), do not edit directly.
// See also: https://pub.dev/packages/pigeon

package io.flutter.plugins.urllauncher;
Expand Down Expand Up @@ -298,7 +298,10 @@ public interface UrlLauncherApi {
Boolean canLaunchUrl(@NonNull String url);
/** Opens the URL externally, returning true if successful. */
@NonNull
Boolean launchUrl(@NonNull String url, @NonNull Map<String, String> headers);
Boolean launchUrl(
@NonNull String url,
@NonNull Map<String, String> headers,
@NonNull Boolean requireNonBrowser);
/**
* Opens the URL in an in-app Custom Tab or WebView, returning true if it opens successfully.
*/
Expand Down Expand Up @@ -367,8 +370,9 @@ static void setUp(
ArrayList<Object> args = (ArrayList<Object>) message;
String urlArg = (String) args.get(0);
Map<String, String> headersArg = (Map<String, String>) args.get(1);
Boolean requireNonBrowserArg = (Boolean) args.get(2);
try {
Boolean output = api.launchUrl(urlArg, headersArg);
Boolean output = api.launchUrl(urlArg, headersArg, requireNonBrowserArg);
wrapped.add(0, output);
} catch (Throwable exception) {
wrapped = wrapError(exception);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,20 @@ void setActivity(@Nullable Activity activity) {
}

@Override
public @NonNull Boolean launchUrl(@NonNull String url, @NonNull Map<String, String> headers) {
public @NonNull Boolean launchUrl(
@NonNull String url,
@NonNull Map<String, String> headers,
@NonNull Boolean requireNonBrowser) {
ensureActivity();
assert activity != null;

Intent launchIntent =
new Intent(Intent.ACTION_VIEW)
.setData(Uri.parse(url))
.putExtra(Browser.EXTRA_HEADERS, extractBundle(headers));
if (requireNonBrowser) {
launchIntent.addFlags(Intent.FLAG_ACTIVITY_REQUIRE_NON_BROWSER);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The FLAG_ACTIVITY_REQUIRE_NON_BROWSER flag was introduced in API level 30. While the system may ignore unknown flags on older versions, it's best practice to explicitly check the device's API level before adding a flag that is not available on all supported versions. This makes the code more robust and self-documenting about its API level dependency. You may need to add an import for android.os.Build.

    if (requireNonBrowser) {
      if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.R) {
        launchIntent.addFlags(Intent.FLAG_ACTIVITY_REQUIRE_NON_BROWSER);
      }
    }

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gmackall @reidbaker Does the Android team have a preference here? I had considered adding this but then didn't because presumably adding a flag that didn't exist in old versions should be harmless, so I figured it didn't add any value. I guess the self-documenting aspect is worth considering though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong preference here, but slightly lean towards including it just for readability.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guard added.

try {
activity.startActivity(launchIntent);
} catch (ActivityNotFoundException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public void launch_throwsForNoCurrentActivity() {
Messages.FlutterError exception =
assertThrows(
Messages.FlutterError.class,
() -> api.launchUrl("https://flutter.dev", new HashMap<>()));
() -> api.launchUrl("https://flutter.dev", new HashMap<>(), false));
assertEquals("NO_ACTIVITY", exception.code);
}

Expand All @@ -100,11 +100,29 @@ public void launch_createsIntentWithPassedUrl() {
api.setActivity(activity);
doThrow(new ActivityNotFoundException()).when(activity).startActivity(any());

api.launchUrl("https://flutter.dev", new HashMap<>());
api.launchUrl("https://flutter.dev", new HashMap<>(), false);

final ArgumentCaptor<Intent> intentCaptor = ArgumentCaptor.forClass(Intent.class);
verify(activity).startActivity(intentCaptor.capture());
assertEquals(url, intentCaptor.getValue().getData().toString());
assertEquals(0, intentCaptor.getValue().getFlags() & Intent.FLAG_ACTIVITY_REQUIRE_NON_BROWSER);
}

@Test
public void launch_setsRequireNonBrowserWhenRequested() {
Activity activity = mock(Activity.class);
String url = "https://flutter.dev";
UrlLauncher api = new UrlLauncher(ApplicationProvider.getApplicationContext());
api.setActivity(activity);
doThrow(new ActivityNotFoundException()).when(activity).startActivity(any());

api.launchUrl("https://flutter.dev", new HashMap<>(), true);

final ArgumentCaptor<Intent> intentCaptor = ArgumentCaptor.forClass(Intent.class);
verify(activity).startActivity(intentCaptor.capture());
assertEquals(
Intent.FLAG_ACTIVITY_REQUIRE_NON_BROWSER,
intentCaptor.getValue().getFlags() & Intent.FLAG_ACTIVITY_REQUIRE_NON_BROWSER);
}

@Test
Expand All @@ -114,7 +132,7 @@ public void launch_returnsFalse() {
api.setActivity(activity);
doThrow(new ActivityNotFoundException()).when(activity).startActivity(any());

boolean result = api.launchUrl("https://flutter.dev", new HashMap<>());
boolean result = api.launchUrl("https://flutter.dev", new HashMap<>(), false);

assertFalse(result);
}
Expand All @@ -125,7 +143,7 @@ public void launch_returnsTrue() {
UrlLauncher api = new UrlLauncher(ApplicationProvider.getApplicationContext());
api.setActivity(activity);

boolean result = api.launchUrl("https://flutter.dev", new HashMap<>());
boolean result = api.launchUrl("https://flutter.dev", new HashMap<>(), false);

assertTrue(result);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,17 @@ class _MyHomePageState extends State<MyHomePage> {
}
}

Future<void> _launchInNonBrowserExternalApp(String url) async {
if (!await launcher.launchUrl(
url,
const LaunchOptions(
mode: PreferredLaunchMode.externalNonBrowserApplication,
),
)) {
throw Exception('Could not launch $url');
}
}

Future<void> _launchInCustomTab(String url) async {
if (!await launcher.launchUrl(
url,
Expand Down Expand Up @@ -187,6 +198,14 @@ class _MyHomePageState extends State<MyHomePage> {
: null,
child: const Text('Launch in browser'),
),
ElevatedButton(
onPressed: _hasCustomTabSupport
? () => setState(() {
_launched = _launchInNonBrowserExternalApp(toLaunch);
})
: null,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The onPressed callback for the 'Launch in non-browser app' button is conditioned on _hasCustomTabSupport. However, the ability to launch an external non-browser application is not dependent on Custom Tabs support. This condition seems to be incorrectly copied from another button and could prevent users from testing this feature on devices that don't support Custom Tabs but can handle the intent. The button should likely be enabled unconditionally.

                onPressed: () => setState(() {
                  _launched = _launchInNonBrowserExternalApp(toLaunch);
                }),

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, this was wrong because the code I copied from also shouldn't have had it; it had incorrectly been set up to gate the "launch in browser" button rather than the "launch in custom tab" button.

Fixed my copypasta and the pre-existing bug.

child: const Text('Launch in non-browser app'),
),
const Padding(padding: EdgeInsets.all(16.0)),
ElevatedButton(
onPressed: () => setState(() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Autogenerated from Pigeon (v22.4.1), do not edit directly.
// Autogenerated from Pigeon (v22.7.4), do not edit directly.
// See also: https://pub.dev/packages/pigeon
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import, no_leading_underscores_for_local_identifiers

Expand Down Expand Up @@ -142,7 +142,11 @@ class UrlLauncherApi {
}

/// Opens the URL externally, returning true if successful.
Future<bool> launchUrl(String url, Map<String, String> headers) async {
Future<bool> launchUrl(
String url,
Map<String, String> headers,
bool requireNonBrowser,
) async {
final String pigeonVar_channelName =
'dev.flutter.pigeon.url_launcher_android.UrlLauncherApi.launchUrl$pigeonVar_messageChannelSuffix';
final BasicMessageChannel<Object?> pigeonVar_channel =
Expand All @@ -152,7 +156,8 @@ class UrlLauncherApi {
binaryMessenger: pigeonVar_binaryMessenger,
);
final List<Object?>? pigeonVar_replyList =
await pigeonVar_channel.send(<Object?>[url, headers]) as List<Object?>?;
await pigeonVar_channel.send(<Object?>[url, headers, requireNonBrowser])
as List<Object?>?;
if (pigeonVar_replyList == null) {
throw _createConnectionError(pigeonVar_channelName);
} else if (pigeonVar_replyList.length > 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,16 @@ class UrlLauncherAndroid extends UrlLauncherPlatform {
@override
Future<bool> launchUrl(String url, LaunchOptions options) async {
final bool inApp;
bool requireNonBrowser = false;
switch (options.mode) {
case PreferredLaunchMode.inAppWebView:
case PreferredLaunchMode.inAppBrowserView:
inApp = true;
case PreferredLaunchMode.externalApplication:
inApp = false;
case PreferredLaunchMode.externalNonBrowserApplication:
// TODO(stuartmorgan): Add full support for
// externalNonBrowsingApplication; see
// https://github.com/flutter/flutter/issues/66721.
// Currently it's treated the same as externalApplication.
inApp = false;
requireNonBrowser = true;
case PreferredLaunchMode.platformDefault:
// Intentionally treat any new values as platformDefault; see comment in
// supportsMode.
Expand All @@ -114,6 +113,7 @@ class UrlLauncherAndroid extends UrlLauncherPlatform {
succeeded = await _hostApi.launchUrl(
url,
options.webViewConfiguration.headers,
requireNonBrowser,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ abstract class UrlLauncherApi {
bool canLaunchUrl(String url);

/// Opens the URL externally, returning true if successful.
bool launchUrl(String url, Map<String, String> headers);
bool launchUrl(
String url,
Map<String, String> headers,
bool requireNonBrowser,
);

/// Opens the URL in an in-app Custom Tab or WebView, returning true if it
/// opens successfully.
Expand Down
2 changes: 1 addition & 1 deletion packages/url_launcher/url_launcher_android/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: url_launcher_android
description: Android implementation of the url_launcher plugin.
repository: https://github.com/flutter/packages/tree/main/packages/url_launcher/url_launcher_android
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+url_launcher%22
version: 6.3.21
version: 6.3.22

environment:
sdk: ^3.9.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ void main() {
);
expect(launched, true);
expect(api.usedWebView, false);
expect(api.requiredNonBrowser, false);
expect(api.passedWebViewOptions?.headers, isEmpty);
});

Expand All @@ -254,6 +255,19 @@ void main() {
expect(api.passedWebViewOptions?.headers['key'], 'value');
});

test('passes non-browser flag', () async {
final UrlLauncherAndroid launcher = UrlLauncherAndroid(api: api);
final bool launched = await launcher.launchUrl(
'http://example.com/',
const LaunchOptions(
mode: PreferredLaunchMode.externalNonBrowserApplication,
),
);
expect(launched, true);
expect(api.usedWebView, false);
expect(api.requiredNonBrowser, true);
});

test('passes through no-activity exception', () async {
final UrlLauncherAndroid launcher = UrlLauncherAndroid(api: api);
await expectLater(
Expand Down Expand Up @@ -484,6 +498,7 @@ class _FakeUrlLauncherApi implements UrlLauncherApi {
BrowserOptions? passedBrowserOptions;
bool? usedWebView;
bool? allowedCustomTab;
bool? requiredNonBrowser;
bool? closed;

/// A domain that will be treated as having no handler, even for http(s).
Expand All @@ -495,13 +510,18 @@ class _FakeUrlLauncherApi implements UrlLauncherApi {
}

@override
Future<bool> launchUrl(String url, Map<String, String> headers) async {
Future<bool> launchUrl(
String url,
Map<String, String> headers,
bool requireNonBrowser,
) async {
passedWebViewOptions = WebViewOptions(
enableJavaScript: false,
enableDomStorage: false,
headers: headers,
);

requiredNonBrowser = requireNonBrowser;
usedWebView = false;
return _launch(url);
}
Expand Down