Closed Bug 859133 Opened 10 years ago Closed 8 years ago

Add a plain request/response headers view


(DevTools :: Netmonitor, defect, P2)



(relnote-firefox 35+)

Firefox 35
Tracking Status
relnote-firefox --- 35+


(Reporter: vporof, Assigned: rlustin, Mentored)



(Whiteboard: [lang=js])


(1 file, 4 obsolete files)

The headers are currently shown in a VariablesView instance. This is great, but if someone wants to batch-copy all the headers, they're goona' have a bad time. Something like "view source" would be nice (this can even just simply open a new tab with the headers listed as plain text).
Summary: Add a plain request/response headers view → [netmonitor] Add a plain request/response headers view
See also bug 859138.
Moving into Developer Tools: Netmonitor component. Filter on NETMONITORAMA.
Component: Developer Tools → Developer Tools: Netmonitor
Summary: [netmonitor] Add a plain request/response headers view → Add a plain request/response headers view
Blocks: 851538
Priority: -- → P2
Duplicate of this bug: 883842
Duplicate of this bug: 922765
Blocks: 978144
Duplicate of this bug: 989335
I'm offering to mentor the netmonitor actor changes needed to make available the raw request/response headers over the protocol. For UI we can either ask Victor for mentoring or we can do it in a separate bug. This also depends on the contributor's experience.

To the potential contributor: please note this is not a good first bug - fixing this bug requires some experience with our codebase and work flow. Thanks!
Whiteboard: [mentor=msucan][lang=js]
Mentor: mihai.sucan
Whiteboard: [mentor=msucan][lang=js] → [lang=js]
Hi Victor! I've tried something with this feature.

It adds a "Show raw headers" button in headers tab panel.
This button shows 2 multi-lines textboxes with request and response headers, ready to consult or copy.

What do you think ?
Flags: needinfo?(vporof)
Comment on attachment 8489165 [details] [diff] [review]
Add a plain request/response headers view.

Review of attachment 8489165 [details] [diff] [review]:

I very much like this idea! Please fix the issues below and add a test, so we can land it ;)

::: browser/devtools/netmonitor/netmonitor-view.js
@@ +651,5 @@
> +
> +    let selectedRequestHeaders = selected.requestHeaders.headers;
> +    $("#raw-request-headers-textarea").value = writeHeaderText(selectedRequestHeaders);
> +    let selectedResponseHeaders = selected.responseHeaders.headers;
> +    $("#raw-resposne-headers-textarea").value = writeHeaderText(selectedResponseHeaders);

Typo: raw-resposne.

::: browser/devtools/netmonitor/netmonitor.xul
@@ +323,5 @@
>                    </hbox>
> +                  <hbox id="raw-headers"
> +                        class="tabpanel-summary-container"
> +                        align="center"
> +                        hidden="true" />

This is invalid xul. Don't close the hbox tag here.

@@ +326,5 @@
> +                        align="center"
> +                        hidden="true" />
> +                    <hbox id="raw-request-headers-textarea-box" flex="1" hidden="true">
> +                      <textbox id="raw-request-headers-textarea" flex="1" multiline="true"/>
> +                    </vbox>


@@ +329,5 @@
> +                      <textbox id="raw-request-headers-textarea" flex="1" multiline="true"/>
> +                    </vbox>
> +                    <hbox id="raw-response-headers-textarea-box" flex="1" hidden="true">
> +                      <textbox id="raw-response-headers-textarea" flex="1" multiline="true"/>
> +                    </vbox>

Attachment #8489165 - Flags: feedback+
Flags: needinfo?(vporof)
- Issues fixed.
- Toggle behaviour added for this 'Raw headers' button.
- Tests added.
Attachment #8489165 - Attachment is obsolete: true
Attachment #8496577 - Flags: review?(vporof)
Comment on attachment 8496577 [details] [diff] [review]
Add a plain request/response headers view.

Review of attachment 8496577 [details] [diff] [review]:

I like this! There's a point that can be made about displaying the actual raw headers text instead of reconstructing it (as it is with the implementation in this patch), but we can always do that in a followup if anybody complains.

The only suggestion I have is increasing the height of the texboxes a little more. A { height: ?vh } rule in the css should work, although I'll let you decide what number should be used, as long as it's in viewport units (vh). 

r+ with the above tweak.
Attachment #8496577 - Flags: review?(vporof) → review+
Once you fix the above, please add a patch and a checkin-needed keyword in this bug.
Hi Victor. My patch is updated with the css rule requested.
I'm not assigned to the bug, therefore I can't edit the bug to add checkin-needed keyword.
Attachment #8497450 - Attachment is obsolete: true
Assignee: nobody → raphael
Keywords: checkin-needed
Can we please verify that this passes tests on Try first?
Keywords: checkin-needed
Test passes locally. I can't push it on Try.

Victor, can you push my patch on Try ?
Flags: needinfo?(vporof)
Flags: needinfo?(vporof)
My test failed on Try. I've updated my patch to address the issue.
If the try is green this time, don't forget to add the checkin-needed flag. Thanks!
Try is green.
Keywords: checkin-needed
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [lang=js] → [lang=js][fixed-in-fx-team]
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [lang=js][fixed-in-fx-team] → [lang=js]
Target Milestone: --- → Firefox 35
Release Note Request (optional, but appreciated)
[Why is this notable]: New feature in netmonitor
[Suggested wording]: Network Monitor : New request/response headers view
[Links (documentation, blog post, etc)]:
relnote-firefox: --- → ?
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.