Summary: | Web Inspector: pass all parameters to WebInspector.addConsoleMessage as a single payload object | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yury Semikhatsky <yurys> | ||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Yury Semikhatsky <yurys> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | bweinstein, joepeck, keishi, pfeldman, pmuellr, rik, timothy, yurys | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Yury Semikhatsky
2010-07-15 02:26:56 PDT
Created attachment 61621 [details]
Patch
Comment on attachment 61621 [details]
Patch
I'd suggest that we remove ScriptArray as a whole instead. Ilya is almost there.
(In reply to comment #2) > (From update of attachment 61621 [details]) > I'd suggest that we remove ScriptArray as a whole instead. Ilya is almost there. At the present time we cannot serialize console parameters into InspectorObject's, because there is no way to get InspectorObject from JavaScript object. I think it should be addressed in a separate change . Comment on attachment 61621 [details]
Patch
WebCore/inspector/ConsoleMessage.cpp:
+ frontend->addConsoleMessage(jsonObj, m_frames, arguments, m_message);
m_frames, arguments are missing.
Created attachment 61630 [details]
Patch
(In reply to comment #4) > (From update of attachment 61621 [details]) > WebCore/inspector/ConsoleMessage.cpp: > + frontend->addConsoleMessage(jsonObj, m_frames, arguments, m_message); > m_frames, arguments are missing. Fixed. Comment on attachment 61630 [details]
Patch
WebCore/inspector/front-end/ConsoleView.js:278
+ msgCopy = new WebInspector.ConsoleMessage(msg.source, msg.type, msg.level, msg.line, msg.url, msg.groupLevel, count, msg.messageText, msg.parameters, msg.stackTrace);
I think you affect the repeatCount and repeatDelta properties.
WebCore/inspector/front-end/ConsoleView.js:658
+ this.parameters = parameters;
These should be private.
WebCore/inspector/front-end/ConsoleView.js:676
+ var stack = this.stackTrace.slice();
No need.
Created attachment 61646 [details]
Patch
(In reply to comment #7) > (From update of attachment 61630 [details]) > WebCore/inspector/front-end/ConsoleView.js:278 > + msgCopy = new WebInspector.ConsoleMessage(msg.source, msg.type, msg.level, msg.line, msg.url, msg.groupLevel, count, msg.messageText, msg.parameters, msg.stackTrace); > I think you affect the repeatCount and repeatDelta properties. > Fixed. > WebCore/inspector/front-end/ConsoleView.js:658 > + this.parameters = parameters; > These should be private. > Done. > > > WebCore/inspector/front-end/ConsoleView.js:676 > + var stack = this.stackTrace.slice(); > No need. Done. Comment on attachment 61646 [details]
Patch
I think it is too risky to rename everything. Could you name only new members as private?
Created attachment 61664 [details]
Patch
Comment on attachment 61664 [details]
Patch
WebCore/inspector/front-end/ConsoleView.js:685
+ this.formattedMessage = this._format(["%O", this._parameters[0]]);
I am a bit concerned with the potential switch from <non-undefined>[0] to <potentially-undefined>[0].
(In reply to comment #12) > (From update of attachment 61664 [details]) > WebCore/inspector/front-end/ConsoleView.js:685 > + this.formattedMessage = this._format(["%O", this._parameters[0]]); > I am a bit concerned with the potential switch from <non-undefined>[0] to <potentially-undefined>[0]. Added explicit check. Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/bindings/js/ScriptArray.cpp M WebCore/bindings/js/ScriptArray.h M WebCore/bindings/v8/ScriptArray.cpp M WebCore/bindings/v8/ScriptArray.h M WebCore/inspector/ConsoleMessage.cpp M WebCore/inspector/InspectorFrontend.cpp M WebCore/inspector/InspectorFrontend.h M WebCore/inspector/front-end/ConsoleView.js M WebCore/inspector/front-end/ElementsPanel.js M WebCore/inspector/front-end/InjectedScriptAccess.js M WebCore/inspector/front-end/Resource.js M WebCore/inspector/front-end/inspector.js Committed r63427 |