Skip to content

Commit bcfd60a

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 <[email protected]>
1 parent 6bd8c76 commit bcfd60a

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 user's choice of visual theme for 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/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,17 +1439,14 @@ 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,
1447+
mode: switch (globalSettings.effectiveBrowserPreference) {
1448+
BrowserPreference.embedded => UrlLaunchMode.inAppBrowserView,
1449+
BrowserPreference.external => UrlLaunchMode.externalApplication,
14531450
});
14541451
} on PlatformException catch (e) {
14551452
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
@@ -53,4 +53,5 @@ extension PerAccountStoreChecks on Subject<PerAccountStore> {
5353

5454
extension GlobalSettingsDataChecks on Subject<GlobalSettingsData> {
5555
Subject<ThemeSetting?> get themeSetting => has((x) => x.themeSetting, 'themeSetting');
56+
Subject<BrowserPreference> get effectiveBrowserPreference => has((x) => x.effectiveBrowserPreference, 'effectiveBrowserPreference');
5657
}

‎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';
@@ -521,7 +522,7 @@ void main() {
521522
final expectedLaunchUrl = expectedVideo.hrefUrl;
522523
await tester.tap(find.byIcon(Icons.play_arrow_rounded));
523524
check(testBinding.takeLaunchUrlCalls())
524-
.single.equals((url: Uri.parse(expectedLaunchUrl), mode: LaunchMode.platformDefault));
525+
.single.equals((url: Uri.parse(expectedLaunchUrl), mode: LaunchMode.inAppBrowserView));
525526
}
526527

527528
testWidgets('video preview for youtube embed', (tester) async {
@@ -793,11 +794,23 @@ void main() {
793794
await tapText(tester, find.text('hello'));
794795

795796
final expectedLaunchMode = defaultTargetPlatform == TargetPlatform.iOS ?
796-
LaunchMode.externalApplication : LaunchMode.platformDefault;
797+
LaunchMode.externalApplication : LaunchMode.inAppBrowserView;
797798
check(testBinding.takeLaunchUrlCalls())
798799
.single.equals((url: Uri.parse('https://example/'), mode: expectedLaunchMode));
799800
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
800801

802+
testWidgets('override default with browser preference setting', (tester) async {
803+
await testBinding.globalStore.updateGlobalSettings(
804+
eg.globalSettings(
805+
browserPreference: BrowserPreference.external).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())
811+
.single.equals((url: Uri.parse('https://example/'), mode: LaunchMode.externalApplication));
812+
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
813+
801814
testWidgets('multiple links in paragraph', (tester) async {
802815
const fontSize = kBaseFontSize;
803816

@@ -811,19 +824,19 @@ void main() {
811824

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

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

821834
testWidgets('link nested in other spans', (tester) async {
822835
await prepare(tester,
823836
'<p><strong><em><a href="https://a/">word</a></em></strong></p>');
824837
await tapText(tester, find.text('word'));
825838
check(testBinding.takeLaunchUrlCalls())
826-
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.platformDefault));
839+
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.inAppBrowserView));
827840
});
828841

829842
testWidgets('link containing other spans', (tester) async {
@@ -836,27 +849,27 @@ void main() {
836849

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

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

846859
testWidgets('relative links are resolved', (tester) async {
847860
await prepare(tester,
848861
'<p><a href="/a/b?c#d">word</a></p>');
849862
await tapText(tester, find.text('word'));
850863
check(testBinding.takeLaunchUrlCalls())
851-
.single.equals((url: Uri.parse('${eg.realmUrl}a/b?c#d'), mode: LaunchMode.platformDefault));
864+
.single.equals((url: Uri.parse('${eg.realmUrl}a/b?c#d'), mode: LaunchMode.inAppBrowserView));
852865
});
853866

854867
testWidgets('link inside HeadingNode', (tester) async {
855868
await prepare(tester,
856869
'<h6><a href="https://a/">word</a></h6>');
857870
await tapText(tester, find.text('word'));
858871
check(testBinding.takeLaunchUrlCalls())
859-
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.platformDefault));
872+
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.inAppBrowserView));
860873
});
861874

862875
testWidgets('error dialog if invalid link', (tester) async {
@@ -866,7 +879,7 @@ void main() {
866879
await tapText(tester, find.text('word'));
867880
await tester.pump();
868881
check(testBinding.takeLaunchUrlCalls())
869-
.single.equals((url: Uri.parse('file:///etc/bad'), mode: LaunchMode.platformDefault));
882+
.single.equals((url: Uri.parse('file:///etc/bad'), mode: LaunchMode.inAppBrowserView));
870883
checkErrorDialog(tester, expectedTitle: 'Unable to open link');
871884
});
872885
});
@@ -910,7 +923,7 @@ void main() {
910923
await tapText(tester, find.text('invalid'));
911924
final expectedUrl = eg.realmUrl.resolve('/#narrow/stream/1-check/topic');
912925
check(testBinding.takeLaunchUrlCalls())
913-
.single.equals((url: expectedUrl, mode: LaunchMode.platformDefault));
926+
.single.equals((url: expectedUrl, mode: LaunchMode.inAppBrowserView));
914927
check(pushedRoutes).isEmpty();
915928
});
916929
});

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