Closed Bug 1561362 Opened 4 years ago Closed 3 years ago

"Expand All" button missing in JSON Viewer

Categories

(DevTools :: JSON Viewer, defect, P3)

Product:
DevTools
Component:
JSON Viewer
Version:
67 Branch
Type:
defect

Tracking

(firefox76 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 76
Tracking Flags:
Tracking Status
firefox76 --- fixed

People

(Reporter: stevetcook, Assigned: zacnomore)

References

Details

(Keywords: good-first-bug, regression)

Attachments

(3 files)

Screenshot of Issue
14.73 KB, image/png
Details
Bug 1561362 - Implement slow label on JSON viewer expand all button r=Honza
47 bytes, text/x-phabricator-request
Details | Review
Bug 1561362 - Fix linting issues r=Honza
47 bytes, text/x-phabricator-request
Details | Review
Attached image Screenshot of Issue —

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

Steps to reproduce:

Navigated to a URL (private) that returns raw JSON.

Actual results:

JSON was parsed and displayed in JSON Viewer, with all nodes collapsed; "expand all" button was not shown.

Expected results:

According to Bug 1264557, this is a regression.
There should be an "expand all" button.

Component: Untriaged → JSON Viewer
Product: Firefox → DevTools

This is by design, see bug 1264557 comment 13

The priority flag is not set for this bug.
:Honza, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(odvarko)

Harald, instead of hiding the button completely - should we rather disable it with a tooltip explaining that the 'expand all' feature isn't available for JSON documents bigger than 100KB?

Honza

Flags: needinfo?(hkirschner)

Since we are running in content, it seems reasonable to always show the button.

We can explore to warn users for larger files that this can take a while (showing a [slow] warning in the button).

Flags: needinfo?(hkirschner)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(odvarko)
Priority: -- → P3

We can explore to warn users for larger files that this can take a while (showing a [slow] warning in the button).

FF74: I'd appreciate this. Definitely don't take the button unconditionally away for some arbitrary file size. What if I have a 64-core 128G RAM beast under my desk? Also, in some cases, it doesn't matter if it takes 1 minute to unfold - seeing important data is much more valuable.

Ok, let's add a (slow) label inside the button and always offer it.

The logic for hiding the button is in https://searchfox.org/mozilla-central/source/devtools/client/jsonview/components/JsonToolbar.js#76.

Keywords: good-first-bug
Assignee: nobody → cczac1
Status: NEW → ASSIGNED

Created this draft: https://phabricator.services.mozilla.com/D67766
I see some .orig files added in there, not sure if that's correct. Riding the hg struggle bus but doing my best!

This implementation shows the slow label if the data is above the size threshold

I looked into it more and it seems like those .orig files shouldn't be there so I removed them

The .orig files are created when resolving merge conflicts. They shouldn't be included indeed.

Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1e70c2d4173d
Implement slow label on JSON viewer expand all button r=Honza

Backed out for eslint failures on JsonToolbar.js

backout: https://hg.mozilla.org/integration/autoland/rev/cbe6a9e7a2eac1308d5548e0d3df4c55e36d675c

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=294474338&revision=1e70c2d4173d43dc252df5da61a17d6d80c92781&searchStr=linting%2Copt%2Cjavascript%2Cchecks%2Csource-test-mozlint-eslint%2Cjs%28es%29

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=294474338&repo=autoland&lineNumber=285

[task 2020-03-24T08:01:47.552Z] /builds/worker/checkouts/gecko/uriloader
[task 2020-03-24T08:01:47.576Z] 08:01:47.575 eslint (103) | Command: /usr/local/bin/node /builds/worker/checkouts/gecko/node_modules/eslint/bin/eslint.js --ext [js,jsm,jsx,xul,html,xhtml] --format json --no-error-on-unmatched-pattern --quiet --ignore-pattern dom/media/webspeech/recognition/endpointer.cc --ignore-pattern dom/u2f/tests/pkijs --ignore-pattern dom/tests/mochitest/ajax --ignore-pattern dom/tests/mochitest/dom-level1-core --ignore-pattern dom/webauthn/tests/pkijs --ignore-pattern dom/media/platforms/ffmpeg/libav54 --ignore-pattern dom/media/webaudio/test/blink --ignore-pattern dom/media/webspeech/recognition/energy_endpointer.h --ignore-pattern dom/imptests --ignore-pattern dom/media/platforms/ffmpeg/ffmpeg58 --ignore-pattern dom/media/gmp/widevine-adapter/content_decryption_module_export.h --ignore-pattern dom/tests/mochitest/dom-level2-html --ignore-pattern dom/media/webspeech/recognition/endpointer.h --ignore-pattern dom/media/webspeech/recognition/energy_endpointer_params.h --ignore-pattern dom/media/webspeech/recognition/energy_endpointer.cc --ignore-pattern dom/media/platforms/ffmpeg/libav53 --ignore-pattern dom/media/platforms/ffmpeg/ffmpeg57 --ignore-pattern dom/tests/mochitest/dom-level2-core --ignore-pattern dom/media/gmp/widevine-adapter/content_decryption_module_ext.h --ignore-pattern dom/media/gmp/rlz --ignore-pattern dom/media/platforms/ffmpeg/libav55 --ignore-pattern dom/canvas/test/webgl-conf/checkout --ignore-pattern dom/media/webvtt/vtt.jsm --ignore-pattern dom/media/gmp/widevine-adapter/content_decryption_module.h --ignore-pattern dom/media/webspeech/recognition/energy_endpointer_params.cc --ignore-pattern dom/webauthn/cbor-cpp --ignore-pattern dom/media/gmp/widevine-adapter/content_decryption_module_proxy.h /builds/worker/checkouts/gecko/dom /builds/worker/checkouts/gecko/memory /builds/worker/checkouts/gecko/services /builds/worker/checkouts/gecko/taskcluster /builds/worker/checkouts/gecko/config /builds/worker/checkouts/gecko/uriloader
[task 2020-03-24T08:02:20.149Z] 08:02:20.148 eslint (101) | Finished in 32.74 seconds
[task 2020-03-24T08:02:36.701Z] 08:02:36.700 eslint (106) | Finished in 49.29 seconds
[task 2020-03-24T08:02:42.396Z] 08:02:42.395 eslint (100) | Finished in 54.99 seconds
[task 2020-03-24T08:02:46.221Z] 08:02:46.221 eslint (107) | Finished in 58.81 seconds
[task 2020-03-24T08:03:38.571Z] 08:03:38.571 eslint (102) | Finished in 111.17 seconds
[task 2020-03-24T08:06:29.346Z] 08:06:29.346 eslint (104) | Finished in 281.94 seconds
[task 2020-03-24T08:07:17.134Z] 08:07:17.134 eslint (103) | Finished in 329.73 seconds
[task 2020-03-24T08:10:21.757Z] 08:10:21.757 eslint (105) | Finished in 514.35 seconds
[task 2020-03-24T08:10:21.763Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/client/jsonview/components/JsonToolbar.js:78:11 | Replace ··this.props.dataSize·>·EXPAND_THRESHOLD·?· with this.props.dataSize·>·EXPAND_THRESHOLD (prettier/prettier)
[task 2020-03-24T08:10:21.763Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/client/jsonview/components/JsonToolbar.js:79:13 | Replace ··JSONView.Locale["jsonViewer.ExpandAllSlow"]·: with ?·JSONView.Locale["jsonViewer.ExpandAllSlow"] (prettier/prettier)
[task 2020-03-24T08:10:21.763Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/client/jsonview/components/JsonToolbar.js:80:13 | Replace · with : (prettier/prettier)
[taskcluster 2020-03-24 08:10:22.280Z] === Task Finished ===
[taskcluster 2020-03-24 08:10:22.976Z] Unsuccessful task run with exit code: 1 completed in 747.213 seconds

Flags: needinfo?(cczac1)

Ah, I overlooked this,

@zacnomore:

Here is the sum of the issues:

/builds/worker/checkouts/gecko/devtools/client/jsonview/components/JsonToolbar.js:78:11 | Replace `··this.props.dataSize·>·EXPAND_THRESHOLD·?·` with `this.props.dataSize·>·EXPAND_THRESHOLD` (prettier/prettier)
/builds/worker/checkouts/gecko/devtools/client/jsonview/components/JsonToolbar.js:79:13 | Replace `··JSONView.Locale["jsonViewer.ExpandAllSlow"]·:` with `?·JSONView.Locale["jsonViewer.ExpandAllSlow"]` (prettier/prettier)
/builds/worker/checkouts/gecko/devtools/client/jsonview/components/JsonToolbar.js:80:13 | Replace `·` with `:` (prettier/prettier)

Honza

Depends on D67766

So sorry about that! I've got my linter set up correctly now as well. This fix should do it

Flags: needinfo?(cczac1)

(In reply to Zachary Svoboda :zacnomore from comment #15)

So sorry about that! I've got my linter set up correctly now as well. This fix should do it

Thanks for quick update

Can you please just update the previous revision D67766?

Honza

Flags: needinfo?(cczac1)

I've looked through these docs: https://docs.firefox-dev.tools/contributing/making-prs.html
And found that I'm looking to hg commit --amend but I'm not sure how to go back to the correct version. I tried hg clone -r c9f08ac7256d but that says invalid arguements

How do I get back to that revision so I can edit it?

(..Sorry, thank you!!)

Flags: needinfo?(cczac1)

You can use hg wip (alias of hg log --graph --rev=wip --template=wip) in order to see your commits.
Then use hg up -r REV where REV is the desired revision number from the tree above.

Thank you so much @Oriol! Fixed and submitted

Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/17fe4ba9ed5a
Implement slow label on JSON viewer expand all button r=Honza

https://hg.mozilla.org/mozilla-central/rev/17fe4ba9ed5a

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
status-firefox76: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 76

What was the resolution for the bug?

I'm monitoring servers accessing REST that returns me errors with Java stack traces (if any)
Now everything coming collapsed, and still no ExpandAll button.

I need it even if my JSON bigger than 100K.
Or whatever...

I'm on 74.0 now

Thanks,
Roman

(In reply to roman.geraschenko from comment #22)

What was the resolution for the bug?

I'm monitoring servers accessing REST that returns me errors with Java stack traces (if any)
Now everything coming collapsed, and still no ExpandAll button.

I need it even if my JSON bigger than 100K.
Or whatever...

I'm on 74.0 now

Thanks,
Roman

I put the button back, it's now always available. If it's very big JSON the button will change from
[ Show ] to [ Show (slow) ]

(In reply to roman.geraschenko from comment #22)

I'm on 74.0 now

The change is in 76.

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Keywords: regression
QA Whiteboard: [qa-76b-p2]

Is this closed?

Yes

See, the "Status: RESOLVED FIXED" at the top of the page

Honza

You need to log in before you can comment on or make changes to this bug.