Web Inspector: Details of InjectedScript function call shall not be linkified in Timeline panel.

[pdeng6]

Currently, details of InjectedScript function call is linkified, but actually, no valid resource provided for it, see attachment 1.
when user click the link, "This page is not available" will pop up, see attachement 2.
I suggest remove the link and line number in this case :)

Thanks
Pan

[pdeng6]

Created attachment 191925 [details]
Link of InjectedScript function call

[pdeng6]

Created attachment 191926 [details]
error page that the invalid link lead to

[pdeng6]

Created attachment 191927 [details]
Patch

[Pavel Feldman]

Comment on attachment 191927 [details]
Patch

I think this edge case is to minor to special case / plumb callFromInjectedScript with it.

[pdeng6]

Created attachment 192652 [details]
Patch

[pdeng6]

(In reply to comment #4)
> (From update of attachment 191927 [details])
> I think this edge case is to minor to special case / plumb callFromInjectedScript with it.

yes, to avoid plumb things, I suggest only make funcitonCall link for valid script url.
It also solve the invalid link "undefined:1" besides "InjectedScript:1" :)

Thanks!
Pan

[Alexander Pavlov (apavlov)]

Comment on attachment 192652 [details]
Patch

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

> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:1127
> +     * @return {true|false}

{boolean}

> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:1133
> +                var parsedURL = new WebInspector.ParsedURL(this.scriptName);

You can make use of String.prototype.asParsedURL found in ParsedURL.js and rewrite this as:

this._validScriptLocation = !!this.scriptName.asParsedURL();

> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:1135
> +            } else {

Please remove curly braces around a single-line block.

> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:1140
> +        return  this._validScriptLocation;

Extraneous whitespace after "return"

[pdeng6]

Created attachment 192854 [details]
Patch

[pdeng6]

(In reply to comment #7)
> (From update of attachment 192652 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=192652&action=review
> 
> > Source/WebCore/inspector/front-end/TimelinePresentationModel.js:1127
> > +     * @return {true|false}
> 
> {boolean}
> 
Done

> > Source/WebCore/inspector/front-end/TimelinePresentationModel.js:1133
> > +                var parsedURL = new WebInspector.ParsedURL(this.scriptName);
> 
> You can make use of String.prototype.asParsedURL found in ParsedURL.js and rewrite this as:
> 
> this._validScriptLocation = !!this.scriptName.asParsedURL();
> 
Done, thanks a lot for guidance!

> > Source/WebCore/inspector/front-end/TimelinePresentationModel.js:1135
> > +            } else {
> 
> Please remove curly braces around a single-line block.
> 
Done

> > Source/WebCore/inspector/front-end/TimelinePresentationModel.js:1140
> > +        return  this._validScriptLocation;
> 
> Extraneous whitespace after "return"
Done,

Thanks!
Pan

[pdeng6]

Comment on attachment 192854 [details]
Patch

@apavlov, could you please help review this patch?

thanks
Pan

[Vsevolod Vlasov]

Comment on attachment 192854 [details]
Patch

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

> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:1132
> +            this._validScriptLocation = this.scriptName ? !!this.scriptName.asParsedURL() : false;

I think it's possible that a script with the name that could not be correctly parsed can still be correctly linkified (think sourceURL).
Such false negative would be a much worse issue than current false positive behavior.

[Brian Burg]

Closing as invalid, as this bug pertains to the old inspector UI and/or its tests.
Please file a new bug (https://www.webkit.org/new-inspector-bug) if the bug/feature/issue is still relevant to WebKit trunk.