[MM-32946][MM-40602] Ensure URL is valid before showing tooltip link, allow parsed URLs that aren't valid to open in browser (#1925)

* [MM-40602] Ensure URL is valid before showing tooltip link

* Rework to allow invalid URLs to display

* [MM-32946] Allow parsable URLs and open invalid URIs in browser

src/common/utils/url.ts

@@ -40,7 +40,11 @@ function getHost(inputURL: URL | string) {
 
 // isInternalURL determines if the target url is internal to the application.
 // - currentURL is the current url inside the webview
-function isInternalURL(targetURL: URL, currentURL: URL) {
+function isInternalURL(targetURL: URL | undefined, currentURL: URL) {
+    if (!targetURL) {
+        return false;
+    }
+
     if (targetURL.host !== currentURL.host) {
         return false;
     }

src/main/views/MattermostView.test.js

@@ -297,6 +297,12 @@ describe('main/views/MattermostView', () => {
             expect(mattermostView.emit).toHaveBeenCalledWith(UPDATE_TARGET_URL, 'http://server-2.com/some/other/path');
         });
 
+        it('should not throw error if URL is invalid', () => {
+            expect(() => {
+                mattermostView.handleUpdateTarget(null, 'not<a-real;;url');
+            }).not.toThrow();
+        });
+
         it('should not emit tooltip URL if internal', () => {
             mattermostView.handleUpdateTarget(null, 'http://server-1.com/path/to/channels');
             expect(mattermostView.emit).not.toHaveBeenCalled();

src/main/views/MattermostView.ts

@@ -311,7 +311,7 @@ export class MattermostView extends EventEmitter {
     }
 
     handleUpdateTarget = (e: Event, url: string) => {
-        if (!url || !urlUtils.isInternalURL(new URL(url), this.tab.server.url)) {
+        if (url && !urlUtils.isInternalURL(urlUtils.parseURL(url), this.tab.server.url)) {
             this.emit(UPDATE_TARGET_URL, url);
         }
     }

src/main/views/webContentEvents.test.js

@@ -183,9 +183,10 @@ describe('main/views/webContentsEvents', () => {
             expect(newWindow({url: 'devtools://aaaaaa.com'})).toStrictEqual({action: 'allow'});
         });
 
-        it('should deny invalid URI', () => {
+        it('should open invalid URIs in browser', () => {
             urlUtils.isValidURI.mockReturnValue(false);
-            expect(newWindow({url: 'http::'})).toStrictEqual({action: 'deny'});
+            expect(newWindow({url: 'https://google.com/?^'})).toStrictEqual({action: 'deny'});
+            expect(shell.openExternal).toBeCalledWith('https://google.com/?^');
         });
 
         it('should divert to allowProtocolDialog for custom protocols that are not mattermost or http', () => {

src/main/views/webContentEvents.ts

@@ -119,7 +119,9 @@ export class WebContentsEventManager {
             }
 
             // Check for valid URL
+            // Let the browser handle invalid URIs
             if (!urlUtils.isValidURI(details.url)) {
+                shell.openExternal(details.url);
                 return {action: 'deny'};
             }