From c0a660467665009281f9066c9f426aeb9cb9f87b Mon Sep 17 00:00:00 2001 From: Ronan Jouchet Date: Thu, 4 Mar 2021 10:00:53 -0500 Subject: [PATCH] Fix considering "same domain-ish" URLs as internal (PR #1126) In 6b266b78150, as I got rid of deprecated dep `wurl`, I wrote: > This one may be problematic, as it used to do TLD stuff: > https://github.com/websanova/node-url/blob/7982a613bc/wurl.js#L4 > > So, the new WHATWG-URL-based implementation will consider > `asana.com` to be "external" to `app.asana.com`, contrarily to before. > Given the nature of Nativefier, I think it's actually what to expect, > that in this case you're "out of the app", and in e.g. asana's landing > page, which you'd expect to see in your browser. Turns out it's even more problematic: @TheCleric notices in https://github.com/nativefier/nativefier/pull/1124#issuecomment-790279403 that this breaks app `https://evernote.com` doing its login in `www.evernote.com` The present change fixes this, by behaving mostly similarly to before, but without re-introducing `wurl` or another dep needing a TLD/SLD list. --- app/src/helpers/helpers.test.ts | 26 ++++++++++++++++++++++++++ app/src/helpers/helpers.ts | 14 +++++++++++--- docs/api.md | 8 +++++++- 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/app/src/helpers/helpers.test.ts b/app/src/helpers/helpers.test.ts index c7792d9..3b86ec7 100644 --- a/app/src/helpers/helpers.test.ts +++ b/app/src/helpers/helpers.test.ts @@ -1,6 +1,10 @@ import { linkIsInternal, getCounterValue } from './helpers'; const internalUrl = 'https://medium.com/'; +const internalUrlWww = 'https://www.medium.com/'; +const sameBaseDomainUrl = 'https://app.medium.com/'; +const internalUrlCoUk = 'https://medium.co.uk/'; +const sameBaseDomainUrlCoUk = 'https://app.medium.co.uk/'; const internalUrlSubPath = 'topic/technology'; const externalUrl = 'https://www.wikipedia.org/wiki/Electron'; const wildcardRegex = /.*/; @@ -27,6 +31,28 @@ test('all urls should be internal with wildcard regex', () => { expect(linkIsInternal(internalUrl, externalUrl, wildcardRegex)).toEqual(true); }); +test('a "www." of a domain should be considered internal', () => { + expect(linkIsInternal(internalUrl, internalUrlWww, undefined)).toEqual(true); +}); + +test('urls on the same "base domain" should be considered internal', () => { + expect(linkIsInternal(internalUrl, sameBaseDomainUrl, undefined)).toEqual( + true, + ); +}); + +test('urls on the same "base domain" should be considered internal, even with a www', () => { + expect(linkIsInternal(internalUrlWww, sameBaseDomainUrl, undefined)).toEqual( + true, + ); +}); + +test('urls on the same "base domain" should be considered internal, long SLD', () => { + expect( + linkIsInternal(internalUrlCoUk, sameBaseDomainUrlCoUk, undefined), + ).toEqual(true); +}); + const smallCounterTitle = 'Inbox (11) - nobody@example.com - Gmail'; const largeCounterTitle = 'Inbox (8,756) - nobody@example.com - Gmail'; const noCounterTitle = 'Inbox - nobody@example.com - Gmail'; diff --git a/app/src/helpers/helpers.ts b/app/src/helpers/helpers.ts index 3eaa233..4658f99 100644 --- a/app/src/helpers/helpers.ts +++ b/app/src/helpers/helpers.ts @@ -33,9 +33,17 @@ export function linkIsInternal( } try { - const currentDomain = new URL(currentUrl).hostname; - const newDomain = new URL(newUrl).hostname; - return currentDomain === newDomain; + // Consider as "same domain-ish", without TLD/SLD list: + // 1. app.foo.com and foo.com + // 2. www.foo.com and foo.com + // 3. www.foo.com and app.foo.com + const currentDomain = new URL(currentUrl).hostname.replace(/^www\./, ''); + const newDomain = new URL(newUrl).hostname.replace(/^www./, ''); + const [longerDomain, shorterDomain] = + currentDomain.length > newDomain.length + ? [currentDomain, newDomain] + : [newDomain, currentDomain]; + return longerDomain.endsWith(shorterDomain); } catch (err) { console.warn( 'Failed to parse domains as determining if link is internal. From:', diff --git a/docs/api.md b/docs/api.md index 18a7144..a1449e0 100644 --- a/docs/api.md +++ b/docs/api.md @@ -400,7 +400,13 @@ Forces the packaged app to ignore web security errors, such as [Mixed Content](h --internal-urls ``` -Regular expression of URLs to consider "internal"; all other URLs will be opened in an external browser. Defaults to URLs on same second-level domain as app. +Regular expression of URLs to consider "internal" while following a hyperlink. +Internal URLs will open in Nativefier, other URLs will open in your preferred browser. + +Defaults to view as "internal" two URLs that share the same base domain, +once stripped of `www.`. For example, by default, +- URLs from/to `foo.com`, `app.foo.com`, `www.foo.com` are considered internal. +- URLs from/to `abc.com` and `xyz.com` are considered external. Example: