1264557 - JSON Viewer: manually auto-expand documents

Status: RESOLVED FIXED

(DevTools :: JSON Viewer, defect, P3)

Description

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

This is a follow up for Bug 1247064

JSON documents that are not bigger than 100KB are autoexpanded, but there should also be a toolbar-button that can be used to auto expand manually any document.

Honza

Comment 1

[Ognjen Galic]

Attachment: 1264557-add-expand-collapse.patch

This is a patch that adds two buttons to the toolbar in the "JSON" tab.
The buttons are:

"Expand All" - Expands the whole JSON tree
"Collapse All" - Collapses the whole JSON tree

See the attached screenshot for a reference.

Comment 2

[Ognjen Galic]

Attachment: Screenshot of the buttons added in the patch

Comment 3

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

Comment on attachment 8964276 [details] [diff] [review]
1264557-add-expand-collapse.patch

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

This is nice feature, thanks for the patch!

Please see my inline comments.

Also, my biggest worry is about performance.
Expanding big JSON docs can be time consuming and it can freeze the browser
for significant amount of time. We want to avoid that.

I think that the `Expand All` button should be there only for documents
smaller then AUTO_EXPAND_MAX_SIZE.

The Collapse button seem to be fine, I don't see perf penalties there.

Honza

::: devtools/client/jsonview/json-viewer.js
@@ +86,5 @@
>        input.prettified = !input.prettified;
>      },
> +
> +    onCollapseTree: function() {
> +      input.expandedNodes.clear();

`input.expandedNodes` is null if the document is bigger then AUTO_EXPAND_MAX_SIZE.

The `expandedNodes` set it only set if the tree is auto expanded.

Search for this comment:
// Expand the document by default if its size isn't bigger than 100KB.

I think that it should be initialized to an empty set when the application state object `input` is defined (at the top of the file).

@@ +91,5 @@
> +      theApp.forceUpdate();
> +    },
> +
> +    onExpandTree: function() {
> +      input.expandedNodes.clear();

`input.expandedNodes` is null if the document is bigger then AUTO_EXPAND_MAX_SIZE.

Comment 5

[Ognjen Galic]

I think that the button should stay functional regardless of the document size. 

It makes sense to disallow such a long-running operation on a document that is automatically loaded, but I think it is acceptable on a action that is triggered by the user.

I would like to hear your comment about this, and then I will fix the nullpointer issue you pointed out.

Thanks for the time!

Comment 6

[Oriol Brufau [:Oriol]]

Comment on attachment 8964276 [details] [diff] [review]
1264557-add-expand-collapse.patch

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

::: devtools/client/jsonview/json-viewer.js
@@ +94,5 @@
> +    onExpandTree: function() {
> +      input.expandedNodes.clear();
> +      TreeViewClass.getExpandedNodes(
> +        input.json
> +      ).forEach((item) => {

Creating a set with all nodes can be expensive. Maybe it would be better to have a TreeView flag which says whether nodes are expanded or collapsed by default. If they are hidden by default, expandedNodes contains the ones which are expanded. If they are expanded by default, introduce a new collapsedNodes set which contains the ones which are collapsed.

What do you think? This would require some changes in https://searchfox.org/mozilla-central/source/devtools/client/shared/components/tree/TreeView.js

@@ +95,5 @@
> +      input.expandedNodes.clear();
> +      TreeViewClass.getExpandedNodes(
> +        input.json
> +      ).forEach((item) => {
> +        input.expandedNodes.add(item);

Why do you copy all set items into the old set, one by one?
I would just replace the set, i.e.

  input.expandedNodes = TreeViewClass.getExpandedNodes(input.json);
  theApp.setState({expandedNodes: input.expandedNodes});

@@ +97,5 @@
> +        input.json
> +      ).forEach((item) => {
> +        input.expandedNodes.add(item);
> +      });
> +      theApp.forceUpdate();

If you replace the set (see comment above), I think forceUpdate is not needed.

Comment 7

[Ognjen Galic]

I copied all the stuff as the tree was not updating after a replace and forceUpdate(). No idea what was that.

I think the whole idea of removing and adding tree items is very expensive. Could we maybe collapse the rows in the TreeView to 0px instead of removing them?

Comment 8

[Oriol Brufau [:Oriol]]

(In reply to Ognjen Galic from comment #7)
> I think the whole idea of removing and adding tree items is very expensive.
> Could we maybe collapse the rows in the TreeView to 0px instead of removing
> them?

React can't handle big trees in a reasonable amount of time. If you always render all rows (even if then you hide them with CSS) I suspect it would be much worse.

Comment 9

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

(In reply to Jan Honza Odvarko [:Honza] from comment #3)
> Also, my biggest worry is about performance.
> Expanding big JSON docs can be time consuming and it can freeze the browser
> for significant amount of time. We want to avoid that.
> 
> I think that the `Expand All` button should be there only for documents
> smaller then AUTO_EXPAND_MAX_SIZE.
> 
> The Collapse button seem to be fine, I don't see perf penalties there.

(In reply to Ognjen Galic from comment #5)
> I think that the button should stay functional regardless of the document
> size. 
> 
> It makes sense to disallow such a long-running operation on a document that
> is automatically loaded, but I think it is acceptable on a action that is
> triggered by the user.

@Harald, what do you think about this?
Should we let the use to expand-all (manually) even if it can freeze the browser?

Honza

Comment 10

[:Harald Kirschner :digitarald]

> React can't handle big trees in a reasonable amount of time. If you always render all rows (even if then you hide them with CSS) I suspect it would be much worse.

Where is the time spent? Would React 16's new async methods that stay within frame budget help here as follow up work?

> Should we let the use to expand-all (manually) even if it can freeze the browser?

Sounds OK, as long as there is visual feedback (active state + compositor based animation).

- Are there other issues that users could run into like memory that could lead to worse experience than hangs?
- Does this hang main or content thread?

Comment 11

[Ognjen Galic]

(In reply to :Harald Kirschner :digitarald from comment #10)
> > React can't handle big trees in a reasonable amount of time. If you always render all rows (even if then you hide them with CSS) I suspect it would be much worse.
> 
> Where is the time spent? Would React 16's new async methods that stay within
> frame budget help here as follow up work?
> 
> > Should we let the use to expand-all (manually) even if it can freeze the browser?
> 
> Sounds OK, as long as there is visual feedback (active state + compositor
> based animation).

Is there any examples I could use for info about Firefox's internal threads or any documentation how to offload work from the display thread?

> 
> - Are there other issues that users could run into like memory that could
> lead to worse experience than hangs?

I did some memory profiles and seems that the garbage collector does it's job fine, no object retention.

> - Does this hang main or content thread?

It does hang some thread. Don't know which.

Comment 12

[Oriol Brufau [:Oriol]]

Attachment: jsonview-hang.png

(In reply to :Harald Kirschner :digitarald from comment #10)
> Where is the time spent?

Time is spent in react, which is blatantly slow.
If all rows are rendered, then the render() method will take a lot.
And then, anytime something changes, react will diff. Since both the old and new trees will be huge, this will also be very expensive.

> Would React 16's new async methods that stay within frame budget help here as follow up work?

I don't know these methods, but I suspect there is no hope unless we get rid of react.

> Does this hang main or content thread?

Content thread. See attached image.

Comment 13

[Ognjen Galic]

Attachment: 1264557-add-expand-collapse-v2.patch

This is a patch that has a couple of changes in relation to the past patch.

* The expandedNodes set is not copied one-by-one, but is replaced as suggested by Oriol
* The "Expand All" button is now hidden for documents larger than 100kB, the same size 
   that disables auto-expand, as suggested by Honza.

Comment 14

[Ognjen Galic]

Attachment: 1264557-add-expand-collapse-v3.patch

Sorry, this is the same patch as the previous one but a rogue "console.log" has been removed.

Comment 15

[Ognjen Galic]

Again, sorry for the mess, haven't used Bugzilla in a while.

Comment 16

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

Comment on attachment 8973438 [details] [diff] [review]
1264557-add-expand-collapse-v3.patch

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

Looks good to me, thanks!

We also need a test for this new feature. You can take a look at test/browser_jsonview_copy_json.js and get some inspiration (the new test will be very similar).

Honza

Comment 17

[Ognjen Galic]

Attachment: 1264557-add-expand-collapse-v4.patch

This is a small, little copy mistake that I did not saw. It did not impact the functionality but it inhibited testing. What I did was assign the same class for the "Expand All" and "Collapse All" buttons. 

That is fixed in this v4 patch, its just a one-line change.

Comment 18

[Ognjen Galic]

Attachment: 1264557-add-test-cases-for-expand-collapse-buttons.patch

This are the suggested changes. This is a test-case for the expand and collapse all buttons.

Comment 19

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

Comment on attachment 8974293 [details] [diff] [review]
1264557-add-expand-collapse-v4.patch

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

> This is a small, little copy mistake that I did not saw. It did not impact the functionality but it inhibited testing.
> What I did was assign the same class for the "Expand All" and "Collapse All" buttons. 

I see, looks good, thanks!
Honza

Comment 20

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

Comment on attachment 8974294 [details] [diff] [review]
1264557-add-test-cases-for-expand-collapse-buttons.patch

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

Thanks for implementing the test!

Looks good, just couple of typos

R+ assuming try is green.

You don't have to ask for another review after fixing the typos.
(just set the review flag to R+ manually, and don't forget to add checkin-needed keyword when the patch is ready)

Honza

::: devtools/client/jsonview/test/browser_jsonview_expand_collapse.js
@@ +15,5 @@
> +  let browser = gBrowser.selectedBrowser, selector, countAfter, countBefore, json;
> +
> +  /* Initial sanity check */
> +  countBefore = await getElementCount(".treeRow");
> +  ok(countBefore == 6, "There must be six row");

nit: six row => six rows

@@ +27,5 @@
> +  /* Test the "Expand All" button */
> +  selector = ".jsonPanelBox .toolbar button.expand";
> +  await BrowserTestUtils.synthesizeMouseAtCenter(selector, {}, browser);
> +  countAfter = await getElementCount(".treeRow");
> +  ok(countAfter == 6, "There mus be six expanded rows");

nit: there mus => there must

Comment 21

[Ognjen Galic]

Attachment: 1264557-add-test-cases-for-expand-collapse-buttons-v2.patch

Thanks for the reviews and pointers. This fixes the nits.

Comment 22

[Ognjen Galic]

Seems like I can't edit the keywords. Is it because I did not report the bug?

Comment 23

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

(In reply to Ognjen Galic from comment #22)
> Seems like I can't edit the keywords. Is it because I did not report the bug?
I see, it requires commit access.

You might consider becoming Mozilla committer. Read more here:
https://www.mozilla.org/en-US/about/governance/policies/commit/

I am adding the keyword now.

Thanks for the help!
Honza

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d3bf0a95e4e4
Add expand all/collapse all buttons. r=honza
https://hg.mozilla.org/integration/mozilla-inbound/rev/e17cd801b578
Add test cases for expand/collapse buttons. r=honza

Comment 25

[Natalia Csoregi [:nataliaCs]]

Backed out for ESlint failure.

backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/8cd4cabdce0bc4c55cbb6a6c783563286af0bcc2
push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=ac582f3509d43aea21251329dae9f314b4875681
failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=177834397&repo=mozilla-inbound&lineNumber=252

[task 2018-05-10T10:33:36.871Z] New python executable in /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python2.7
[task 2018-05-10T10:33:36.871Z] Also creating executable in /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python
[task 2018-05-10T10:33:38.514Z] Installing setuptools, pip, wheel...done.
[task 2018-05-10T10:33:39.560Z] running build_ext
[task 2018-05-10T10:33:39.560Z] building 'psutil._psutil_linux' extension
[task 2018-05-10T10:33:39.560Z] creating build
[task 2018-05-10T10:33:39.560Z] creating build/temp.linux-x86_64-2.7
[task 2018-05-10T10:33:39.560Z] creating build/temp.linux-x86_64-2.7/psutil
[task 2018-05-10T10:33:39.560Z] 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-05-10T10:33:39.561Z] 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-05-10T10:33:39.561Z] 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_linux.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o
[task 2018-05-10T10:33:39.561Z] creating build/lib.linux-x86_64-2.7
[task 2018-05-10T10:33:39.562Z] creating build/lib.linux-x86_64-2.7/psutil
[task 2018-05-10T10:33:39.562Z] 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-05-10T10:33:39.562Z] building 'psutil._psutil_posix' extension
[task 2018-05-10T10:33:39.563Z] 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-05-10T10:33:39.563Z] 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-05-10T10:33:39.564Z] 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-05-10T10:33:39.564Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so -> psutil
[task 2018-05-10T10:33:39.564Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so -> psutil
[task 2018-05-10T10:33:39.564Z] 
[task 2018-05-10T10:33:39.564Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2018-05-10T10:38:46.689Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/client/jsonview/test/browser_jsonview_expand_collapse.js:39:1 | Block must not be padded by blank lines. (padded-blocks)
[taskcluster 2018-05-10 10:38:46.963Z] === Task Finished ===
[taskcluster 2018-05-10 10:38:46.963Z] Unsuccessful task run with exit code: 1 completed in 552.276 seconds

Comment 26

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

@Ognjen: from the log:

TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/client/jsonview/test/browser_jsonview_expand_collapse.js:39:1 | Block must not be padded by blank lines. (padded-blocks)

Honza

Comment 27

[Oriol Brufau [:Oriol]]

(In reply to Jan Honza Odvarko [:Honza] from comment #23)
> I see, it requires commit access.

More than commit access, I think it requires the bugzilla editbugs privilege.
https://developer.mozilla.org/docs/Mozilla/Bugzilla/What_to_do_and_what_not_to_do_in_Bugzilla#Editbugs_privilege

To obtain it, see
https://bugzilla.mozilla.org/page.cgi?id=get_permissions.html

Comment 28

[Ognjen Galic]

Attachment: 1264557-add-test-cases-for-expand-collapse-buttons-v3.patch

This is a really funny issue. The whole fix is removing a single newline. 
This patch is the same as the previous, the only difference is the newline on line 39.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c45f75d2190f
Add test cases for expand/collapse buttons r=[e-mail]

Comment 30

[Arthur Iakab [arthur_iakab]]

Backed out changeset c45f75d2190f (bug 1264557) for failing pt-test-verify-e10s on devtools/client/jsonview/test/browser_jsonview_expand_collapse.j on a CLOSED TREE
Failing log: https://treeherder.mozilla.org/logviewer.html#?job_id=177963750&repo=mozilla-inbound&lineNumber=1965
Backout revision https://hg.mozilla.org/integration/mozilla-inbound/rev/23e1618c1c3959196c3b22fc44d9e69733306115
Failing revision https://hg.mozilla.org/integration/mozilla-inbound/rev/c45f75d2190f9ac306c9694d4add288408ea1aba


[task 2018-05-10T23:03:43.387Z] 23:03:43     INFO - TEST-START | devtools/client/jsonview/test/browser_jsonview_expand_collapse.js
[task 2018-05-10T23:03:44.801Z] 23:03:44     INFO - TEST-INFO | started process screentopng
[task 2018-05-10T23:03:45.213Z] 23:03:45     INFO - TEST-INFO | screentopng: exit 0
[task 2018-05-10T23:03:45.213Z] 23:03:45     INFO - Buffered messages logged at 23:03:43
[task 2018-05-10T23:03:45.217Z] 23:03:45     INFO - Entering test bound 
[task 2018-05-10T23:03:45.218Z] 23:03:45     INFO - Test expand/collapse JSON started
[task 2018-05-10T23:03:45.219Z] 23:03:45     INFO - Adding a new JSON tab with URL: 'http://example.com/browser/devtools/client/jsonview/test/array_json.json'
[task 2018-05-10T23:03:45.220Z] 23:03:45     INFO - Adding a new tab with URL: http://example.com/browser/devtools/client/jsonview/test/array_json.json
[task 2018-05-10T23:03:45.222Z] 23:03:45     INFO - Console message: [JavaScript Warning: "Use of nsIFile in content process is deprecated." {file: "resource://gre/modules/FileUtils.jsm" line: 170}]
[task 2018-05-10T23:03:45.223Z] 23:03:45     INFO - Buffered messages logged at 23:03:44
[task 2018-05-10T23:03:45.225Z] 23:03:45     INFO - Console message: [JavaScript Warning: "Use of nsIFile in content process is deprecated." {file: "resource://gre/modules/FileUtils.jsm" line: 170}]
[task 2018-05-10T23:03:45.226Z] 23:03:45     INFO - Loading the helper frame script chrome://mochitests/content/browser/devtools/client/shared/test/frame-script-utils.js
[task 2018-05-10T23:03:45.227Z] 23:03:45     INFO - DocReadyState is "loading". Await "complete"
[task 2018-05-10T23:03:45.229Z] 23:03:45     INFO - DocReadyState is "interactive". Await "complete"
[task 2018-05-10T23:03:45.230Z] 23:03:45     INFO - AppReadyState is "uninitialized". Await "complete"
[task 2018-05-10T23:03:45.231Z] 23:03:45     INFO - Tab added and finished loading
[task 2018-05-10T23:03:45.232Z] 23:03:45     INFO - AppReadyState is "loading". Await "complete"
[task 2018-05-10T23:03:45.233Z] 23:03:45     INFO - Get element count: '.treeRow'
[task 2018-05-10T23:03:45.235Z] 23:03:45     INFO - Sending message Test:JsonView:GetElementCount to content
[task 2018-05-10T23:03:45.236Z] 23:03:45     INFO - Expecting message Test:JsonView:GetElementCount from content
[task 2018-05-10T23:03:45.237Z] 23:03:45     INFO - TEST-PASS | devtools/client/jsonview/test/browser_jsonview_expand_collapse.js | There must be six rows - 
[task 2018-05-10T23:03:45.238Z] 23:03:45     INFO - Get element count: '.treeRow'
[task 2018-05-10T23:03:45.240Z] 23:03:45     INFO - Sending message Test:JsonView:GetElementCount to content
[task 2018-05-10T23:03:45.241Z] 23:03:45     INFO - Expecting message Test:JsonView:GetElementCount from content
[task 2018-05-10T23:03:45.242Z] 23:03:45     INFO - Buffered messages finished
[task 2018-05-10T23:03:45.243Z] 23:03:45     INFO - TEST-UNEXPECTED-FAIL | devtools/client/jsonview/test/browser_jsonview_expand_collapse.js | There must be three rows - 
[task 2018-05-10T23:03:45.244Z] 23:03:45     INFO - Stack trace:
[task 2018-05-10T23:03:45.245Z] 23:03:45     INFO - chrome://mochitests/content/browser/devtools/client/jsonview/test/browser_jsonview_expand_collapse.js:null:25
[task 2018-05-10T23:03:45.247Z] 23:03:45     INFO - Get element count: '.treeRow'
[task 2018-05-10T23:03:45.248Z] 23:03:45     INFO - Sending message Test:JsonView:GetElementCount to content
[task 2018-05-10T23:03:45.249Z] 23:03:45     INFO - Expecting message Test:JsonView:GetElementCount from content
[task 2018-05-10T23:03:45.250Z] 23:03:45     INFO - TEST-PASS | devtools/client/jsonview/test/browser_jsonview_expand_collapse.js | There must be six expanded rows - 
[task 2018-05-10T23:03:45.251Z] 23:03:45     INFO - TEST-PASS | devtools/client/jsonview/test/browser_jsonview_expand_collapse.js | The generated JSON must be larger than 100kB -

Comment 31

[Ognjen Galic]

Is this failing because I need to roll up the two patches into one in order for it to be tested as one unit?

Comment 32

[Ognjen Galic]

This has failed because the patch set has not been applied in a series but the tests have been run without the previous patch that implements that functionality.

See: https://hg.mozilla.org/integration/mozilla-inbound/file/c45f75d2190f9ac306c9694d4add288408ea1aba/devtools/client/jsonview/components/JsonToolbar.js

There are no buttons in that revision for the test to click.

Comment 33

[Ognjen Galic]

So, ehm, what to do here? Roll up the patch or?

Comment 34

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

(In reply to Ognjen Galic from comment #33)
> So, ehm, what to do here? Roll up the patch or?

I don't know what that happened. But, please merge both
patches and attach just one file.

Thanks!
Honza

Comment 35

[Ognjen Galic]

Attachment: 1264557-add-expand-collapse-v6.patch

This is the patchset squashed into one patch.

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e03a8d74e9eb
Add expand all/collapse all buttons. r=Honza

Comment 37

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/e03a8d74e9eb