Bug 22920

Adding CCs and confirming it

I definitely would like this feature too. This would be really useful to me in debugging javascript that makes POST requests. Right now I use firefox and it's really a pain to open up firefox just for this.

By data/variable/parameters do you mean the request body, that is, the thing you pass to send() when you do a POST or PUT?

I've started to look into this. Presumably, when "form data" is associated with a script, we'd show it in the resource panel, along side the request and response headers. I think it would be easiest to show it as a peer of the headers, with a new title of "Request Parameters", perhaps? "form data" would be either the HTTP payload for a post with an acceptable content-type for form data submission, or query string parameters specified on a GET. The form data would then be parsed into key/value pairs, and displayed similar to the way the headers are currently displayed. While at it, it seems like it would be nice to show the HTTP method used for the resource invocation. Thinking that could be prepended to the URL displayed in the Resource panel (the URL above the Request headers, Response Headers tree items).

(In reply to comment #4) > I've started to look into this. > > Presumably, when "form data" is associated with a script, we'd show it in the > resource panel, along side the request and response headers. I think it would > be easiest to show it as a peer of the headers, with a new title of "Request > Parameters", perhaps? > > "form data" would be either the HTTP payload for a post with an acceptable > content-type for form data submission, or query string parameters specified on > a GET. > > The form data would then be parsed into key/value pairs, and displayed similar > to the way the headers are currently displayed. > > While at it, it seems like it would be nice to show the HTTP method used for > the resource invocation. Thinking that could be prepended to the URL displayed > in the Resource panel (the URL above the Request headers, Response Headers tree > items). YEs I think that is the right approch. I am not sure about prepending the method to the URL, but I don't have a better solution.

Created attachment 35057 [details] work in progress showing HTTP verb prefixing URL in resource panel This works for me. Good use of real estate, instead of eating another line or something. Also matches the way the HTTP invocation actually works.

Maybe bold it? I think it will be fine then. Maybe only show it if the method isn't GET?

Created attachment 35065 [details] naive patch to InspectorResource I've attached a naive patch in support of being able to display the form data and method for a request. It seems that my years of experience in C are no help in trying to make sense of the C++ universe. What I've attached is the simplest possible way of making the copies of the data I need. And it works! Other examples of String usage didn't show any of the sort of templating I've seen in other code, nor the use of any explicit destruction. I'm assuming that String types are basically considered values, never shared, and so more or less get free memory management? If not, any other examples I can look at?

If it's possible to have the output directly in the console (i.e. exactly like firebug), I think this would be a much better solution than just displaying it in the resources tab. The issues with displaying xhr requests in the resources tab is firstly there are so many resources you have to dig through them. You are not able to have console.log('example') and see where the xhr occurs in relation to the outputted text. Firebug's xhr debugging is close to perfect, so rather than reinventing the wheel, it would better to use that as a base to copy and then improve on it. I've written about this more in depth on https://bugs.webkit.org/show_bug.cgi?id=28119

Note you can now filter the reosurces panel to just show XHRs now, so there is less hunting for them.

As Tim mentions, nightlies now include resource filtering, so you can select XHR and just see those. There is also a little toggle in the bottom line of the window where you can use smaller rows. But in addition, XHRs are logged in the console window, though I believe just the completions. I like the list you have in Bug 28119. But a lot of that isn't really relevant to this particular bug. I >do< think it makes sense to render the HTTP request payload as appropriate, but will probably just do the form data for now, plain text dump if the content type doesn't indicate it's form data (or whatever). Request output AND input should be formatted for specific type handlers (JSON, HTML, XML), but I'll leave that for another bug.

Created attachment 35130 [details] work in progress showing query string and form data current progress. I've redacted bits of the UI, as this was from a live session w/gmail - which turns out to be a great test case for this stuff. You can see new "Query String Parameters" and "Form Data" elements after the "Request Headers" but before "Response Headers". They only appear if applicable, most of the time they won't be there at all. Note this was a case of a POST with a URL that had query string parms in the URL as well as form data, so both sections appear. The key/vals are laid out with the exact same styling as the headers and values; this is somewhat problematic - there are already sizing issues with the header keys in some cases, but that will have to be fixed later, for all of these sections. When collapsed, the number of pairs will be displayed in the collapsed title, just as with the Request and Response Headers. The values are displayed raw. For example, look at the bottom of the image, with the req3_json key. URL encoding goodness. I feel like it's important to display the raw value, but obviously useful to show the decoded one, so the decoded version is available as a tooltip, as shown in this example (the yellow div at the bottom of the image). The last thing to do is figure out what to do with an HTTP request payload which is NOT form data. I'm currently thinking of just doing the simplest possible thing and displaying it as another (optionally visible) section called "Request Data" or something. With one element called data, whose value is the actual payload. Ideally, the payload would actually be displayed as a peer of the response payload - ie, the div underneath all the headers (where, instance, an image might be displayed if the resource was an image). Seems like perhaps this is something that can be properly done in another bug, which could do some other payload enhancements like render JSON in a friendly way.

Rather than a tooltip, for req3_json, would it be possible to actually have this displayed and formatted. Do you know which form elements contain json and which don't? For example: req3_json: { something: 'bla', yep: [ 1, 2, 3 ], more: { etc: 'test' } } Having copyable and formatable request and response headers would be really handy. Quite often the json response headers are huge so response formatting would need some sort of contract and expand nesting (like firebug).

Created attachment 38547 [details] example showing query string parameters along with form data in the same request clean version of a previous attachment. Note there are cases, as with gmail, where the request contains both query string parameters and form data in the request payload; this example shows that both are displayed

Created attachment 38548 [details] example showing a binary http payload As with some other cases in inspector, we do a best effort to show the content, even if it's binary. Data sent with an HTTP request will be displayed in this fashion. It's the same format as other key/value pairs in this pane, only there is no key, just a value. Note that file upload data is not available using the APIs I'm using to get the request data, and so that should never appear here. I think that actually works out for the best.

(In reply to comment #14) > Created an attachment (id=38547) [details] > example showing query string parameters along with form data in the same > request Would it be appropriate to decode the query string parameters in the collapsible area? So that "a%20b" in the URI shows up as "a b" in the collapsible area.

Created attachment 38550 [details] proposed patch Ready for review. Two notes: - contains changes to localizedStrings.js, so diff won't help - two strings added at end - contains a change to some C code, using two new String type members in InspectorResource.h/.cpp; I didn't see any special allocation/deallocation magic for other Strings used here, so didn't do anything special myself.

(In reply to comment #16) > Would it be appropriate to decode the query string parameters in the > collapsible area? So that "a%20b" in the URI shows up as "a b" in the > collapsible area. Seemed like the right thing to do was to show the strings encoded. A tooltip is available for encoded strings to show the decoded version. I could do the opposite.

(In reply to comment #18) > (In reply to comment #16) > > Would it be appropriate to decode the query string parameters in the > > collapsible area? So that "a%20b" in the URI shows up as "a b" in the > > collapsible area. > > Seemed like the right thing to do was to show the strings encoded. A tooltip > is available for encoded strings to show the decoded version. Yah, I looked over the patch and saw the decodeURIComponent text was in the tooltip. The problem I have is that the text in the tooltip it isn't easily copyable and I think more often then not I would want to copy/paste the decoded version. Maybe there could be a way to toggle between the encoded/decoded versions?

(In reply to comment #19) > Yah, I looked over the patch and saw the decodeURIComponent text was in the > tooltip. The problem I have is that the text in the tooltip it isn't easily > copyable and I think more often then not I would want to copy/paste the decoded > version. Maybe there could be a way to toggle between the encoded/decoded > versions? A toggle would work for me, but I'm not sure where the toggle button would go. There are some toggly buttons at the bottom of the window, but I've not really like those to begin with, and are too far from the UI in question. Suggestions? One simple suggestion would be to make the entire form data / query string parameters sections clickable, and when you click, it toggles between encoded and decoded. non-intuitive, but good use of screen real estate :-)

I think we should show them decoded. Maybe the raw data form should be a mini-hex view (with an asci side representation)? Somthing like: http://www.pcmesh.com/images/index.dat_view_hex.gif Review coming next.

Comment on attachment 38550 [details] proposed patch > + this.requestMethod = requestMethod || ""; > + this.requestFormData = requestFormData || ""; Is this needed, can they be null/undefined? > + var parmString = url.split("?",2)[1]; Need a space after the comma (a few places like this in the patch.) > + if (testKey.toLowerCase() == lowerKey) { Triple equals would be best. No need for the braces. > + return undefined; Just remove this and let the default undefined return happen. > + title += "<div style='white-space:pre-wrap' class='header-value'>" + formData.escapeHTML() + "</div>"; You should add a style class to the inspector.css and not have this inline style. Use double quotes for the HTML too. > + for (var i = 0; i < parms.length; i++) { Use ++i. > + parm = parm.split("=",2); Space after the comma. > + if (parm.length == 1) parm.push(""); Should be on two lines. > + var tooltip; > + if (val.indexOf("%") >= 0) { > + tooltip = decodeURIComponent(val).replace("+"," "); > + } No need for the braces. I think we should show decoded by default. I like Joe's idea of a way to toggle the representations inline so they can be copied. I don't think a tooltip is useful, but maybe a tooltip showing the size in bytes/KB/MB is. > + if (tooltip) parmTreeElement.tooltip = tooltip; Should be on two lines, if we need it.

(In reply to comment #21) > I think we should show them decoded. > > Maybe the raw data form should be a mini-hex view (with an asci side > representation)? Somthing like: > > http://www.pcmesh.com/images/index.dat_view_hex.gif The idea of a hex view is interesting, but would probably be appropriate to show it for the response data as well; optionally, of course, somehow. I suggest we open a new bug on that. At this point, if we don't need to integrate that into this bug (lots more work), and if we want to show the values decoded, then I think there are two options to showing the encoded versions: - in a tooltip, per form data item / query string parameter - make the entire form data / query string parameters section clickable, and when clicked, toggle between encoded/decoded. A tooltip over the area could provide the hint that this gesture is possible. I kind of like the second. Perhaps this would be a way to toggle a hex view on the response as well, though I think there are more real estate options for the response data than the request data, and thus perhaps a button would be more appropriate for the response data.

Hex view can be done later for sure. Toggle on click is what I would like. The tooltip showing the data size would be nice too (maybe jiust for raw data.)

(In reply to comment #24) > Hex view can be done later for sure. I've opened bug 28775 for this. > Toggle on click is what I would like. The tooltip showing the data size would > be nice too (maybe jiust for raw data.) The size of the request data, in it's entirety is >generally< available via the Content-Length header. This is not always the case, for both the request and response data; the server can close a connection after writing signalling the end of the data rather than send a Content-Length for the response , and then there is the Transfer-Encoding: Chunked business for both requests and responses (see: http://en.wikipedia.org/wiki/Chunked_transfer_encoding ). In those kind of cases, it may well be useful to know the size of the HTTP message payload. I suspect this will end up getting a little more complicated; for a large resource, if we provide a "size" capability like this, the user would probably like to see the amount of data received so far. In theory, same for the request. Suggest we open another bug on this as well. If so, we have a tooltip available! Should I provide a hint to the user about the toggle on click behavior of encoded/decoded. I can't imagine anyone's going to figure that one out on their own.

A hint in the tooltip is fine.

Created attachment 38829 [details] proposed patch - 2009/08/31 All of the issues from the previous review have been attended to, except for the first: in WebCore/inspector/front-end/Resource.js: > + this.requestMethod = requestMethod || ""; > + this.requestFormData = requestFormData || ""; > Is this needed, can they be null/undefined? It's not clear to me that they will always be string values, they might be null at some point along the way. Code further down in the processing expects these to be string values, so seems best to initialize them to an empty string here. Empty string turns out to be a good default value for both of these. Regarding the encoded/decoded versions of the form data; the initial display is now decoded. A hover help over a form data item displays: "Double-Click to toggle between URL encoded and decoded formats". Double-clicking results in toggling between encoded and decoded. Initially I was thinking of doing this with a single click, but realized (without trying it), that this would likely interfere with trying to select the items if you wanted to copy them to the clipboard.

Comment on attachment 38829 [details] proposed patch - 2009/08/31 Clearing flags on attachment: 38829 Committed r47903: <http://trac.webkit.org/changeset/47903>

All reviewed patches have been landed. Closing bug.