Skip to content

Commit 70d2d19

Browse files
PIG208gnprice
authored andcommitted
content [nfc]: Follow browser preference setting when opening links
This preserves the previous behavior (external application on iOS, and in-app browser 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 f06b14c commit 70d2d19

File tree

4 files changed

+41
-5
lines changed

4 files changed

+41
-5
lines changed

lib/model/settings.dart

+3-2
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ enum BrowserPreference {
4848
}
4949

5050
extension GlobalSettingsHelpers on GlobalSettingsData {
51-
BrowserPreference get defaultBrowserPreference {
51+
BrowserPreference get effectiveBrowserPreference {
52+
if (browserPreference != null) return browserPreference!;
5253
return switch (defaultTargetPlatform) {
5354
// On iOS we prefer UrlLaunchMode.externalApplication because (for
5455
// HTTP URLs) UrlLaunchMode.platformDefault uses SFSafariViewController,
@@ -64,7 +65,7 @@ extension GlobalSettingsHelpers on GlobalSettingsData {
6465
}
6566

6667
UrlLaunchMode getUrlLaunchMode(Uri url) {
67-
switch (defaultBrowserPreference) {
68+
switch (effectiveBrowserPreference) {
6869
case BrowserPreference.inApp:
6970
if (!(url.scheme == 'https' || url.scheme == 'http')) {
7071
// For URLs on non-HTTP schemes such as `mailto`,

test/model/settings_test.dart

+24-2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import 'package:checks/checks.dart';
22
import 'package:flutter/foundation.dart';
33
import 'package:flutter_test/flutter_test.dart';
44
import 'package:zulip/model/binding.dart';
5+
import 'package:zulip/model/settings.dart';
56

67
import '../example_data.dart' as eg;
78
import 'store_checks.dart';
@@ -12,20 +13,41 @@ void main() {
1213
final nonHttpLink = Uri.parse('mailto:[email protected]');
1314

1415
group('getUrlLaunchMode', () {
15-
testAndroidIos('use our per-platform defaults for HTTP links', () {
16+
testAndroidIos('globalSettings.browserPreference is null; use our per-platform defaults for HTTP links', () {
1617
final globalStore = eg.globalStore(globalSettings: eg.globalSettings(
1718
browserPreference: null));
1819
check(globalStore).globalSettings.getUrlLaunchMode(httpLink).equals(
1920
defaultTargetPlatform == TargetPlatform.iOS
2021
? UrlLaunchMode.externalApplication : UrlLaunchMode.inAppBrowserView);
2122
});
2223

23-
testAndroidIos('use our per-platform defaults for non-HTTP links', () {
24+
testAndroidIos('globalSettings.browserPreference is null; use our per-platform defaults for non-HTTP links', () {
2425
final globalStore = eg.globalStore(globalSettings: eg.globalSettings(
2526
browserPreference: null));
2627
check(globalStore).globalSettings.getUrlLaunchMode(nonHttpLink).equals(
2728
defaultTargetPlatform == TargetPlatform.android
2829
? UrlLaunchMode.platformDefault : UrlLaunchMode.externalApplication);
2930
});
31+
32+
testAndroidIos('globalSettings.browserPreference is inApp; follow the user preference for http links', () {
33+
final globalStore = eg.globalStore(globalSettings: eg.globalSettings(
34+
browserPreference: BrowserPreference.inApp));
35+
check(globalStore).globalSettings.getUrlLaunchMode(httpLink).equals(
36+
UrlLaunchMode.inAppBrowserView);
37+
});
38+
39+
testAndroidIos('globalSettings.browserPreference is inApp; use platform default for non-http links', () {
40+
final globalStore = eg.globalStore(globalSettings: eg.globalSettings(
41+
browserPreference: BrowserPreference.inApp));
42+
check(globalStore).globalSettings.getUrlLaunchMode(nonHttpLink).equals(
43+
UrlLaunchMode.platformDefault);
44+
});
45+
46+
testAndroidIos('globalSettings.browserPreference is external; follow the user preference', () {
47+
final globalStore = eg.globalStore(globalSettings: eg.globalSettings(
48+
browserPreference: BrowserPreference.external));
49+
check(globalStore).globalSettings.getUrlLaunchMode(httpLink).equals(
50+
UrlLaunchMode.externalApplication);
51+
});
3052
});
3153
}

test/model/store_checks.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ extension GlobalStoreChecks on Subject<GlobalStore> {
3333
extension GlobalSettingsDataChecks on Subject<GlobalSettingsData> {
3434
Subject<ThemeSetting?> get themeSetting => has((x) => x.themeSetting, 'themeSetting');
3535
Subject<BrowserPreference?> get browserPreference => has((x) => x.browserPreference, 'browserPreference');
36-
Subject<BrowserPreference> get defaultBrowserPreference => has((x) => x.defaultBrowserPreference, 'defaultBrowserPreference');
36+
Subject<BrowserPreference> get effectiveBrowserPreference => has((x) => x.effectiveBrowserPreference, 'effectiveBrowserPreference');
3737
Subject<UrlLaunchMode> getUrlLaunchMode(Uri url) => has((x) => x.getUrlLaunchMode(url), 'getUrlLaunchMode');
3838
}
3939

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)