Skip to content

Commit f06b14c

Browse files
PIG208gnprice
andcommitted
setting [nfc]: 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 is NFC given the current behavior of url_launcher, which appears to be that platformDefault means an in-app browser for at least HTTP links. Co-authored-by: Greg Price <[email protected]> Signed-off-by: Zixuan James Li <[email protected]>
1 parent 89bf840 commit f06b14c

File tree

6 files changed

+97
-27
lines changed

6 files changed

+97
-27
lines changed

lib/model/settings.dart

+42
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,46 @@ 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) where a browser won't work,
43+
/// this falls back to [UrlLaunchMode.platformDefault].
3744
inApp,
3845

3946
/// Use the user's default browser app.
4047
external,
4148
}
49+
50+
extension GlobalSettingsHelpers on GlobalSettingsData {
51+
BrowserPreference get defaultBrowserPreference {
52+
return switch (defaultTargetPlatform) {
53+
// On iOS we prefer UrlLaunchMode.externalApplication because (for
54+
// HTTP URLs) UrlLaunchMode.platformDefault uses SFSafariViewController,
55+
// which gives an awkward UX as described here:
56+
// https://chat.zulip.org/#narrow/stream/48-mobile/topic/in-app.20browser/near/1169118
57+
TargetPlatform.iOS => BrowserPreference.external,
58+
59+
// On Android we prefer an in-app browser. See discussion from 2021:
60+
// https://chat.zulip.org/#narrow/channel/48-mobile/topic/in-app.20browser/near/1169095
61+
// That's also the `url_launcher` default (at least as of 2025).
62+
_ => BrowserPreference.inApp,
63+
};
64+
}
65+
66+
UrlLaunchMode getUrlLaunchMode(Uri url) {
67+
switch (defaultBrowserPreference) {
68+
case BrowserPreference.inApp:
69+
if (!(url.scheme == 'https' || url.scheme == 'http')) {
70+
// For URLs on non-HTTP schemes such as `mailto`,
71+
// `url_launcher.launchUrl` rejects `inAppBrowserView` with an error:
72+
// https://github.com/flutter/packages/blob/9cc6f370/packages/url_launcher/url_launcher/lib/src/url_launcher_uri.dart#L46-L51
73+
// TODO(upstream/url_launcher): should fall back in this case instead
74+
// (as the `launchUrl` doc already says it may do).
75+
return UrlLaunchMode.platformDefault;
76+
}
77+
return UrlLaunchMode.inAppBrowserView;
78+
79+
case BrowserPreference.external:
80+
return UrlLaunchMode.externalApplication;
81+
}
82+
}
83+
}

lib/widgets/content.dart

+3-9
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

+31
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

+3
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

+16-16
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

+2-2
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)