Summary: | Web Inspector: Console Evaluation links in the Console should not show normally | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Timothy Hatcher <timothy> | ||||||||||||||
Component: | Web Inspector | Assignee: | Devin Rousso <hi> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | commit-queue, graouts, hi, inspector-bugzilla-changes, mattbaker, webkit-bug-importer | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | All | ||||||||||||||||
OS: | All | ||||||||||||||||
Attachments: |
|
Created attachment 304691 [details]
Patch
Created attachment 304692 [details]
[Image] After Patch is applied
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.
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).
(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! Created attachment 304699 [details]
Patch
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? 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. Created attachment 304717 [details]
Patch
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. Created attachment 304736 [details]
Patch
Comment on attachment 304736 [details] Patch Clearing flags on attachment: 304736 Committed r214093: <http://trac.webkit.org/changeset/214093> All reviewed patches have been landed. Closing bug. |
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.