[DevTools] Do not linkify relative urls.

[DevTools] Do not linkify relative urls.

They happen to point to some DevTools frontend file, which is misleading at best.

BUG=591123
Committed: https://crrev.com/f82e78397ae82e65234d5449297aa3a6661055c4
Cr-Commit-Position: refs/heads/master@{#382087}

[dgozman, 2016-03-16 00:21:58]

dgozman@chromium.org changed reviewers:
+ caseq@chromium.org

[dgozman, 2016-03-16 00:21:59]

Take a look please.

[caseq, 2016-03-16 00:50:33]

I think in the particular scenario of the referred bug we could actually resolve
the URL, couldn't we?

https://codereview.chromium.org/1805763002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/common/ParsedURL.js (right):

https://codereview.chromium.org/1805763002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/common/ParsedURL.js:314: var
hasPort = /^[^:^\/]*:/.test(url);
I think this is wrong both on part of URL grammar...

https://codereview.chromium.org/1805763002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/common/ParsedURL.js:317: var
hasDomain = /^[^.^\/]*\.[^\/]*\//.test(url);
... and regexp syntax.

[dgozman, 2016-03-16 01:41:45]

PTAL

[caseq, 2016-03-16 01:44:28]

https://codereview.chromium.org/1805763002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/common/ParsedURL.js (right):

https://codereview.chromium.org/1805763002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/common/ParsedURL.js:311: return
!(/^([A-Za-z][A-Za-z0-9+.-]*):\/\//.test(url));
so javascript:alert('hello-world') is relative according to this? ditto for
data: etc.

[dgozman, 2016-03-16 01:55:18]

https://codereview.chromium.org/1805763002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/common/ParsedURL.js (right):

https://codereview.chromium.org/1805763002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/common/ParsedURL.js:311: return
!(/^([A-Za-z][A-Za-z0-9+.-]*):\/\//.test(url));
On 2016/03/16 01:44:28, caseq wrote:
> so javascript:alert('hello-world') is relative according to this? ditto for
> data: etc.

Nope, we test for //.

[dgozman, 2016-03-17 00:21:33]

PTAL

[caseq, 2016-03-17 00:28:39]

lgtm

https://codereview.chromium.org/1805763002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/common/ParsedURL.js (right):

https://codereview.chromium.org/1805763002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/common/ParsedURL.js:311: return
!(/^([A-Za-z][A-Za-z0-9+.-]*):/.test(url));
nit: drop parenthesis before ':'

https://codereview.chromium.org/1805763002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/components/Linkifier.js
(right):

https://codereview.chromium.org/1805763002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/Linkifier.js:515:
classes = (classes ? classes + " " : "");
please move this down to conditional where we modify classes now, otherwise it
doesn't make sense. Better yet, use createElementWithClass() below and append
appropriate "webkit-html-foo-link" to a.classList if setting a.href.

[dgozman, 2016-03-18 19:06:47]

https://codereview.chromium.org/1805763002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/common/ParsedURL.js (right):

https://codereview.chromium.org/1805763002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/common/ParsedURL.js:311: return
!(/^([A-Za-z][A-Za-z0-9+.-]*):/.test(url));
On 2016/03/17 00:28:39, caseq wrote:
> nit: drop parenthesis before ':'

Done.

https://codereview.chromium.org/1805763002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/components/Linkifier.js
(right):

https://codereview.chromium.org/1805763002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/Linkifier.js:515:
classes = (classes ? classes + " " : "");
On 2016/03/17 00:28:39, caseq wrote:
> please move this down to conditional where we modify classes now, otherwise it
> doesn't make sense. Better yet, use createElementWithClass() below and append
> appropriate "webkit-html-foo-link" to a.classList if setting a.href.

Done.

[dgozman, 2016-03-18 19:06:52]

The CQ bit was checked by dgozman@chromium.org

[dgozman, 2016-03-18 19:06:52]

The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org
Link to the patchset: https://codereview.chromium.org/1805763002/#ps80001
(title: "fixed review comments")

[commit-bot: I haz the power, 2016-03-18 19:07:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1805763002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1805763002/80001

[commit-bot: I haz the power, 2016-03-18 21:17:22]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-03-18 21:19:22]

Description was changed from

==========
[DevTools] Do not linkify relative urls.

They happen to point to some DevTools frontend file, which is misleading at
best.

BUG=591123
==========

to

==========
[DevTools] Do not linkify relative urls.

They happen to point to some DevTools frontend file, which is misleading at
best.

BUG=591123

Committed: https://crrev.com/f82e78397ae82e65234d5449297aa3a6661055c4
Cr-Commit-Position: refs/heads/master@{#382087}
==========

[commit-bot: I haz the power, 2016-03-18 21:19:23]

Patchset 5 (id:??) landed as
https://crrev.com/f82e78397ae82e65234d5449297aa3a6661055c4
Cr-Commit-Position: refs/heads/master@{#382087}