Closed Bug 1398524 Opened 7 years ago Closed 7 years ago

Devtools/Netmonitor - sort request/response cookies in Cookies tab

Categories

(DevTools :: Netmonitor, enhancement, P3)

Product:
DevTools
Component:
Netmonitor
Type:
enhancement

Tracking

(firefox58 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 58
Tracking Flags:
Tracking Status
firefox58 --- fixed

People

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

Details

(Keywords: good-first-bug)

Attachments

(2 files)

Bug 1398524 - Sort Request-Cookies,Response-Cookies in 'Cookies' tab and Query-String,Form-Params,JSON in Params tab.
59 bytes, text/x-review-board-request
Honza
: review+
Details
CookieNestedHeaders.png
291.66 KB, image/png
Details 
It could help the readability of Netmonitor's Cookies tab if request and response cookies were sorted alphabetically (case insensitive).
I like that idea, thanks for the report!

Honza
Severity: minor → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Is this the file we'd be looking at modifying? http://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/components/cookies-panel.js
Flags: needinfo?(odvarko)
Hi Honza,
I have taken a stab at this issue and have submitted a mozreview-request.
Please let me know if I have missed anything.
Comment on attachment 8909045 [details]
Bug 1398524 - Sort Request-Cookies,Response-Cookies in 'Cookies' tab and Query-String,Form-Params,JSON in Params tab.

https://reviewboard.mozilla.org/r/180640/#review185934

Thanks for the patch!

It would be great if Params and Headers are also sorted.
File a follow up?

R+ assuming try is green

Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=09950e79edba495499cbe482a990fc236d8d3fc2

Honza

::: devtools/client/netmonitor/src/components/cookies-panel.js:77
(Diff revision 2)
> +/**
> + * Sorts object by keys in alphabetical order
> + * If object has nested children, it sorts the child-elements also by keys
> + * @param {object} which should be sorted by keys in alphabetical order
> + */
> +function sortObjectKeys(object) {

This method might be useful for sorting other net side panels (params and headers). In such case we should put it into utils/sort-utils.js (the files doesn't exist yet).

::: devtools/client/netmonitor/src/components/cookies-panel.js:83
(Diff revision 2)
> +  return Object.keys(object).sort(function (left, right) {
> +    return left.toLowerCase().localeCompare(right.toLowerCase());
> +  }).reduce((acc, key) => {
> +    if (typeof object[key] === "object") {
> +      acc[key] = sortObjectKeys(object[key]);
> +    } else {

Just curious, why it does the recursion here?
Attachment #8909045 - Flags: review?(odvarko) → review+
Assignee: nobody → abhinav.koppula
Status: NEW → ASSIGNED
Flags: needinfo?(odvarko)
Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=32dd89b6ae7f

@Abhinav: please see my question in comment #7

Honza
Flags: needinfo?(abhinav.koppula)
Attached image CookieNestedHeaders.png — 
Hi Honza,
1. I am seeing that both bug 1398522 and this issue are linked as far as refactoring is concerned. Can we first land bug 1398522 and then as part of this bug, I will refactor the sort method to a utils file? Also, I was thinking of sorting the Params tab as part of this bug only. Your thoughts on the above things? 

2. I was using recursion after I saw that we sometimes render nested objects in Cookies Panel as the attached screenshot shows for youtube.com. So I felt that all keys(child as well as parent) should be sorted.
Flags: needinfo?(abhinav.koppula) → needinfo?(odvarko)
(In reply to Abhinav Koppula from comment #9)
> Created attachment 8910392 [details]
> CookieNestedHeaders.png
> 
> Hi Honza,
> 1. I am seeing that both bug 1398522 and this issue are linked as far as
> refactoring is concerned. Can we first land bug 1398522
Yes

> and then as part of
> this bug, I will refactor the sort method to a utils file?
Yes

> Also, I was
> thinking of sorting the Params tab as part of this bug only. Your thoughts
> on the above things? 
Sounds good to me.

> 2. I was using recursion after I saw that we sometimes render nested objects
> in Cookies Panel as the attached screenshot shows for youtube.com. So I felt
> that all keys(child as well as parent) should be sorted.
I see

Thanks!
Honza
Flags: needinfo?(odvarko)
Comment on attachment 8909045 [details]
Bug 1398524 - Sort Request-Cookies,Response-Cookies in 'Cookies' tab and Query-String,Form-Params,JSON in Params tab.

https://reviewboard.mozilla.org/r/180640/#review185934

Hi Honza,
Thanks for the review.
I have moved the sorting to a utils file and have sorted Headers/Cookies/Params panels using the utils.
I am seeing some failures
devtools/client/netmonitor/test/browser_net_post-data-01.js | The first query param name was incorrect.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=30fb3c9ce23c&selectedJob=133090872

Honza
Flags: needinfo?(abhinav.koppula)
Hi Honza,
I have updated the review-request and have fixed the following tests - browser_net_post-data-01.js, browser_net_post-data-02.js and browser_net_post-data-03.js which were failing due to my changes.
browser_net_prefs-reload.js seems to fail with and without my changes on my local.
Can you push it to TRY once?
Flags: needinfo?(abhinav.koppula)
Comment on attachment 8909045 [details]
Bug 1398524 - Sort Request-Cookies,Response-Cookies in 'Cookies' tab and Query-String,Form-Params,JSON in Params tab.

https://reviewboard.mozilla.org/r/180640/#review190896

Try looks good, let's try to land.

Great job Abhinav!

Honza
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7c6aaee62972
Sort Request-Cookies,Response-Cookies in 'Cookies' tab and Query-String,Form-Params,JSON in Params tab. r=Honza
https://hg.mozilla.org/mozilla-central/rev/7c6aaee62972
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: