Closed Bug 1394323 Opened 5 years ago Closed 5 years ago

In the Response tab, the "JSON" container should have an overflow, like the "Response payload" one


(DevTools :: Netmonitor, defect, P3)



(firefox57 fixed)

Firefox 57
Tracking Status
firefox57 --- fixed


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


(Keywords: good-first-bug)


(3 files, 2 obsolete files)

Steps to reproduce:

1. Open the netmonitor panel
2. Open the split console
3. Evaluate `fetch("")`
4. In the netmonitor panel, select the github api call
5. Select the response tab
6. Reduce the panel size until you see a scrollbar 

Expected results: 
The JSON panel has an overflow, like the "Response payload" one

Actual results:
The overflow is on the parent container (see attached screenshot). With the Browser toolbox, it seems like the overflow is on .tree-container .treeTable

This is no big deal in the netmonitor, but when the component is embedded (like in the console), the response can't be scrolled.
Mentor: odvarko
Keywords: good-first-bug
Priority: -- → P3
Attached patch bz-1394323-json-panel.patch (obsolete) — Splinter Review
Hi Nicolas,
I have attached a fix for the issue where the "Json view" in the Response tab of Console panel was not scrollable.

Please let me know if I was supposed to approach this issue in a different way.
Attachment #8904055 - Flags: review?(nchevobbe)
This is fixing a part of the problem: now we can scroll the response indeed.
But, we do have this double scrollbar which is a bit weird.
Since it might be tricky to remove the overflow of the Response payload (it's a textarea if I'm correct, so we can reliably expand it to have the size of the content), we should add an overflow on the  JSON panel body only.

This is not an easy job due to the way the component is structured (everything is a table row). I'd say that we can land this patch and resolve the other issue in a follow-up bug later.

If you want to do this in this bug, it's also fine, do as you wish.

But before landing the patch, it needs testing. We should have a test that make sure that the panel body is scrollable.
Attachment #8904149 - Flags: review+
Comment on attachment 8904055 [details] [diff] [review]

Review of attachment 8904055 [details] [diff] [review]:

r+-able with a test :)
Attachment #8904055 - Flags: review?(nchevobbe) → review-
Attached patch bz-1394323-json-panel-test.patch (obsolete) — Splinter Review
Hi Nicolas,
I have updated the patch and have added the test which checks for overflow-y.
I feel it would be better to land this patch and then take up a follow-up bug which involves the restructuring.
Attachment #8904055 - Attachment is obsolete: true
Attachment #8904735 - Flags: review?(nchevobbe)
Comment on attachment 8904735 [details] [diff] [review]

Sorry it took me so long to do this review.
It turns out that we do turn off the overflow on the console on purpose here : , because it shows 2 scrollbars.
Unfortunately, it prevent scrolling.
I'd say that we should be fine removing this specific rule in the console, and file a bug to better handle this double scrollbar issue.
Attachment #8904735 - Flags: review?(nchevobbe) → review-
Attachment #8904735 - Attachment is obsolete: true
Hi Nicolas,
I have submitted a mozreview-request as per your comments.
Also, please let me know when you file a new bug for handling the double scrollbar issue.
Comment on attachment 8908790 [details]
Bug 1394323 - Remove overflow css rule for 'response-panel' as it prevents scrolling in Console 'Response' tab.
Attachment #8908790 - Flags: review?(nchevobbe) → review+
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Assignee: nobody → abhinav.koppula
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.