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
BrowserPreference.getUrlLaunchMode, similar to ThemeSetting.displayName,
but that's a lot more verbose for the caller.

This also updates references to use the alias `UrlLaunchMode` in the
comment instead, since that's the type the developer sees in this
context.

Signed-off-by: Zixuan James Li <zixuan@zulip.com>

Author: PIG208

lib/model/settings.dart

@@ -1,4 +1,8 @@
+import 'package:flutter/foundation.dart';
+
 import '../generated/l10n/zulip_localizations.dart';
+import 'binding.dart';
+import 'database.dart';
 
 /// The user's choice of visual theme for the app.
 ///
@@ -34,8 +38,31 @@ enum ThemeSetting {
 /// Write a migration if such a change is necessary.
 enum BrowserPreference {
   /// Use the in-app browser for HTTP links.
+  ///
+  /// This translates to [UrlLaunchMode.platformDefault], so that non-in-app
+  /// browsers/apps will still be used for other types of links (e.g. mailto)
+  /// on both iOS and Android.
   inApp,
 
   /// Use the user's default browser app.
   external,
 }
+
+extension GlobalSettingsHelpers on GlobalSettingsData {
+  UrlLaunchMode get urlLaunchMode {
+    if (browserPreference != null) {
+      return switch (browserPreference!) {
+        BrowserPreference.inApp => UrlLaunchMode.platformDefault,
+        BrowserPreference.external => UrlLaunchMode.externalApplication,
+      };
+    }
+    return switch (defaultTargetPlatform) {
+      // On iOS we prefer UrlLaunchMode.externalApplication because (for
+      // HTTP URLs) UrlLaunchMode.platformDefault uses SFSafariViewController,
+      // which gives an awkward UX as described here:
+      //  https://chat.zulip.org/#narrow/stream/48-mobile/topic/in-app.20browser/near/1169118
+      TargetPlatform.iOS => UrlLaunchMode.externalApplication,
+      _ => UrlLaunchMode.platformDefault,
+    };
+  }
+}

lib/widgets/content.dart

@@ -1,7 +1,6 @@
 import 'dart:async';
 
 import 'package:flutter/cupertino.dart';
-import 'package:flutter/foundation.dart';
 import 'package:flutter/gestures.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/rendering.dart';
@@ -16,6 +15,7 @@ import '../model/avatar_url.dart';
 import '../model/binding.dart';
 import '../model/content.dart';
 import '../model/internal_link.dart';
+import '../model/settings.dart';
 import 'code_block.dart';
 import 'dialog.dart';
 import 'icons.dart';
@@ -1439,18 +1439,12 @@ void _launchUrl(BuildContext context, String urlString) async {
     return;
   }
 
+  final globalSettings = GlobalStoreWidget.of(context).globalSettings;
   bool launched = false;
   String? errorMessage;
   try {
     launched = await ZulipBinding.instance.launchUrl(url,
-      mode: switch (defaultTargetPlatform) {
-        // On iOS we prefer LaunchMode.externalApplication because (for
-        // HTTP URLs) LaunchMode.platformDefault uses SFSafariViewController,
-        // which gives an awkward UX as described here:
-        //  https://chat.zulip.org/#narrow/stream/48-mobile/topic/in-app.20browser/near/1169118
-        TargetPlatform.iOS => UrlLaunchMode.externalApplication,
-        _ => UrlLaunchMode.platformDefault,
-      });
+      mode: globalSettings.urlLaunchMode);
   } on PlatformException catch (e) {
     errorMessage = e.message;
   }

test/model/settings_test.dart

@@ -0,0 +1,26 @@
+import 'package:checks/checks.dart';
+import 'package:flutter/foundation.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:zulip/model/binding.dart';
+import 'package:zulip/model/settings.dart';
+
+import '../example_data.dart' as eg;
+import 'store_checks.dart';
+import 'store_test.dart';
+
+void main() {
+  group('urlLaunchMode', () {
+    testAndroidIos('globalSettings.browserPreference is null; use platform-specific defaults', () {
+      check(eg.globalStore()).globalSettings.urlLaunchMode.equals(
+        defaultTargetPlatform == TargetPlatform.iOS
+          ? UrlLaunchMode.externalApplication : UrlLaunchMode.platformDefault);
+    });
+
+    testAndroidIos('globalSettings.browserPreference is non-null; follow the preference', () {
+      final globalStore = eg.globalStore(globalSettings: eg.globalSettings(
+        browserPreference: BrowserPreference.external));
+      check(globalStore).globalSettings.urlLaunchMode.equals(
+        UrlLaunchMode.externalApplication);
+    });
+  });
+}

test/model/store_checks.dart

@@ -3,6 +3,7 @@ import 'package:zulip/api/core.dart';
 import 'package:zulip/api/model/initial_snapshot.dart';
 import 'package:zulip/api/model/model.dart';
 import 'package:zulip/model/autocomplete.dart';
+import 'package:zulip/model/binding.dart';
 import 'package:zulip/model/database.dart';
 import 'package:zulip/model/recent_dm_conversations.dart';
 import 'package:zulip/model/settings.dart';
@@ -32,6 +33,7 @@ extension GlobalStoreChecks on Subject<GlobalStore> {
 extension GlobalSettingsDataChecks on Subject<GlobalSettingsData> {
   Subject<ThemeSetting?> get themeSetting => has((x) => x.themeSetting, 'themeSetting');
   Subject<BrowserPreference?> get browserPreference => has((x) => x.browserPreference, 'browserPreference');
+  Subject<UrlLaunchMode> get urlLaunchMode => has((x) => x.urlLaunchMode, 'urlLaunchMode');
 }
 
 extension PerAccountStoreChecks on Subject<PerAccountStore> {

test/widgets/content_test.dart

@@ -9,6 +9,7 @@ import 'package:url_launcher/url_launcher.dart';
 import 'package:zulip/api/core.dart';
 import 'package:zulip/model/content.dart';
 import 'package:zulip/model/narrow.dart';
+import 'package:zulip/model/settings.dart';
 import 'package:zulip/model/store.dart';
 import 'package:zulip/widgets/content.dart';
 import 'package:zulip/widgets/icons.dart';
@@ -798,6 +799,18 @@ void main() {
         .single.equals((url: Uri.parse('https://example/'), mode: expectedLaunchMode));
     }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
 
+    testWidgets('follow browser preference setting to open URL', (tester) async {
+      await testBinding.globalStore.updateGlobalSettings(
+        eg.globalSettings(
+          browserPreference: BrowserPreference.inApp).toCompanion(false));
+      await prepare(tester,
+        '<p><a href="https://example/">hello</a></p>');
+
+      await tapText(tester, find.text('hello'));
+      check(testBinding.takeLaunchUrlCalls()).single.equals((
+        url: Uri.parse('https://example/'), mode: LaunchMode.platformDefault));
+    }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
+
     testWidgets('multiple links in paragraph', (tester) async {
       const fontSize = kBaseFontSize;