1412607 - Response panel has two scrollbars for JSON requests

Status: NEW

(DevTools :: Netmonitor, defect, P3)

Description

[Brandon Cheng]

Attachment: attachment.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171026211016

Steps to reproduce:

1. Open the developer tools and go to the "Network" tab.
2. Load a URL that returns a JSON payload. Ex: http://api.duckduckgo.com/?q=valley+forge+national+park&format=json&pretty=1
3. View the new record in the network tab.
4. Scroll past the JSON representation and into the second section below.


Actual results:

Scrolling through the JSON "Response payload" shows two scrollbars. On macOS, if a trackpad is used, the second scrollbar won't be shown until scrolling on the "Response payload" label. In some cases, the end of the raw response payload may be unreachable.


Expected results:

Only a single scroll context should exist on the response panel. Reaching the end of the response payload should be possible. The horizontal scrollbar on the response payload should also be visible. (It is currently hidden due to the offset caused by the two "JSON" and "Response payload" labels.)

This bug was found while working on bug 1353319. Honza suggested using an accordion to fix this issue.

https://bugzilla.mozilla.org/show_bug.cgi?id=1353319#c25

HTML requests don't show this issue since a CSS class was added to handle that specific case in bug 1353319.

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks for the report, I can reproduce the issue.

Honza

Comment 2

[Brandon Cheng]

Attachment: Firefox 59 change

I just want to add a note that the original issue description is no long applicable. This following commit added "overflow-x: auto;" to the relevant CSS class: https://hg.mozilla.org/mozilla-central/rev/c5c1fe6738f8

This prevents the "Response payload" panel from being scrolled to when the "JSON" panel is expanded. "overflow-x: auto" causes "overflow-y" to change from "visible" to "auto", which in turn causes the panel to become 0px in height.

Comment 3

[Brandon Cheng]

Honza,

I have a question if you have time to answer it. You suggested switching the entire response panel to use an accordion, but I'm not sure if that will fix this issue. Right now we have two scroll contexts: one from the container and a second one from CodeMirror. I think the only way to fix the double scrolling is to remove "overflow: scroll" from CodeMirror, thus eliminating the second scroll context.

I see why "overflow: scroll" is there. It's a hack to allow the line numbers to be sticky while horizontally scrolling.

Do you agree with that assessment?

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Brandon Cheng from comment #3)


> Honza,
> 
> I have a question if you have time to answer it. You suggested switching the
> entire response panel to use an accordion, but I'm not sure if that will fix
> this issue.
Yes, accordion component is used also in the Debugger panel and it would make
the UI/UX more consistent across DevTools panels.

Agree, it wouldn't itself fix the issue, but we could use the refactoring to
also clean up scrollbars CSS (I recall multiple issues with scrollbars
in the past).

> Right now we have two scroll contexts: one from the container
> and a second one from CodeMirror. I think the only way to fix the double
> scrolling is to remove "overflow: scroll" from CodeMirror, thus eliminating
> the second scroll context.
> 
> I see why "overflow: scroll" is there. It's a hack to allow the line numbers
> to be sticky while horizontally scrolling.
> 
> Do you agree with that assessment?
Yes, your suggestion make sense. Are you interested in fixing this?
I can test the patch if it's attached.

Thanks!
Honza