[webkit-reviews] review denied: [Bug 106900] Web Inspector: Show formatted content of JSON request : [Attachment 182765] Patch

[bugzilla-daemon at webkit.org]

Vsevolod Vlasov <vsevik at chromium.org> has denied Sergey Ryazanov
<serya at chromium.org>'s request for review:
Bug 106900: Web Inspector: Show formatted content of JSON request
https://bugs.webkit.org/show_bug.cgi?id=106900

Attachment 182765: Patch
https://bugs.webkit.org/attachment.cgi?id=182765&action=review

------- Additional Comments from Vsevolod Vlasov <vsevik at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=182765&action=review


> Source/WebCore/ChangeLog:1
> +2013-01-15  robert at webkit.org 
<robert at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>

Please fix the name

> Source/WebCore/ChangeLog:3
> +	   Web Inspector: [Chromium] Show formatted content of JSON request

It is not chromium-specific.

> Source/WebCore/inspector/front-end/RequestHeadersView.js:202
> +	       this._refreshParams(WebInspector.UIString("Form Data"),
formParameters, formData, this._formDataTreeElement, "URLEncoded");

Please don't use hard coded string constants for behavior logic. You could use
booleans instead or enums if you really need such constants.

> Source/WebCore/inspector/front-end/RequestHeadersView.js:206
> +	       if (json) {

No brackets for one-line conditionals/loops in webkit.

> Source/WebCore/inspector/front-end/RequestHeadersView.js:258
> +	       var object = WebInspector.RemoteObject.fromLocalObject(params);

You don't need header count for JSON, do you?
I would rather move this to a separate method, there is no much to reuse
anyway.