Bug 157483 – Web Inspector: Console Evaluation links in the Console should not show normally

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Timothy Hatcher]

Created attachment 278426 [details]
Screenshot

We should not show the console.log (etc) location for direct calls when a result has not been returned yet. This would let the location show up for async logs later still.

[Radar WebKit Bug Importer]

<rdar://problem/26177460>

[Devin Rousso]

Created attachment 304691 [details]
Patch

[Devin Rousso]

Created attachment 304692 [details]
[Image] After Patch is applied

[Timothy Hatcher]

I'd argue we drop the whole right hand thing, not jus the location. I think it is only useful when it is a function call defined in the console that gets called out of line.

Like:

> setTimeout(100, function() { console.log("Here") })
< undefined
L: "Here"    [S] Console Evaluation — ConsoleEvaluation:1:13


Nothing else would have the [S] Console Evaluation line.

[Matt Baker]

Comment on attachment 304691 [details]
Patch

Subtitle should only be created if we're linking to a source location. Also the cursor when hovering Console Evaluation should be the default, instead of a pointer (hand).

[Matt Baker]

(In reply to comment #4)
> I'd argue we drop the whole right hand thing, not jus the location. I think
> it is only useful when it is a function call defined in the console that
> gets called out of line.
> 
> Like:
> 
> > setTimeout(100, function() { console.log("Here") })
> < undefined
> L: "Here"    [S] Console Evaluation — ConsoleEvaluation:1:13
> 
> 
> Nothing else would have the [S] Console Evaluation line.

Let's drop it!

[Devin Rousso]

Created attachment 304699 [details]
Patch

[Matt Baker]

Comment on attachment 304699 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Models/CallFrame.js:266
> +        return new WebInspector.CallFrame(target, id, sourceCodeLocation, functionName, thisObject, scopeChain, nativeCode, programCode, isTailDeleted, isConsoleEvaluation);

I think this change is unnecessary. Why not have CallFrame determine `isConsoleEvaluation` in its constructor?

[Matt Baker]

Comment on attachment 304699 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Models/CallFrame.js:259
>              programCode = true;

This can also move into CallFrame's constructor, guaranteeing that `isConsoleEvaluation`, `functionName`, and `programCode` are internally consistent.

[Devin Rousso]

Created attachment 304717 [details]
Patch

[Matt Baker]

Comment on attachment 304717 [details]
Patch

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

Looks good, r=me with a nit.

> Source/WebInspectorUI/UserInterface/Models/CallFrame.js:52
> +        this._isConsoleEvaluation = isConsoleEvaluation || false;

Don't need to OR with false, `isConsoleEvaluation` is a boolean by definition.

[Devin Rousso]

Created attachment 304736 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 304736 [details]
Patch

Clearing flags on attachment: 304736

Committed r214093: <http://trac.webkit.org/changeset/214093>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.