Skip to content

Commit d1242a2

Browse files
committed
content [nfc]: Follow browser preference setting when opening links
This preserves the previous behavior (external application on iOS, and platform default on Android) when browserPreference is `null`. There is no way for the client to update the setting for now, hence the NFC. The helper could have been a static method instead of an extension, similar to ThemeSetting.displayName, but that will be a lot more verbose for the caller. Signed-off-by: Zixuan James Li <[email protected]>
1 parent 487d40d commit d1242a2

File tree

5 files changed

+91
-9
lines changed

5 files changed

+91
-9
lines changed

lib/model/settings.dart

+28
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,32 @@ 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+
UrlLaunchMode getUrlLaunchMode(Uri url) {
53+
if (browserPreference == null) {
54+
return switch (defaultTargetPlatform) {
55+
// On iOS we prefer UrlLaunchMode.externalApplication because (for
56+
// HTTP URLs) UrlLaunchMode.platformDefault uses SFSafariViewController,
57+
// which gives an awkward UX as described here:
58+
// https://chat.zulip.org/#narrow/stream/48-mobile/topic/in-app.20browser/near/1169118
59+
TargetPlatform.iOS => UrlLaunchMode.externalApplication,
60+
_ => UrlLaunchMode.platformDefault,
61+
};
62+
}
63+
return switch (browserPreference!) {
64+
BrowserPreference.inApp => url.scheme == 'http' || url.scheme == 'https'
65+
? UrlLaunchMode.inAppBrowserView : UrlLaunchMode.platformDefault,
66+
BrowserPreference.external => UrlLaunchMode.externalApplication,
67+
};
68+
}
69+
}

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

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
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+
import 'package:zulip/model/settings.dart';
6+
7+
import '../example_data.dart' as eg;
8+
import 'store_checks.dart';
9+
import 'store_test.dart';
10+
11+
void main() {
12+
final httpLink = Uri.parse('http://chat.zulip.org');
13+
final nonHttpLink = Uri.parse('mailto:[email protected]');
14+
15+
group('getUrlLaunchMode', () {
16+
testAndroidIos('globalSettings.browserPreference is null; use platform-specific defaults', () {
17+
final globalStore = eg.globalStore(globalSettings: eg.globalSettings(
18+
browserPreference: null));
19+
check(globalStore).globalSettings.getUrlLaunchMode(httpLink).equals(
20+
defaultTargetPlatform == TargetPlatform.iOS
21+
? UrlLaunchMode.externalApplication : UrlLaunchMode.platformDefault);
22+
});
23+
24+
testAndroidIos('globalSettings.browserPreference is inApp; follow the preference for http links', () {
25+
final globalStore = eg.globalStore(globalSettings: eg.globalSettings(
26+
browserPreference: BrowserPreference.inApp));
27+
check(globalStore).globalSettings.getUrlLaunchMode(httpLink).equals(
28+
UrlLaunchMode.inAppBrowserView);
29+
});
30+
31+
testAndroidIos('globalSettings.browserPreference is inApp; use platform default for non-http links', () {
32+
final globalStore = eg.globalStore(globalSettings: eg.globalSettings(
33+
browserPreference: BrowserPreference.inApp));
34+
check(globalStore).globalSettings.getUrlLaunchMode(nonHttpLink).equals(
35+
UrlLaunchMode.platformDefault);
36+
});
37+
38+
testAndroidIos('globalSettings.browserPreference is external; follow the preference', () {
39+
final globalStore = eg.globalStore(globalSettings: eg.globalSettings(
40+
browserPreference: BrowserPreference.external));
41+
check(globalStore).globalSettings.getUrlLaunchMode(httpLink).equals(
42+
UrlLaunchMode.externalApplication);
43+
});
44+
});
45+
}

test/model/store_checks.dart

+2
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,7 @@ 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<UrlLaunchMode> getUrlLaunchMode(Uri url) => has((x) => x.getUrlLaunchMode(url), 'getUrlLaunchMode');
3537
}
3638

3739
extension PerAccountStoreChecks on Subject<PerAccountStore> {

test/widgets/content_test.dart

+13
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import 'package:url_launcher/url_launcher.dart';
99
import 'package:zulip/api/core.dart';
1010
import 'package:zulip/model/content.dart';
1111
import 'package:zulip/model/narrow.dart';
12+
import 'package:zulip/model/settings.dart';
1213
import 'package:zulip/model/store.dart';
1314
import 'package:zulip/widgets/content.dart';
1415
import 'package:zulip/widgets/icons.dart';
@@ -798,6 +799,18 @@ void main() {
798799
.single.equals((url: Uri.parse('https://example/'), mode: expectedLaunchMode));
799800
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
800801

802+
testWidgets('follow browser preference setting to open URL', (tester) async {
803+
await testBinding.globalStore.updateGlobalSettings(
804+
eg.globalSettings(
805+
browserPreference: BrowserPreference.inApp).toCompanion(false));
806+
await prepare(tester,
807+
'<p><a href="https://example/">hello</a></p>');
808+
809+
await tapText(tester, find.text('hello'));
810+
check(testBinding.takeLaunchUrlCalls()).single.equals((
811+
url: Uri.parse('https://example/'), mode: LaunchMode.inAppBrowserView));
812+
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
813+
801814
testWidgets('multiple links in paragraph', (tester) async {
802815
const fontSize = kBaseFontSize;
803816

0 commit comments

Comments
 (0)