FIX(BB-564) : Converted revision notes string url to clickable links

[akashgp09]

Problem

This PR Fixes: BB-564

Solution

Used regex to convert any string url that has a prefix http|https|ftp|ftps to a clickable link and then rendered the HTML string as real HTML.

Before

bandicam.2021-03-14.20-11-46-501.mp4

After

bandicam.2021-03-14.20-11-12-974.mp4

[coveralls]

Coverage decreased (-0.09%) to 60.849% when pulling b769b76 on akashgp09:make-link into 1c810bd on bookbrainz:master.

[akashgp09]

I have added the DOMPurify package, but using DOMPurify.sanitize throws TypeError: dompurify_1.default.sanitize is not a function. I don't know what's causing this, so i have opened this issue in DOMPurify, let's see if we can get some insights on this.
Related Issue: #353

[MonkeyDo]

I have added the DOMPurify package, but using DOMPurify.sanitize throws TypeError: dompurify_1.default.sanitize is not a function. I don't know what's causing this, so i have opened this issue in DOMPurify, let's see if we can get some insights on this.
Related Issue: #353

It is probably because of missing typescript types.
You'll need to install the @types/dompurify package: https://www.npmjs.com/package/@types/dompurify

Let me know if that fixes it.

[akashgp09]

Let me know if that fixes it.

I installed this package but still getting the error.

I tried debugging it using console.log

import DOMPurify from "dompurify";
console.log(DOMPurify);  // [Function: DOMPurify] { version: '2.2.6', removed: [], isSupported: false }
console.log(DOMPurify.sanitize); // undefined

DOMPurify.sanitize is undefined

[MonkeyDo]

Let me know if that fixes it.

I installed this package but still getting the error.

I tried debugging it using console.log

import DOMPurify from "dompurify";
console.log(DOMPurify);  // [Function: DOMPurify] { version: '2.2.6', removed: [], isSupported: false }
console.log(DOMPurify.sanitize); // undefined

DOMPurify.sanitize is undefined

Hm. In another project I imported it like this: import { sanitize } from "dompurify"; and then used simple with sanitize(…) . Maybe try that?

[akashgp09]

Hm. In another project I imported it like this: import { sanitize } from "dompurify"; and then used simple with sanitize(…) . Maybe try that?

Tried this but same results. Can you check it on this branch on your system. Or what about using sanitize-html package alternative to dompurify.

[MonkeyDo]

Hm. In another project I imported it like this: import { sanitize } from "dompurify"; and then used simple with sanitize(…) . Maybe try that?

Tried this but same results. Can you check it on this branch on your system. Or what about using sanitize-html package alternative to dompurify.

I ran npm install --save dompurify(the PR is currently missing that change in package.json and package-locks.json), then in revision-table.js:
import {sanitize} from 'dompurify';
and later on:

<span
   // eslint-disable-next-line react/no-danger
   dangerouslySetInnerHTML={{
   	__html: sanitize(note.content.replace(
   		// eslint-disable-next-line max-len
   		/((http|https|ftp|ftps)\:\/\/[a-zA-Z0-9\-\.]+\.[a-zA-Z]{2,3}(\/\S*)?)/g,
   		'<a target="_blank" href="$1">$1</a>'
   	))
   }}
/>

Everything works as expected, no warnings of any kind.

[akashgp09]

Regarding the url extension part, .[a-zA-Z]{2,3} is too restrictive. For example http://www.test.info/ will return http://www.test.inf with a missing final 'o'.

hm, our current regex needs to be modified 👀

I also wonder if we should also convert www.test.info without the protocol at the start. What do you think?

Yes indeed, we should also convert URLs without the protocol at the beginning.
will make the changes soon, thanks for the review:)

[MonkeyDo]

So I should probably have realized earlier, but we're going to want to make this code more reusable by extracting it into a utility function (probably in src/client/helpers/utils.ts).

For example on any page with revision notes (e.g. revision page: https://test.bookbrainz.org/revision/29833), as well as annotations (https://github.com/bookbrainz/bookbrainz-site/blob/master/src/client/components/pages/entities/annotation.js#L49).
In the future there will be more uses for it but let's start with those.

The utility should return the whole <span> element with the dangerouslySetInnerHTML.

Separately to that, something else we missed is that a short url (www.xyz…) will be parsed correctly, but the url is interpreted relative to the current page.
Here's an example where on the page http://localhost:9099/editor/1245/revisions, the url www.bookbrainz.org will link to http://localhost:9099/editor/1245/revisions/www.bookbrainz.org:

If I manually turn the URL from www.bookbrainz.org to https://www.bookbrainz.org (or even the less descriptive //www.bookbrainz.org), it points to the right page.
Full URLs are working fine however 👍

[akashgp09]

So I should probably have realized earlier, but we're going to want to make this code more reusable by extracting it into a utility function (probably in src/client/helpers/utils.ts).
The utility should return the whole <span> element with the dangerouslySetInnerHTML.

Then I guess we will need to rename our utils.ts file to utils.tsx so that we can return jsx <span dangerouslySetInnerHTML={{...}} />

@MonkeyDo what are your thoughts on this ?

[MonkeyDo]

Then I guess we will need to rename our utils.ts file to utils.tsx so that we can return jsx <span dangerouslySetInnerHTML={{...}} />

Yes, that's fine. There are no ill effects with doing that.

[akashgp09]

If I manually turn the URL from www.bookbrainz.org to https://www.bookbrainz.org (or even the less descriptive //www.bookbrainz.org), it points to the right page.

Added addHttpRegex which will prepend https:// protocol before www. if there isn't any.

[MonkeyDo]

Looking good !
Last bits before merging this useful tool.