Closed Bug 1445963 Opened 4 years ago Closed 4 years ago

Array payload is rendered as object in the params panel

Categories

(DevTools :: Netmonitor, defect, P3)

defect

Tracking

(firefox61 fixed)

RESOLVED FIXED
Firefox 61
Tracking Status
firefox61 --- fixed

People

(Reporter: nchevobbe, Assigned: Honza)

Details

Attachments

(1 file)

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.
Priority: -- → P3
Assignee: nobody → odvarko
Status: NEW → ASSIGNED
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: […]  
`);
```
Attachment #8959505 - Flags: review?(nchevobbe) → review+
(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
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c8543a491242
Render array payload as array in the params panel; r=nchevobbe
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]
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 ===
Flags: needinfo?(odvarko)
Should be fixed now, try push eslint green
https://treeherder.mozilla.org/#/jobs?repo=try&revision=136878962672acc5779c3b954bdfb64fb14acdd8

Landing again.
Honza
Flags: needinfo?(odvarko)
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/11e482980c35
Render array payload as array in the params panel; r=nchevobbe
https://hg.mozilla.org/mozilla-central/rev/11e482980c35
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.