Closed Bug 1380828 Opened 5 years ago Closed 4 years ago

Hide "Pretty Print" button for invalid JSON

Categories

(DevTools :: JSON Viewer, defect, P3)

defect

Tracking

(firefox57 verified)

VERIFIED FIXED
Firefox 57
Tracking Status
firefox57 --- verified

People

(Reporter: Oriol, Assigned: abhinav.koppula, Mentored)

Details

(Keywords: good-first-bug)

Attachments

(1 file)

1. Load data:application/json,[
2. Go to "Raw data" tab
3. Click "Pretty Print"

Result: the panel shows "SyntaxError: JSON.parse: unexpected end of data at line 1 column 2 of the JSON data"

If the JSON is not valid, the button should be hidden.
Mentor: odvarko
Keywords: good-first-bug
Priority: -- → P3
Hi Honza,
I've taken a stab at this issue and have sent a review-request.
Please let me know if any changes need to be done.
Comment on attachment 8908731 [details]
Bug 1380828 - Hide 'Pretty Print' button in 'Raw Data' tab if json is invalid.

https://reviewboard.mozilla.org/r/180362/#review185528

After filing the issue I started wondering whether it would be better UX to hide the button or display it in disabled state.

::: devtools/client/jsonview/components/text-panel.js:24
(Diff revision 1)
>     */
>    let TextPanel = createClass({
>      displayName: "TextPanel",
>  
>      propTypes: {
> +      isValidJson: PropTypes.boolean,

It should be `PropTypes.bool`

::: devtools/client/jsonview/test/browser_jsonview_bug_1380828.js:14
(Diff revision 1)
> +
> +add_task(function* () {
> +  info("Test 'Pretty Print' button disappears on parsing invalid JSON");
> +
> +  let count = yield testPrettyPrintButton(INVALID_JSON_URL);
> +  ok(count == 0, "There must be no pretty-print button for invalid json");

Using `ok` for comparisons is annoying when the check fails. Better use `is(count, 0, descr)`.
Hi Oriol,
Thanks for the review. 
I have made the changes you mentioned.
Let me know if you want the button to be disabled. will make changes accordingly.
Comment on attachment 8908731 [details]
Bug 1380828 - Hide 'Pretty Print' button in 'Raw Data' tab if json is invalid.

https://reviewboard.mozilla.org/r/180362/#review185964

Looks good to me, thanks!

R+ assuming try is green

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

Honza
Attachment #8908731 - Flags: review?(odvarko) → review+
The failure in the try is caused by bug 1395759 (I am working on that one)

Honza
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ff2b91e284d2
Hide 'Pretty Print' button in 'Raw Data' tab if json is invalid. r=Honza
Assignee: nobody → abhinav.koppula
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/ff2b91e284d2
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
I have reproduced this bug with Nightly 57.0a1 (2017-09-15) on Windows 7, 64 Bit!

This bug's fix is Verified with latest Beta!

Build   ID    20171026211016
User Agent    Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
QA Whiteboard: [bugday-20171101]
Verifying per comment 10.
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.