1445963 - Array payload is rendered as object in the params panel

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P3)

Description

[Nicolas Chevobbe [:nchevobbe]]

Steps to reproduce:

1. Open the netmonitor
2. Go to the console
3. Evaluate the following

```js
fetch("https://jsonplaceholder.typicode.com/posts/1", {
  method: "PUT",
  headers: {
    "Content-Type": "application/hal+json",
  },
  body: JSON.stringify({
    watches: ['hello', 'how', 'are', 'you'],
  }),
})
```

4. Select the requests in the netmonitor
5. Open the "Params" panel

Expected result:

In the JSON View I see `▼ watches […]`

Actual result:

In the JSON View I see `▼ watches {…}`, i.e. I can think `watches` is an object, and not an array, as it is.

Comment 1

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

Attachment: Bug 1445963 - Render array payload as array in the params panel;

Review commit: https://reviewboard.mozilla.org/r/228288/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228288/

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8959505 [details]
Bug 1445963 - Render array payload as array in the params panel;

https://reviewboard.mozilla.org/r/228288/#review234232

Looks good. If I understand correctly, it was the sorting of the array that would transform it into an object. Maybe we could state it in the commit message ?
I have one comment about the test, but this is already good to go.
Thanks Honza for this quick patch :)

::: devtools/client/netmonitor/test/browser_net_params_sorted.js:32
(Diff revision 1)
> -  let actualKeys = document.querySelectorAll(".treeLabel");
> -  let expectedKeys = ["Query string", "baz", "foo", "type",
> -                      "Form data", "baz", "foo"];
> +  let actualKeys = document.querySelectorAll(".treeTable .treeRow");
> +  let expectedKeys = [
> +    "JSON",
> +    "watches: [...]",
> +    "0: hello",
> +    "1: how",
> +    "2: are",
> +    "3: you",
> +    "4: {...}",
> +    "a: 10",
> +    "b: [...]",
> +    "0: a",
> +    "1: c",
> +    "2: b",
> +    "c: 15",
> +  ];

it's a bit hard to see which properties are root and which one are children.
Could we somehow have a "text renderer", like we have in the ObjectInspector and assert this: 

```js
is(
  renderTextTree(document.querySelectorAll(".treeTable"),
`▼ JSON
  ▼ watches: […]
      0: hello
      1: how
      ...
    ▼ 4: {…}
        a: 10
      ▼ b: […]  
`);
```

Comment 3

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

Comment on attachment 8959505 [details]
Bug 1445963 - Render array payload as array in the params panel;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228288/diff/1-2/

Comment 4

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

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #2)
> Looks good. If I understand correctly, it was the sorting of the array that
> would transform it into an object. Maybe we could state it in the commit
> message ?
Done

> it's a bit hard to see which properties are root and which one are children.
> Could we somehow have a "text renderer", like we have in the ObjectInspector
> and assert this: 
> 
> ```js
> is(
>   renderTextTree(document.querySelectorAll(".treeTable"),
> `▼ JSON
>   ▼ watches: […]
>       0: hello
>       1: how
>       ...
>     ▼ 4: {…}
>         a: 10
>       ▼ b: […]  
> `);
> ```
I turned this into a comment to not make the test-code to complex.
But, if this pattern repeats in net monitor test we might introduce
a helper method!

Thanks!

Honza

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c8543a491242
Render array payload as array in the params panel; r=nchevobbe

Comment 6

[Code Review Bot [:reviewbot]]

Comment on attachment 8959505 [details]
Bug 1445963 - Render array payload as array in the params panel;

https://reviewboard.mozilla.org/r/228288/#review234464


Code analysis found 2 defects in this patch:
 - 2 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: devtools/client/netmonitor/test/html_post-array-data-test-page.html:25
(Diff revision 2)
> +        let xhr = new XMLHttpRequest();
> +        xhr.open("POST", "sjs_simple-test-server.sjs", true);
> +
> +        let postData = JSON.stringify({
> +          watches: ["hello", "how", "are", "you",
> +            {

Error: Expected indentation of 14 spaces but found 6. [eslint: indent-legacy]

::: devtools/client/netmonitor/test/html_post-array-data-test-page.html:29
(Diff revision 2)
> +          watches: ["hello", "how", "are", "you",
> +            {
> +              a: 10,
> +              c: 15,
> +              b: ["a", "c", "b"],
> +            }

Error: Expected indentation of 14 spaces but found 6. [eslint: indent-legacy]

Comment 7

[Cristian Brindusan [:cbrindusan]]

Backed out for ESlint failure on html_post-array-data-test-page.html

Url of the backout: https://hg.mozilla.org/integration/autoland/rev/a0a9ff478d2d8eb44e75f5497ea37c420c2e78cb

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=c8543a4912420f3b6bc18be2111c18f96ab21a66&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success&filter-resultStatus=pending&filter-resultStatus=running&filter-resultStatus=superseded

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=168788454&repo=autoland&lineNumber=248

 creating build/lib.linux-x86_64-2.7/psutil
[task 2018-03-18T08:22:49.896Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so
[task 2018-03-18T08:22:49.896Z] building 'psutil._psutil_posix' extension
[task 2018-03-18T08:22:49.896Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2018-03-18T08:22:49.897Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2018-03-18T08:22:49.897Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so
[task 2018-03-18T08:22:49.897Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so -> psutil
[task 2018-03-18T08:22:49.897Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so -> psutil
[task 2018-03-18T08:22:49.897Z] 
[task 2018-03-18T08:22:49.897Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2018-03-18T08:27:14.504Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/client/netmonitor/test/html_post-array-data-test-page.html:25:13 | Expected indentation of 14 spaces but found 6. (indent-legacy)
[task 2018-03-18T08:27:14.504Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/client/netmonitor/test/html_post-array-data-test-page.html:29:13 | Expected indentation of 14 spaces but found 6. (indent-legacy)
[taskcluster 2018-03-18 08:27:14.870Z] === Task Finished ===

Comment 8

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

Comment on attachment 8959505 [details]
Bug 1445963 - Render array payload as array in the params panel;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228288/diff/2-3/

Comment 9

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

Should be fixed now, try push eslint green
https://treeherder.mozilla.org/#/jobs?repo=try&revision=136878962672acc5779c3b954bdfb64fb14acdd8

Landing again.
Honza

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/11e482980c35
Render array payload as array in the params panel; r=nchevobbe

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/11e482980c35