Skip to content

Commit 42803b4

Browse files
PIG208PIG208
committed
content: Follow browser preference setting when opening links
This preserves the default behavior (platform-dependent) when the preference is unset. Signed-off-by: Zixuan James Li <zixuan@zulip.com>
1 parent 23a3b24 commit 42803b4

File tree

6 files changed

+74
-21
lines changed

6 files changed

+74
-21
lines changed

‎lib/model/settings.dart‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import 'package:flutter/foundation.dart';
22

33
import '../generated/l10n/zulip_localizations.dart';
4+
import 'database.dart';
45

56
/// The visual theme of the app.
67
///
@@ -44,3 +45,19 @@ enum BrowserPreference {
4445
/// Use the user's default browser app.
4546
external,
4647
}
48+
49+
extension GlobalSettingsHelpers on GlobalSettingsData {
50+
BrowserPreference get effectiveBrowserPreference {
51+
if (browserPreference != null) {
52+
return browserPreference!;
53+
}
54+
return switch (defaultTargetPlatform) {
55+
// On iOS we prefer LaunchMode.externalApplication because (for
56+
// HTTP URLs) LaunchMode.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 => BrowserPreference.external,
60+
_ => BrowserPreference.embedded,
61+
};
62+
}
63+
}

‎lib/widgets/content.dart‎

Lines changed: 5 additions & 8 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/services.dart';
@@ -15,6 +14,7 @@ import '../model/avatar_url.dart';
1514
import '../model/binding.dart';
1615
import '../model/content.dart';
1716
import '../model/internal_link.dart';
17+
import '../model/settings.dart';
1818
import 'code_block.dart';
1919
import 'dialog.dart';
2020
import 'icons.dart';
@@ -1343,17 +1343,14 @@ void _launchUrl(BuildContext context, String urlString) async {
13431343
return;
13441344
}
13451345

1346+
final globalSettings = GlobalStoreWidget.of(context).globalSettings;
13461347
bool launched = false;
13471348
String? errorMessage;
13481349
try {
13491350
launched = await ZulipBinding.instance.launchUrl(url,
1350-
mode: switch (defaultTargetPlatform) {
1351-
// On iOS we prefer LaunchMode.externalApplication because (for
1352-
// HTTP URLs) LaunchMode.platformDefault uses SFSafariViewController,
1353-
// which gives an awkward UX as described here:
1354-
// https://chat.zulip.org/#narrow/stream/48-mobile/topic/in-app.20browser/near/1169118
1355-
TargetPlatform.iOS => UrlLaunchMode.externalApplication,
1356-
_ => UrlLaunchMode.platformDefault,
1351+
mode: switch (globalSettings.effectiveBrowserPreference) {
1352+
BrowserPreference.embedded => UrlLaunchMode.inAppBrowserView,
1353+
BrowserPreference.external => UrlLaunchMode.externalApplication,
13571354
});
13581355
} on PlatformException catch (e) {
13591356
errorMessage = e.message;

‎test/model/settings_test.dart‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
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/settings.dart';
5+
6+
import '../example_data.dart' as eg;
7+
import 'store_checks.dart';
8+
import 'store_test.dart';
9+
10+
void main() {
11+
group('effectiveBrowserPreference', () {
12+
testAndroidIos('use non-default values', () {
13+
final globalStore = eg.globalStore(globalSettings: eg.globalSettings(
14+
browserPreference: BrowserPreference.external));
15+
check(globalStore).globalSettings.effectiveBrowserPreference.equals(
16+
BrowserPreference.external);
17+
});
18+
19+
testAndroidIos('use default values by platform', () {
20+
check(eg.globalStore()).globalSettings.effectiveBrowserPreference.equals(
21+
defaultTargetPlatform == TargetPlatform.iOS
22+
? BrowserPreference.external : BrowserPreference.embedded);
23+
});
24+
});
25+
}

‎test/model/store_checks.dart‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,5 @@ extension PerAccountStoreChecks on Subject<PerAccountStore> {
5454

5555
extension GlobalSettingsDataChecks on Subject<GlobalSettingsData> {
5656
Subject<ThemeSetting> get themeSetting => has((x) => x.themeSetting, 'themeSetting');
57+
Subject<BrowserPreference> get effectiveBrowserPreference => has((x) => x.effectiveBrowserPreference, 'effectiveBrowserPreference');
5758
}

‎test/widgets/content_test.dart‎

Lines changed: 24 additions & 11 deletions
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';
@@ -482,7 +483,7 @@ void main() {
482483
final expectedLaunchUrl = expectedVideo.hrefUrl;
483484
await tester.tap(find.byIcon(Icons.play_arrow_rounded));
484485
check(testBinding.takeLaunchUrlCalls())
485-
.single.equals((url: Uri.parse(expectedLaunchUrl), mode: LaunchMode.platformDefault));
486+
.single.equals((url: Uri.parse(expectedLaunchUrl), mode: LaunchMode.inAppBrowserView));
486487
}
487488

488489
testWidgets('video preview for youtube embed', (tester) async {
@@ -754,11 +755,23 @@ void main() {
754755
await tapText(tester, find.text('hello'));
755756

756757
final expectedLaunchMode = defaultTargetPlatform == TargetPlatform.iOS ?
757-
LaunchMode.externalApplication : LaunchMode.platformDefault;
758+
LaunchMode.externalApplication : LaunchMode.inAppBrowserView;
758759
check(testBinding.takeLaunchUrlCalls())
759760
.single.equals((url: Uri.parse('https://example/'), mode: expectedLaunchMode));
760761
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
761762

763+
testWidgets('override default with browser preference setting', (tester) async {
764+
await testBinding.globalStore.updateGlobalSettings(
765+
eg.globalSettings(
766+
browserPreference: BrowserPreference.external).toCompanion(false));
767+
await prepare(tester,
768+
'<p><a href="https://example/">hello</a></p>');
769+
770+
await tapText(tester, find.text('hello'));
771+
check(testBinding.takeLaunchUrlCalls())
772+
.single.equals((url: Uri.parse('https://example/'), mode: LaunchMode.externalApplication));
773+
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
774+
762775
testWidgets('multiple links in paragraph', (tester) async {
763776
const fontSize = kBaseFontSize;
764777

@@ -772,19 +785,19 @@ void main() {
772785

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

777790
await tester.tapAt(base.translate(9*fontSize, 0)); // "foo bar bXz"
778791
check(testBinding.takeLaunchUrlCalls())
779-
.single.equals((url: Uri.parse('https://b/'), mode: LaunchMode.platformDefault));
792+
.single.equals((url: Uri.parse('https://b/'), mode: LaunchMode.inAppBrowserView));
780793
});
781794

782795
testWidgets('link nested in other spans', (tester) async {
783796
await prepare(tester,
784797
'<p><strong><em><a href="https://a/">word</a></em></strong></p>');
785798
await tapText(tester, find.text('word'));
786799
check(testBinding.takeLaunchUrlCalls())
787-
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.platformDefault));
800+
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.inAppBrowserView));
788801
});
789802

790803
testWidgets('link containing other spans', (tester) async {
@@ -797,27 +810,27 @@ void main() {
797810

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

802815
await tester.tapAt(base.translate(6*fontSize, 0)); // "two woXds"
803816
check(testBinding.takeLaunchUrlCalls())
804-
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.platformDefault));
817+
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.inAppBrowserView));
805818
});
806819

807820
testWidgets('relative links are resolved', (tester) async {
808821
await prepare(tester,
809822
'<p><a href="/a/b?c#d">word</a></p>');
810823
await tapText(tester, find.text('word'));
811824
check(testBinding.takeLaunchUrlCalls())
812-
.single.equals((url: Uri.parse('${eg.realmUrl}a/b?c#d'), mode: LaunchMode.platformDefault));
825+
.single.equals((url: Uri.parse('${eg.realmUrl}a/b?c#d'), mode: LaunchMode.inAppBrowserView));
813826
});
814827

815828
testWidgets('link inside HeadingNode', (tester) async {
816829
await prepare(tester,
817830
'<h6><a href="https://a/">word</a></h6>');
818831
await tapText(tester, find.text('word'));
819832
check(testBinding.takeLaunchUrlCalls())
820-
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.platformDefault));
833+
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.inAppBrowserView));
821834
});
822835

823836
testWidgets('error dialog if invalid link', (tester) async {
@@ -827,7 +840,7 @@ void main() {
827840
await tapText(tester, find.text('word'));
828841
await tester.pump();
829842
check(testBinding.takeLaunchUrlCalls())
830-
.single.equals((url: Uri.parse('file:///etc/bad'), mode: LaunchMode.platformDefault));
843+
.single.equals((url: Uri.parse('file:///etc/bad'), mode: LaunchMode.inAppBrowserView));
831844
checkErrorDialog(tester, expectedTitle: 'Unable to open link');
832845
});
833846
});
@@ -871,7 +884,7 @@ void main() {
871884
await tapText(tester, find.text('invalid'));
872885
final expectedUrl = eg.realmUrl.resolve('/#narrow/stream/1-check/topic');
873886
check(testBinding.takeLaunchUrlCalls())
874-
.single.equals((url: expectedUrl, mode: LaunchMode.platformDefault));
887+
.single.equals((url: expectedUrl, mode: LaunchMode.inAppBrowserView));
875888
check(pushedRoutes).isEmpty();
876889
});
877890
});

‎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)