1047148 - [markup view] Dark theme has overlapping scrollbars when editing outer HTML

Status: VERIFIED FIXED

(DevTools :: Inspector, defect)

Description

[sys.sgx]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:34.0) Gecko/20100101 Firefox/34.0 (Beta/Release)
Build ID: 20140731030206

Steps to reproduce:

There is an issue with the dark theme scrollbars in devtools inspector.
First, even on Windows 7, they are too thin, as in bug 1044258.

Next, if you edit a section in the inspector (ie on bugzilla.mozilla.org) where there should normally be two scrollbars (section editing + inspector for page, as seen in the screenshot above), in the dark theme the two scrollbars overlap, making it practically unusable in its current form.




Expected results:

Scrollbar behavour should be fixed.

Comment 1

[sys.sgx]

Attachment: themes.png

Comment 2

[Brian Grinstead [:bgrins]]

It seems like it's a bug with the floating scrollbars if this is only affecting the dark theme.  Could we get rid of the floating scrollbars functionality but keep the color changes so it still looks OK?  This would solve the issue, but I'm not sure exactly the history of the floating scrollbars and why they are only loaded in the dark theme.

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

I don't have Windows currently, but I don't see a problem on Linux or Mac. Indeed, only a single scrollbar is displayed in both themes, which is what we really want. What is the point of having the inner box scroll independently from the outer box in this case? The markup view tries to provide the illusion of an editor and having 2 scrollbars would be working against that goal. I also don't see the border of the editable box bleed into the scrollbar in other platforms. This looks like a Windows-specific issue.

In general, I don't agree with removing the floating scrollbars. They provide a modern look & feel that is reminiscent of mobile devices, even in platforms where we don't support the platform's modern look & feel, like Linux. As for why it's only enabled in the dark theme, I think it had something to do with us not being able to style it so that it would look good on white background. Paul did that work, so he should remember.

Comment 4

[Brian Grinstead [:bgrins]]

(In reply to Panos Astithas [:past] from comment #3)
> I don't have Windows currently, but I don't see a problem on Linux or Mac.
> Indeed, only a single scrollbar is displayed in both themes, which is what
> we really want. What is the point of having the inner box scroll
> independently from the outer box in this case? The markup view tries to
> provide the illusion of an editor and having 2 scrollbars would be working
> against that goal. I also don't see the border of the editable box bleed
> into the scrollbar in other platforms. This looks like a Windows-specific
> issue.

In this case the inner scrollbox is meant to keep a max height on the edit as html (which could expand into a very tall editor, much taller than the markup view currently is).  The reason is so that you can keep context around the element you are editing and answer questions like "am I editing the right element?", "what's the next sibling", etc.  Where if this expands and becomes a 10000px tall markup view, scrolling down to find the next element would be inconvenient and take you away from your editing caret.

I'll upload a screenshot showing this, and also what I see on OSX which actually seems similar to the screenshot from Win7.

Comment 5

[Brian Grinstead [:bgrins]]

Attachment: scrollbars-dark-osx.png

Screenshot on OSX when editing #outer-wrapper on https://www.mozilla.org/en-US/.

If we can just move the right edge of the editor element in so that it doesn't overlap with the floating scrollbar that should fix the issue.  Not sure how to detect whether they are floating in CSS, maybe we could just move it in 12px or so in both themes and see if that looks weird in the light theme.

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

Ah, so it appears that you are using the 'Edit as HTML' option, whereas I was double-clicking inside a <script> node. I can see the same thing when I use the menu item.

I see that on OS X, where we always use the native floating scrollbars, the scrollbars overlap in the light theme, too. Since the floating scrollbars in other platforms were meant to emulate the OS X/iOS behavior, I wonder if that's a common pattern there. However, just moving the inner scrollbar a bit to the left sounds nice and simple.

Comment 7

[Brian Grinstead [:bgrins]]

Attachment: markupview-floating.png

screenshot with / without floating scrollbars with the patch applied

Comment 8

[Brian Grinstead [:bgrins]]

Attachment: markupview-scroll.patch

Comment 9

[sys.sgx]

Just a note I would like to make: please increase the width of the scrollbars a bit. Having an easier to grab with the mouse scrollbar increases mouse productivity. I also agree with moving the scrollbar a bit in the inside of the edit box.

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8484965 [details] [diff] [review]
markupview-scroll.patch

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

Perfect.

Comment 11

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to sys.sgx from comment #9)
> Just a note I would like to make: please increase the width of the
> scrollbars a bit. Having an easier to grab with the mouse scrollbar
> increases mouse productivity. I also agree with moving the scrollbar a bit
> in the inside of the edit box.

I don't think emulated floating scrollbars can be styled at all, it's the same problem that prevents their use in the light theme. Or at least that used to be the case when they were introduced for Firefox OS. I don't know if that has changed now.

Comment 12

[sys.sgx]

Can you please check if we can style scrollbars?
This would give us uniform style and behaviour in all systems.

Comment 13

[Brian Grinstead [:bgrins]]

(In reply to sys.sgx from comment #12)
> Can you please check if we can style scrollbars?
> This would give us uniform style and behaviour in all systems.

Yes, pinging paul.  I believe that the width can be changed in themes/*/devtools/floating-scrollbars.css - it appears that the size is 10px in windows/linux and 8px is osx.  Paul, what do you think about increasing these widths?

Comment 14

[Brian Grinstead [:bgrins]]

https://tbpl.mozilla.org/?tree=Try&rev=c7ba6fc0dbbb

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/ed286c75d42f

Comment 16

[Paul Rouget [:paul]]

(In reply to Brian Grinstead [:bgrins] from comment #13)
> (In reply to sys.sgx from comment #12)
> > Can you please check if we can style scrollbars?
> > This would give us uniform style and behaviour in all systems.
> 
> Yes, pinging paul.  I believe that the width can be changed in
> themes/*/devtools/floating-scrollbars.css - it appears that the size is 10px
> in windows/linux and 8px is osx.  Paul, what do you think about increasing
> these widths?

Sure.

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ed286c75d42f