Bug 106900

Summary: Web Inspector: Show formatted content of JSON request
Product: WebKit Reporter: Sergey Ryazanov <serya>
Component: Web Inspector (Deprecated)Assignee: Sergey Ryazanov <serya>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, keishi, loislo, pfeldman, pmuellr, vsevik, web-inspector-bugs, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://crbug.com/166874
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
JSON preview screenshot
none
Patch
none
Updated screenshot
none
Patch none

Description Sergey Ryazanov 2013-01-15 06:29:20 PST
http://crbug.com/166874

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.11 (KHTML, like Gecko) Chrome/23.0.1271.97 Safari/537.11

Steps to reproduce the problem:
1. 
2. 
3. 

What is the expected behavior?

What went wrong?
In the network part, in details of a request, you have a tab for the request, a tab for the response, sometimes a tab for json preview.

Why not add a tab for JSON preview of parameters ? Like on Firefox.

The goal is not to copy Firefox but simply to get a useful tab

Did this work before? No 

Chrome version: 23.0.1271.97  Channel: stable
OS Version: Debian Squeeze
Comment 1 Sergey Ryazanov 2013-01-15 07:23:53 PST
Created attachment 182765 [details]
Patch
Comment 2 Vsevolod Vlasov 2013-01-15 10:42:11 PST
Comment on attachment 182765 [details]
Patch

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

> Source/WebCore/ChangeLog:1
> +2013-01-15  robert@webkit.org  <robert@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.
Comment 3 Sergey Ryazanov 2013-01-15 22:42:37 PST
Created attachment 182920 [details]
Patch
Comment 4 Vsevolod Vlasov 2013-01-16 04:14:35 PST
Comment on attachment 182920 [details]
Patch

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

Please attach a screenshot.

> Source/WebCore/inspector/front-end/RequestHeadersView.js:272
> +    _refreshRequestJSONPayload: function(parsedObject, sourceText, viewSource)

Please add compiler annotations to all funcitons and make sure this compiles without warnings.
Comment 5 Andrey Adaikin 2013-01-16 04:39:22 PST
Comment on attachment 182920 [details]
Patch

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

> Source/WebCore/inspector/front-end/RequestHeadersView.js:278
> +        listItem.appendChild(document.createTextNode(WebInspector.UIString("Request Payload")));

a new string? if so, add it to WebCore/English.lproj/localizedStrings.js
Comment 6 Sergey Ryazanov 2013-01-16 06:57:04 PST
Created attachment 182976 [details]
Patch
Comment 7 Sergey Ryazanov 2013-01-16 06:58:58 PST
Created attachment 182977 [details]
JSON preview screenshot
Comment 8 Sergey Ryazanov 2013-01-16 07:54:35 PST
Created attachment 182982 [details]
Patch
Comment 9 Sergey Ryazanov 2013-01-16 07:56:14 PST
Created attachment 182983 [details]
Updated screenshot

Removed bold font in JSON payload.
Comment 10 Vsevolod Vlasov 2013-01-16 08:25:49 PST
Comment on attachment 182982 [details]
Patch

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

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

Please remove these weird numbers/characters in the end of this line.
Comment 11 Sergey Ryazanov 2013-01-16 08:28:24 PST
Created attachment 182988 [details]
Patch
Comment 12 WebKit Review Bot 2013-01-16 08:56:14 PST
Comment on attachment 182988 [details]
Patch

Clearing flags on attachment: 182988

Committed r139886: <http://trac.webkit.org/changeset/139886>
Comment 13 WebKit Review Bot 2013-01-16 08:56:18 PST
All reviewed patches have been landed.  Closing bug.