Bug 227549 – [results.webkit.org] linkify urls in commit messages

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Kevin Neal]

Commit messages usually contain URLs pointing to the bug; however, the URLs do not have hyperlinks to the bug.

[Radar WebKit Bug Importer]

<rdar://problem/79988762>

[Kevin Neal]

Created attachment 432638 [details]
Patch

[Kevin Neal]

Created attachment 433222 [details]
Patch

[Zhifei Fang]

Comment on attachment 433222 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433222&action=review

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/commit.js:122
> +                            let linkify = (function(commitMessage) {

better to make this a top level function, not a IIFE. If you put the code here, actually there is no need for have a IIFE. Originally line 117 we used the IIFE for making javascript string template can support normal js statements not just expressions.

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/commit.js:123
> +                                let reg = /\b(https?|rdar):\/{2}[^\s<>&]+/gmi;

May also need to consider http, and I am not sure if we have some links without leading protocol, may worth to find if they are existed.

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/commit.js:125
> +                                let newMessageText = messageText.replace(reg, `<a href="$&">$&</a>`);

It will be better to open a new tab, add target="_blank"

[Jonathan Bedard]

Comment on attachment 433222 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433222&action=review

>> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/commit.js:123
>> +                                let reg = /\b(https?|rdar):\/{2}[^\s<>&]+/gmi;
> 
> May also need to consider http, and I am not sure if we have some links without leading protocol, may worth to find if they are existed.

Existing code does consider http, I think, the regex is `https?` which should match http and https

[Kevin Neal]

Created attachment 433366 [details]
Patch

[Jonathan Bedard]

Comment on attachment 433366 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433366&action=review

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/commit.js:121
>                                  return `<br><div>${escapeHTML(cell.commit.message.split('\n')[0])}</div>`;

We should lankily this text as well, in case someone has a link in the first line

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/commit.js:122
> +                            return linkify(escapeHTML(cell.commit.message));

We still want to keep the `<br><div></div>` surrounding this text

[Kevin Neal]

Created attachment 433409 [details]
Patch

[Kevin Neal]

Created attachment 433410 [details]
fast-cq Patch

[EWS]

Committed r279876 (239629@main): <https://commits.webkit.org/239629@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433410 [details].