Closed Bug 1398522 Opened 2 years ago Closed 2 years ago

Devtools/Netmonitor - sort request/response headers in Headers tab

Categories

(DevTools :: Netmonitor, enhancement, P3)

enhancement

Tracking

(firefox57 fixed)

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: wip.the.gruik, Assigned: abhinav.koppula, Mentored)

Details

(Keywords: good-first-bug)

Attachments

(1 file)

It could help the readability of Netmonitor's Headers tab content if request and response headers were sorted alphabetically (case insensitive).

Since order matters if a same header is set several times, it should be taken in account, at least after bug 1095273 is fixed. 

Developers caring about the real headers' order can still use "Raw headers".
Yep, I like that idea.

Honza
Mentor: odvarko
Severity: minor → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: good-first-bug
Priority: -- → P3
Hi Honza,
I have taken a stab at this issue.
Please let me know if I have missed anything.
Comment on attachment 8909078 [details]
Bug 1398522 - Sort 'Request-Headers' and 'Response-Headers' in 'Headers' tab.

https://reviewboard.mozilla.org/r/180678/#review185940

Thanks for working on this!

I am seeing test failure when running the test

Browser Chrome Test Summary
        Passed: 22
        Failed: 1
        Todo: 0
        Mode: e10s
*** End BrowserChrome Test Results ***
The following tests failed:
85 INFO TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_headers_sorted.js | Actual label value Request headers (471 B) not equal to expected label value Request headers (466 B) - Got Request headers (471 B), expected Request headers (466 B)
Stack trace:
    chrome://mochikit/content/browser-test.js:test_is:1011
    chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_headers_sorted.js:null:37
    chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_headers_sorted.js:null:36
    Tester_execTest@chrome://mochikit/content/browser-test.js:798:9
    Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:697:9
    SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59
Buffered messages finished
SUITE-END | took 10s
Attachment #8909078 - Flags: review?(odvarko) → review-
Just like I mentioned in bug 1398524, we should sort also
URL query arguments (in the Params side panel)
Needs a follow up

Honza
Assignee: nobody → abhinav.koppula
Status: NEW → ASSIGNED
Hi Honza,
Interestingly on my local, all tests are passing.
TEST-INFO | checking window state
Browser Chrome Test Summary
    Passed: 23
    Failed: 0
    Todo: 0
    Mode: e10s
*** End BrowserChrome Test Results ***
Buffered messages finished
SUITE-END | took 9s

Can you please push to try once so that I can check if TRY shows the same results or not?
I don't have access to push to TRY.
Flags: needinfo?(odvarko)
Sure, here is the try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=daf5775178467ae408b4ae96f85f7645086c0de7

If you are interested in getting access to the Try server read this instructions:
https://wiki.mozilla.org/Build:TryServer#Getting_access_to_the_Try_Server
You'll need Level 1 Commit Access and I'll vouch for you

Honza
Flags: needinfo?(odvarko)
Comment on attachment 8909078 [details]
Bug 1398522 - Sort 'Request-Headers' and 'Response-Headers' in 'Headers' tab.

https://reviewboard.mozilla.org/r/180678/#review187054

OK, try is green, let's land this!
https://treeherder.mozilla.org/#/jobs?repo=try&revision=daf5775178467ae408b4ae96f85f7645086c0de7

R+

Honza
Attachment #8909078 - Flags: review- → review+
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/48e2fba9e04e
Sort 'Request-Headers' and 'Response-Headers' in 'Headers' tab. r=Honza
Sorry, this had to be backed out for failing devtools' devtools/client/netmonitor/test/browser_net_headers_sorted.js:

https://hg.mozilla.org/integration/autoland/rev/7a40d9d95e3c59295b0d7adafbad27e777d1c20c

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=48e2fba9e04e3d209b92a02d1e7198ac879703e9&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=132204964&repo=autoland

11:46:24     INFO -  Buffered messages finished
11:46:24    ERROR -  614 INFO TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_headers_sorted.js | Actual label value Request headers (463 B) not equal to expected label value Request headers (466 B) - Got Request headers (463 B), expected Request headers (466 B)
11:46:24     INFO -  Stack trace:
11:46:24     INFO -      chrome://mochikit/content/browser-test.js:test_is:1011
11:46:24     INFO -      chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_headers_sorted.js:null:37
11:46:24     INFO -      chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_headers_sorted.js:null:36
11:46:24     INFO -      Tester_execTest@chrome://mochikit/content/browser-test.js:798:9
11:46:24     INFO -      Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:697:9

Please fix the issue and submit an updated patch.
Flags: needinfo?(abhinav.koppula)
Hi Sebastian, Honza,
Sorry for the inconvenience caused.
I have updated my mozreview-request and have changed the failing test.
Please review and let me know if it is fine.
Flags: needinfo?(abhinav.koppula)
Comment on attachment 8909078 [details]
Bug 1398522 - Sort 'Request-Headers' and 'Response-Headers' in 'Headers' tab.

https://reviewboard.mozilla.org/r/180678/#review187408

The test passes for me on Win10 now, let's try to re-land.

Thanks for the patch update!
Honza
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/10fe5727b04f
Sort 'Request-Headers' and 'Response-Headers' in 'Headers' tab. r=Honza
https://hg.mozilla.org/mozilla-central/rev/10fe5727b04f
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Tested on Nightly 58.0a1 20170922100051, works great.

It's that kind of little thing that sometime eases one's life.
Thanks guys :)
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.