Skip to content

Commit 4d2e388

Browse files
PIG208gnprice
PIG208
and
gnprice
committed
setting: Make in-app explicitly the browser preference on Android
The url_launcher documentation warns that the meaning of platformDefault may change at any time. We've chosen particular behaviors we want on different platforms, so express those choices explicitly. This change affects the behavior on Android, such that we use in-app for HTTP links, partly because it doesn't make sense to use the in-app browser for other types of links, and that url_launcher explicitly prohibits this combination: https://github.com/flutter/packages/blob/9cc6f370/packages/url_launcher/url_launcher/lib/src/url_launcher_uri.dart#L46-L51 Co-authored-by: Greg Price <[email protected]> Signed-off-by: Zixuan James Li <[email protected]>
1 parent 89bf840 commit 4d2e388

File tree

6 files changed

+84
-27
lines changed

6 files changed

+84
-27
lines changed

‎lib/model/settings.dart

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1+
import 'package:flutter/foundation.dart';
2+
13
import '../generated/l10n/zulip_localizations.dart';
4+
import 'binding.dart';
5+
import 'database.dart';
26

37
/// The user's choice of visual theme for the app.
48
///
@@ -34,8 +38,33 @@ enum ThemeSetting {
3438
/// Write a migration if such a change is necessary.
3539
enum BrowserPreference {
3640
/// Use the in-app browser for HTTP links.
41+
///
42+
/// For other types of links (e.g. mailto), this translates to
43+
/// [UrlLaunchMode.platformDefault], so that non-in-app browsers/apps can
44+
/// still be used.
3745
inApp,
3846

3947
/// Use the user's default browser app.
4048
external,
4149
}
50+
51+
extension GlobalSettingsHelpers on GlobalSettingsData {
52+
BrowserPreference get defaultBrowserPreference {
53+
return switch (defaultTargetPlatform) {
54+
// On iOS we prefer UrlLaunchMode.externalApplication because (for
55+
// HTTP URLs) UrlLaunchMode.platformDefault uses SFSafariViewController,
56+
// which gives an awkward UX as described here:
57+
// https://chat.zulip.org/#narrow/stream/48-mobile/topic/in-app.20browser/near/1169118
58+
TargetPlatform.iOS => BrowserPreference.external,
59+
_ => BrowserPreference.inApp,
60+
};
61+
}
62+
63+
UrlLaunchMode getUrlLaunchMode(Uri url) {
64+
return switch (defaultBrowserPreference) {
65+
BrowserPreference.inApp => url.scheme == 'http' || url.scheme == 'https'
66+
? UrlLaunchMode.inAppBrowserView : UrlLaunchMode.platformDefault,
67+
BrowserPreference.external => UrlLaunchMode.externalApplication,
68+
};
69+
}
70+
}

‎lib/widgets/content.dart

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import 'dart:async';
22

33
import 'package:flutter/cupertino.dart';
4-
import 'package:flutter/foundation.dart';
54
import 'package:flutter/gestures.dart';
65
import 'package:flutter/material.dart';
76
import 'package:flutter/rendering.dart';
@@ -16,6 +15,7 @@ import '../model/avatar_url.dart';
1615
import '../model/binding.dart';
1716
import '../model/content.dart';
1817
import '../model/internal_link.dart';
18+
import '../model/settings.dart';
1919
import 'code_block.dart';
2020
import 'dialog.dart';
2121
import 'icons.dart';
@@ -1439,18 +1439,12 @@ void _launchUrl(BuildContext context, String urlString) async {
14391439
return;
14401440
}
14411441

1442+
final globalSettings = GlobalStoreWidget.of(context).globalSettings;
14421443
bool launched = false;
14431444
String? errorMessage;
14441445
try {
14451446
launched = await ZulipBinding.instance.launchUrl(url,
1446-
mode: switch (defaultTargetPlatform) {
1447-
// On iOS we prefer LaunchMode.externalApplication because (for
1448-
// HTTP URLs) LaunchMode.platformDefault uses SFSafariViewController,
1449-
// which gives an awkward UX as described here:
1450-
// https://chat.zulip.org/#narrow/stream/48-mobile/topic/in-app.20browser/near/1169118
1451-
TargetPlatform.iOS => UrlLaunchMode.externalApplication,
1452-
_ => UrlLaunchMode.platformDefault,
1453-
});
1447+
mode: globalSettings.getUrlLaunchMode(url));
14541448
} on PlatformException catch (e) {
14551449
errorMessage = e.message;
14561450
}

‎test/model/settings_test.dart

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import 'package:checks/checks.dart';
2+
import 'package:flutter/foundation.dart';
3+
import 'package:flutter_test/flutter_test.dart';
4+
import 'package:zulip/model/binding.dart';
5+
6+
import '../example_data.dart' as eg;
7+
import 'store_checks.dart';
8+
import 'store_test.dart';
9+
10+
void main() {
11+
final httpLink = Uri.parse('http://chat.zulip.org');
12+
final nonHttpLink = Uri.parse('mailto:[email protected]');
13+
14+
group('getUrlLaunchMode', () {
15+
testAndroidIos('use our per-platform defaults for HTTP links', () {
16+
final globalStore = eg.globalStore(globalSettings: eg.globalSettings(
17+
browserPreference: null));
18+
check(globalStore).globalSettings.getUrlLaunchMode(httpLink).equals(
19+
defaultTargetPlatform == TargetPlatform.iOS
20+
? UrlLaunchMode.externalApplication : UrlLaunchMode.inAppBrowserView);
21+
});
22+
23+
testAndroidIos('use our per-platform defaults for non-HTTP links', () {
24+
final globalStore = eg.globalStore(globalSettings: eg.globalSettings(
25+
browserPreference: null));
26+
check(globalStore).globalSettings.getUrlLaunchMode(nonHttpLink).equals(
27+
defaultTargetPlatform == TargetPlatform.android
28+
? UrlLaunchMode.platformDefault : UrlLaunchMode.externalApplication);
29+
});
30+
});
31+
}

‎test/model/store_checks.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import 'package:zulip/api/core.dart';
33
import 'package:zulip/api/model/initial_snapshot.dart';
44
import 'package:zulip/api/model/model.dart';
55
import 'package:zulip/model/autocomplete.dart';
6+
import 'package:zulip/model/binding.dart';
67
import 'package:zulip/model/database.dart';
78
import 'package:zulip/model/recent_dm_conversations.dart';
89
import 'package:zulip/model/settings.dart';
@@ -32,6 +33,8 @@ extension GlobalStoreChecks on Subject<GlobalStore> {
3233
extension GlobalSettingsDataChecks on Subject<GlobalSettingsData> {
3334
Subject<ThemeSetting?> get themeSetting => has((x) => x.themeSetting, 'themeSetting');
3435
Subject<BrowserPreference?> get browserPreference => has((x) => x.browserPreference, 'browserPreference');
36+
Subject<BrowserPreference> get defaultBrowserPreference => has((x) => x.defaultBrowserPreference, 'defaultBrowserPreference');
37+
Subject<UrlLaunchMode> getUrlLaunchMode(Uri url) => has((x) => x.getUrlLaunchMode(url), 'getUrlLaunchMode');
3538
}
3639

3740
extension PerAccountStoreChecks on Subject<PerAccountStore> {

‎test/widgets/content_test.dart

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,7 @@ void main() {
521521
final expectedLaunchUrl = expectedVideo.hrefUrl;
522522
await tester.tap(find.byIcon(Icons.play_arrow_rounded));
523523
check(testBinding.takeLaunchUrlCalls())
524-
.single.equals((url: Uri.parse(expectedLaunchUrl), mode: LaunchMode.platformDefault));
524+
.single.equals((url: Uri.parse(expectedLaunchUrl), mode: LaunchMode.inAppBrowserView));
525525
}
526526

527527
testWidgets('video preview for youtube embed', (tester) async {
@@ -793,7 +793,7 @@ void main() {
793793
await tapText(tester, find.text('hello'));
794794

795795
final expectedLaunchMode = defaultTargetPlatform == TargetPlatform.iOS ?
796-
LaunchMode.externalApplication : LaunchMode.platformDefault;
796+
LaunchMode.externalApplication : LaunchMode.inAppBrowserView;
797797
check(testBinding.takeLaunchUrlCalls())
798798
.single.equals((url: Uri.parse('https://example/'), mode: expectedLaunchMode));
799799
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
@@ -811,19 +811,19 @@ void main() {
811811

812812
await tester.tapAt(base.translate(1*fontSize, 0)); // "fXo bar baz"
813813
check(testBinding.takeLaunchUrlCalls())
814-
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.platformDefault));
814+
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.inAppBrowserView));
815815

816816
await tester.tapAt(base.translate(9*fontSize, 0)); // "foo bar bXz"
817817
check(testBinding.takeLaunchUrlCalls())
818-
.single.equals((url: Uri.parse('https://b/'), mode: LaunchMode.platformDefault));
818+
.single.equals((url: Uri.parse('https://b/'), mode: LaunchMode.inAppBrowserView));
819819
});
820820

821821
testWidgets('link nested in other spans', (tester) async {
822822
await prepare(tester,
823823
'<p><strong><em><a href="https://a/">word</a></em></strong></p>');
824824
await tapText(tester, find.text('word'));
825825
check(testBinding.takeLaunchUrlCalls())
826-
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.platformDefault));
826+
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.inAppBrowserView));
827827
});
828828

829829
testWidgets('link containing other spans', (tester) async {
@@ -836,27 +836,27 @@ void main() {
836836

837837
await tester.tapAt(base.translate(1*fontSize, 0)); // "tXo words"
838838
check(testBinding.takeLaunchUrlCalls())
839-
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.platformDefault));
839+
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.inAppBrowserView));
840840

841841
await tester.tapAt(base.translate(6*fontSize, 0)); // "two woXds"
842842
check(testBinding.takeLaunchUrlCalls())
843-
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.platformDefault));
843+
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.inAppBrowserView));
844844
});
845845

846846
testWidgets('relative links are resolved', (tester) async {
847847
await prepare(tester,
848848
'<p><a href="/a/b?c#d">word</a></p>');
849849
await tapText(tester, find.text('word'));
850850
check(testBinding.takeLaunchUrlCalls())
851-
.single.equals((url: Uri.parse('${eg.realmUrl}a/b?c#d'), mode: LaunchMode.platformDefault));
851+
.single.equals((url: Uri.parse('${eg.realmUrl}a/b?c#d'), mode: LaunchMode.inAppBrowserView));
852852
});
853853

854854
testWidgets('link inside HeadingNode', (tester) async {
855855
await prepare(tester,
856856
'<h6><a href="https://a/">word</a></h6>');
857857
await tapText(tester, find.text('word'));
858858
check(testBinding.takeLaunchUrlCalls())
859-
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.platformDefault));
859+
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.inAppBrowserView));
860860
});
861861

862862
testWidgets('error dialog if invalid link', (tester) async {
@@ -910,7 +910,7 @@ void main() {
910910
await tapText(tester, find.text('invalid'));
911911
final expectedUrl = eg.realmUrl.resolve('/#narrow/stream/1-check/topic');
912912
check(testBinding.takeLaunchUrlCalls())
913-
.single.equals((url: expectedUrl, mode: LaunchMode.platformDefault));
913+
.single.equals((url: expectedUrl, mode: LaunchMode.inAppBrowserView));
914914
check(pushedRoutes).isEmpty();
915915
});
916916
});
@@ -1060,11 +1060,11 @@ void main() {
10601060

10611061
await tester.tap(find.text('Zulip — organized team chat'));
10621062
check(testBinding.takeLaunchUrlCalls())
1063-
.single.equals((url: url, mode: LaunchMode.platformDefault));
1063+
.single.equals((url: url, mode: LaunchMode.inAppBrowserView));
10641064

10651065
await tester.tap(find.byType(RealmContentNetworkImage));
10661066
check(testBinding.takeLaunchUrlCalls())
1067-
.single.equals((url: url, mode: LaunchMode.platformDefault));
1067+
.single.equals((url: url, mode: LaunchMode.inAppBrowserView));
10681068
debugNetworkImageHttpClientProvider = null;
10691069
});
10701070

@@ -1078,7 +1078,7 @@ void main() {
10781078

10791079
await tester.tap(find.byType(RealmContentNetworkImage));
10801080
check(testBinding.takeLaunchUrlCalls())
1081-
.single.equals((url: url, mode: LaunchMode.platformDefault));
1081+
.single.equals((url: url, mode: LaunchMode.inAppBrowserView));
10821082
debugNetworkImageHttpClientProvider = null;
10831083
});
10841084

@@ -1088,11 +1088,11 @@ void main() {
10881088

10891089
await tester.tap(find.text('Zulip — organized team chat'));
10901090
check(testBinding.takeLaunchUrlCalls())
1091-
.single.equals((url: url, mode: LaunchMode.platformDefault));
1091+
.single.equals((url: url, mode: LaunchMode.inAppBrowserView));
10921092

10931093
await tester.tap(find.byType(RealmContentNetworkImage));
10941094
check(testBinding.takeLaunchUrlCalls())
1095-
.single.equals((url: url, mode: LaunchMode.platformDefault));
1095+
.single.equals((url: url, mode: LaunchMode.inAppBrowserView));
10961096
debugNetworkImageHttpClientProvider = null;
10971097
});
10981098

@@ -1102,7 +1102,7 @@ void main() {
11021102

11031103
await tester.tap(find.byType(RealmContentNetworkImage));
11041104
check(testBinding.takeLaunchUrlCalls())
1105-
.single.equals((url: url, mode: LaunchMode.platformDefault));
1105+
.single.equals((url: url, mode: LaunchMode.inAppBrowserView));
11061106
debugNetworkImageHttpClientProvider = null;
11071107
});
11081108
});

‎test/widgets/profile_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ void main() {
164164
await tester.tap(find.text(testUrl));
165165
check(testBinding.takeLaunchUrlCalls()).single.equals((
166166
url: Uri.parse(testUrl),
167-
mode: LaunchMode.platformDefault,
167+
mode: LaunchMode.inAppBrowserView,
168168
));
169169
});
170170

@@ -191,7 +191,7 @@ void main() {
191191
await tester.tap(find.text('externalValue'));
192192
check(testBinding.takeLaunchUrlCalls()).single.equals((
193193
url: Uri.parse('http://example/externalValue'),
194-
mode: LaunchMode.platformDefault,
194+
mode: LaunchMode.inAppBrowserView,
195195
));
196196
});
197197

0 commit comments

Comments
 (0)