Bug 106900 - Web Inspector: Show formatted content of JSON request
Summary: Web Inspector: Show formatted content of JSON request
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Sergey Ryazanov
URL: http://crbug.com/166874
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-15 06:29 PST by Sergey Ryazanov
Modified: 2013-01-16 08:56 PST (History)
9 users (show)

See Also:

Attachments
Patch (6.46 KB, patch)
2013-01-15 07:23 PST, Sergey Ryazanov 		no flags Details | Formatted Diff | Diff
Patch (5.00 KB, patch)
2013-01-15 22:42 PST, Sergey Ryazanov 		no flags Details | Formatted Diff | Diff
Patch (5.20 KB, patch)
2013-01-16 06:57 PST, Sergey Ryazanov 		no flags Details | Formatted Diff | Diff
JSON preview screenshot (84.66 KB, image/png)
2013-01-16 06:58 PST, Sergey Ryazanov 		no flags Details
Patch (5.76 KB, patch)
2013-01-16 07:54 PST, Sergey Ryazanov 		no flags Details | Formatted Diff | Diff
Updated screenshot (85.47 KB, image/png)
2013-01-16 07:56 PST, Sergey Ryazanov 		no flags Details
Patch (5.72 KB, patch)
2013-01-16 08:28 PST, Sergey Ryazanov 		no flags Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.