msglist: Give error alert when tapped link has URL that didn't parse

Just before this commit, if an `href` fails to parse, it would do so
in the `new URL` call that's meant to pass a URL object to
`getNarrowFromLink` (the line with the comment "TODO: Use parsedUrl,
below").

The error would propagate up to messageLinkPress's caller, in
handleWebViewOutboundEvent, and from there to a React event handler,
with the result (I expect) that the app wouldn't crash, but it also
wouldn't give any useful feedback to the user.

So, give some useful feedback to the user. The issue is filed as
"Fail early (and tell user) on a message-link tap when the URL
doesn't parse" (#5518) . I guess we have been failing early, we just
haven't been telling the user.

Fixes: #5518

Author: chrisbobbe

src/message/messagesActions.js

@@ -1,7 +1,7 @@
 /* @flow strict-local */
 import * as logging from '../utils/logging';
 import * as NavigationService from '../nav/NavigationService';
-import type { Message, Narrow, ThunkAction } from '../types';
+import type { GetText, Message, Narrow, ThunkAction } from '../types';
 import { getAuth, getRealm, getMessages, getZulipFeatureLevel } from '../selectors';
 import { getNearOperandFromLink, getNarrowFromLink } from '../utils/internalLinks';
 import { openLinkWithUserPreference } from '../utils/openLink';
@@ -21,6 +21,7 @@ import {
   caseNarrowDefault,
 } from '../utils/narrow';
 import { hasMessageEverBeenInStream, hasMessageEverHadTopic } from './messageSelectors';
+import { showErrorAlert } from '../utils/info';
 
 /**
  * Navigate to the given narrow.
@@ -215,24 +216,23 @@ const doNarrowNearLink =
  * Handle a link tap that isn't an image we want to show in the lightbox.
  */
 export const messageLinkPress =
-  (href: string): ThunkAction<Promise<void>> =>
+  (href: string, _: GetText): ThunkAction<Promise<void>> =>
   async (dispatch, getState, { getGlobalSettings }) => {
     const state = getState();
     const auth = getAuth(state);
     const streamsById = getStreamsById(state);
     const streamsByName = getStreamsByName(state);
     const ownUserId = getOwnUserId(state);
-    const narrow = getNarrowFromLink(
-      new URL(href, auth.realm), // TODO: Use parsedUrl, below.
-      auth.realm,
-      streamsById,
-      streamsByName,
-      ownUserId,
-    );
 
     const parsedUrl = tryParseUrl(href, auth.realm);
+    if (!parsedUrl) {
+      showErrorAlert(_('Cannot open link'), _('Invalid URL.'));
+      return;
+    }
     // TODO: Replace all uses of `href` below with `parsedUrl`.
 
+    const narrow = getNarrowFromLink(parsedUrl, auth.realm, streamsById, streamsByName, ownUserId);
+
     // TODO: In some cases getNarrowFromLink successfully parses the link, but
     //   finds it points somewhere we can't see: in particular, to a stream
     //   that's hidden from our user (perhaps doesn't exist.)  For those,
@@ -247,7 +247,7 @@ export const messageLinkPress =
       }
 
       await dispatch(doNarrowNearLink(narrow, nearOperand));
-    } else if (!parsedUrl || !isUrlOnRealm(parsedUrl, auth.realm)) {
+    } else if (!isUrlOnRealm(parsedUrl, auth.realm)) {
       openLinkWithUserPreference(href, getGlobalSettings());
     } else {
       const url = (await api.tryGetFileTemporaryUrl(parsedUrl, auth)) ?? parsedUrl;

src/webview/handleOutboundEvents.js

@@ -313,7 +313,7 @@ export const handleWebViewOutboundEvent = (
       if (isUrlAnImage(event.href)) {
         handleImage(props, navigation, event.href, event.messageId);
       } else {
-        props.dispatch(messageLinkPress(event.href));
+        props.dispatch(messageLinkPress(event.href, props._));
       }
       break;
 

static/translations/messages_en.json

@@ -12,6 +12,8 @@
   "Notify everyone": "Notify everyone",
   "Cannot open image": "Cannot open image",
   "Invalid image URL.": "Invalid image URL.",
+  "Cannot open link": "Cannot open link",
+  "Invalid URL.": "Invalid URL.",
   "Invisible mode": "Invisible mode",
   "{num_of_people, plural,\n  one {This message has been <z-link>read</z-link> by {num_of_people} person:}\n  other {This message has been <z-link>read</z-link> by {num_of_people} people:}}": "{num_of_people, plural,\n  one {This message has been <z-link>read</z-link> by {num_of_people} person:}\n  other {This message has been <z-link>read</z-link> by {num_of_people} people:}}",
   "View read receipts": "View read receipts",