1361739 - Params showing wrong in HTTP inspector in specific case

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P3)

Description

[vedantseta007]

Attachment: Screen Shot 2017-05-03 at 8.32.17 PM.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36
Firefox for Android

Steps to reproduce:

User Agent : Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:53.0) Gecko/20100101 Firefox/53.0

 
http://randomdomain.com?asd[]=&pr=&asd[]=1&asd[]=2

- Open Console
- In HTTP Inspector
- Check Params Tab




Actual results:

Params : asd[] : [2] 
    0 :  "1"
    1 :  "2"


Expected results:

Case 1 : Params : asd[] : [3]
    0 : "" 
    1 :  "1"
    2 :  "2"

Comment 1

[tossj]

This error only happens when the first occurrence of a repeated query parameter is blank. If the blank parameter appears after one with the same name with data, it will be included as a blank parameter in the parameter panel.

This seems to happen because of how multiple parameters of the same name are handled in ParamsPanel.js' getProperties(): https://searchfox.org/mozilla-central/rev/5786b9be9f887ed371c804e786081b476a403104/devtools/client/netmonitor/src/components/ParamsPanel.js#82. The current code does not check if the first occurrence of a parameter was a blank string, and if it was, will not make the parameter's value into a list of values upon receiving a second value for the same parameter name.

I can contribute a fix if Firefox is interested in resolving this issue.

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Great, assigned to you :-)
Honza

Comment 3

[tossj]

Attachment: 1362739.patch

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fc31c60e18f764693dbdbdef08cf14ab51dc61e2

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Looks good to me, thanks for the patch!

Let's see if the Try is green before R+

Honza

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Eslint errors:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9917d61e330897c109cb47ea8f1a2a062e69c416&selectedJob=204770778

Comment 7

[tossj]

Attachment: 1362739.patch

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b2eca48899fc5efb2338f08b4ca1f0b06610ab0

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks for the update!

New Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7bc8a7f55bc2d296e6b043fa4a88b7ba437eb344

Let's see how it goes now.

Honza

Comment 10

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 9016345 [details] [diff] [review]
1362739.patch

Review of attachment 9016345 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, thanks for working on this!

Honza

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c535988954c
Fix Params Panel to display empty values when multiple values exist for one parameter. r=Honza

Comment 12

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/9c535988954c