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

VERIFIED FIXED in Firefox 35

Status

()

Firefox
Developer Tools: Inspector
VERIFIED FIXED
4 years ago
2 years ago

People

(Reporter: sys.sgx, Assigned: bgrins)

Tracking

34 Branch
Firefox 35
All
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [bugday-20140917])

Attachments

(4 attachments)

(Reporter)

Description

4 years ago
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.
(Reporter)

Updated

4 years ago
Hardware: x86 → All
(Reporter)

Comment 1

4 years ago
Created attachment 8465882 [details]
themes.png
(Reporter)

Updated

4 years ago
Component: Untriaged → Developer Tools: Inspector

Updated

4 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 2

4 years ago
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.
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.
(Assignee)

Comment 4

4 years ago
(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.
(Assignee)

Comment 5

4 years ago
Created attachment 8484937 [details]
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.
(Assignee)

Updated

4 years ago
Summary: DevTools: Theme dark has an issue with scrollbars, they overlap → [markup view] Dark theme has overlapping scrollbars when editing outer HTML
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.
(Assignee)

Comment 7

4 years ago
Created attachment 8484964 [details]
markupview-floating.png

screenshot with / without floating scrollbars with the patch applied
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
(Assignee)

Comment 8

4 years ago
Created attachment 8484965 [details] [diff] [review]
markupview-scroll.patch
Attachment #8484965 - Flags: review?(past)
(Reporter)

Comment 9

4 years ago
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 on attachment 8484965 [details] [diff] [review]
markupview-scroll.patch

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

Perfect.
Attachment #8484965 - Flags: review?(past) → review+
(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.
(Reporter)

Comment 12

4 years ago
Can you please check if we can style scrollbars?
This would give us uniform style and behaviour in all systems.
(Assignee)

Comment 13

4 years ago
(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?
Flags: needinfo?(paul)
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/ed286c75d42f
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]

Comment 16

4 years ago
(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.
Flags: needinfo?(paul)
(Assignee)

Updated

4 years ago
Blocks: 1064248
https://hg.mozilla.org/mozilla-central/rev/ed286c75d42f
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 35
Status: RESOLVED → VERIFIED
Whiteboard: [bugday-20140917]

Updated

2 years ago
See Also: → bug 1197229
You need to log in before you can comment on or make changes to this bug.