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

Status: RESOLVED FIXED

Description

[Nicolas Chevobbe [:nchevobbe]]

Steps to reproduce:

1. Open the netmonitor panel
2. Open the split console
3. Evaluate `fetch("https://api.github.com")`
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.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: response_overflow.png

Comment 2

[Abhinav Koppula]

Attachment: bz-1394323-json-panel.patch

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.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Screen Shot 2017-09-04 at 09.00.01.png

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.

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8904055 [details] [diff] [review]
bz-1394323-json-panel.patch

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

r+-able with a test :)

Comment 5

[Abhinav Koppula]

Attachment: bz-1394323-json-panel-test.patch

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.

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8904735 [details] [diff] [review]
bz-1394323-json-panel-test.patch

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 : http://searchfox.org/mozilla-central/source/devtools/client/themes/webconsole.css#894-896 , 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.

Comment 7

[Abhinav Koppula]

Attachment: Bug 1394323 - Remove overflow css rule for 'response-panel' as it prevents scrolling in Console 'Response' tab.

Review commit: https://reviewboard.mozilla.org/r/180408/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180408/

Comment 8

[Abhinav Koppula]

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 9

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8908790 [details]
Bug 1394323 - Remove overflow css rule for 'response-panel' as it prevents scrolling in Console 'Response' tab.

https://reviewboard.mozilla.org/r/180408/#review185738

Comment 10

[Sebastian Hengst [:aryx] (needinfo on intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/4ef69bbd6816