openLink: Have link-opening functions take URL objects, not strings

For the following URLs, this adds a round-trip through the URL
constructor before passing it along to be opened:

- https://zulip.readthedocs.io/en/stable/overview/release-lifecycle.html#compatibility-and-upgrading
- https://zulip.readthedocs.io/en/stable/production/mobile-push-notifications.html
- https://zulip.com/policies/?nav=no
- https://itunes.apple.com/app/zulip/id1203036395
- https://play.google.com/store/apps/details?id=com.zulipmobile
- the URL passed to webAuth.openBrowser (which was actually already
  round-tripped through the URL constructor; see its caller)
- A tapped link in a message, when the link is off-realm

We expect just the last of these to have a functional change: it'll
make #4136 apply more widely ("[iOS] Links get double-%-encoded,
breaking downloads"):
  #5650 (comment)
 But we'll fix #4136 in the next commit.

Author: chrisbobbe

src/common/ServerCompatBanner.js

@@ -101,7 +101,9 @@ export default function ServerCompatBanner(props: Props): Node {
             openLinkWithUserPreference(
               // TODO: Instead, link to new Help Center doc once we have it:
               //   https://github.com/zulip/zulip/issues/23842
-              'https://zulip.readthedocs.io/en/stable/overview/release-lifecycle.html#compatibility-and-upgrading',
+              new URL(
+                'https://zulip.readthedocs.io/en/stable/overview/release-lifecycle.html#compatibility-and-upgrading',
+              ),
               settings,
             );
           },

src/common/ServerPushSetupBanner.js

@@ -82,7 +82,7 @@ export default function PushNotifsSetupBanner(props: Props): Node {
     label: 'Learn more',
     onPress: () => {
       openLinkWithUserPreference(
-        'https://zulip.readthedocs.io/en/stable/production/mobile-push-notifications.html',
+        new URL('https://zulip.readthedocs.io/en/stable/production/mobile-push-notifications.html'),
         settings,
       );
     },

src/common/WebLink.js

@@ -47,7 +47,7 @@ export default function WebLink(props: Props): React.Node {
     <ZulipText
       style={componentStyles.link}
       onPress={() => {
-        openLinkWithUserPreference(url.toString(), globalSettings);
+        openLinkWithUserPreference(url, globalSettings);
       }}
       {...zulipTextProps}
     />

src/lightbox/LightboxActionSheet.js

@@ -39,7 +39,7 @@ type ButtonType = {|
 const tryToDownloadImage = async ({ src, auth }: DownloadImageType) => {
   const tempUrl = await api.tryGetFileTemporaryUrl(src, auth);
   if (tempUrl === null) {
-    openLinkEmbedded(src.toString());
+    openLinkEmbedded(src);
     return;
   }
 

src/lightbox/shareImage.js

@@ -28,7 +28,7 @@ export default async (url: URL, auth: Auth) => {
     //
     // TODO(?): Could find a better way to convey this.
     showToast('Please share the image from your browser');
-    openLinkEmbedded(url.toString());
+    openLinkEmbedded(url);
     return;
   }
 

src/message/messagesActions.js

@@ -248,9 +248,9 @@ export const messageLinkPress =
 
       await dispatch(doNarrowNearLink(narrow, nearOperand));
     } else if (!isUrlOnRealm(parsedUrl, auth.realm)) {
-      openLinkWithUserPreference(href, getGlobalSettings());
+      openLinkWithUserPreference(parsedUrl, getGlobalSettings());
     } else {
       const url = (await api.tryGetFileTemporaryUrl(parsedUrl, auth)) ?? parsedUrl;
-      openLinkWithUserPreference(url.toString(), getGlobalSettings());
+      openLinkWithUserPreference(url, getGlobalSettings());
     }
   };

src/settings/LegalScreen.js

@@ -25,11 +25,11 @@ export default function LegalScreen(props: Props): Node {
   const globalSettings = useGlobalSelector(getGlobalSettings);
 
   const openZulipPolicies = useCallback(() => {
-    openLinkWithUserPreference('https://zulip.com/policies/?nav=no', globalSettings);
+    openLinkWithUserPreference(new URL('https://zulip.com/policies/?nav=no'), globalSettings);
   }, [globalSettings]);
 
   const openRealmPolicies = useCallback(() => {
-    openLinkWithUserPreference(new URL('/policies/?nav=no', realm).toString(), globalSettings);
+    openLinkWithUserPreference(new URL('/policies/?nav=no', realm), globalSettings);
   }, [realm, globalSettings]);
 
   return (

src/start/AuthScreen.js

@@ -295,7 +295,7 @@ class AuthScreenInner extends PureComponent<Props> {
       id_token: credential.identityToken,
     });
 
-    openLinkEmbedded(new URL(`/complete/apple/?${params}`, serverSettings.realm_uri).toString());
+    openLinkEmbedded(new URL(`/complete/apple/?${params}`, serverSettings.realm_uri));
 
     // Currently, the rest is handled with the `zulip://` redirect,
     // same as in the web flow.

src/start/CompatibilityScreen.js

@@ -47,10 +47,10 @@ function GooglePlayBadge() {
 }
 
 export default class CompatibilityScreen extends PureComponent<{||}> {
-  storeURL: string =
+  storeURL: URL =
     Platform.OS === 'ios'
-      ? 'https://itunes.apple.com/app/zulip/id1203036395'
-      : 'https://play.google.com/store/apps/details?id=com.zulipmobile';
+      ? new URL('https://itunes.apple.com/app/zulip/id1203036395')
+      : new URL('https://play.google.com/store/apps/details?id=com.zulipmobile');
 
   openStoreURL: () => void = () => {
     openLinkExternal(this.storeURL);

src/start/webAuth.js

@@ -47,8 +47,12 @@ export const generateRandomToken = async (): Promise<string> => {
 // in its response to protect against credentials intercept
 export const generateOtp = async (): Promise<string> => generateRandomToken();
 
+// TODO: Take URL object instead of string (but make a copy on which to
+//   mutate `searchParams`)
 export const openBrowser = (url: string, otp: string) => {
-  openLinkEmbedded(`${url}?mobile_flow_otp=${otp}`);
+  const urlCopy = new URL(url);
+  urlCopy.searchParams.set('mobile_flow_otp', otp);
+  openLinkEmbedded(urlCopy);
 };
 
 export const closeBrowser = () => {