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
Categories
(DevTools :: Netmonitor, defect, P3)
DevTools
Netmonitor
Tracking
(firefox57 fixed)
RESOLVED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: nchevobbe, Assigned: abhinav.koppula, Mentored)
Details
(Keywords: good-first-bug)
Attachments
(3 files, 2 obsolete files)
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.
Reporter | ||
Comment 1•5 years ago
|
||
Updated•5 years ago
|
Assignee | ||
Comment 2•5 years ago
|
||
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)
Reporter | ||
Comment 3•5 years ago
|
||
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+
Reporter | ||
Comment 4•5 years ago
|
||
Comment on attachment 8904055 [details] [diff] [review] bz-1394323-json-panel.patch Review of attachment 8904055 [details] [diff] [review]: ----------------------------------------------------------------- r+-able with a test :)
Attachment #8904055 -
Flags: review?(nchevobbe) → review-
Assignee | ||
Comment 5•5 years ago
|
||
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)
Reporter | ||
Comment 6•5 years ago
|
||
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.
Attachment #8904735 -
Flags: review?(nchevobbe) → review-
Comment hidden (mozreview-request) |
Assignee | ||
Updated•5 years ago
|
Attachment #8904735 -
Attachment is obsolete: true
Assignee | ||
Comment 8•5 years ago
|
||
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.
Reporter | ||
Comment 9•5 years ago
|
||
mozreview-review |
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
Attachment #8908790 -
Flags: review?(nchevobbe) → review+
![]() |
||
Comment 10•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4ef69bbd6816
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Reporter | ||
Updated•5 years ago
|
Assignee: nobody → abhinav.koppula
Updated•4 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•