1471162 - Get rid of fake floating scrollbars on OSX

Status: RESOLVED FIXED

(DevTools :: Framework, enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

+++ This bug was initially created as a clone of Bug #1464785 +++

Stop using fake floating scrollbars on OSX when Bug 1464722 lands.

Comment 1

[Julian Descottes [:jdescottes]]

I tried to implement this since Bug 1464722 landed, but interestingly, the new style applies to all scrollbars except for 2:
- main scrollbar in options panel
- main scrollbar in markup view

For these two the scrollbars keep the default color. So far I have no idea where this different behavior comes from. 

Also when there are both a horizontal and a vertical scrollbar with custom colors, the corner between the two is transparent.

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: transparent-corner.png

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Julian Descottes [:jdescottes][:julian] from comment #1)
> I tried to implement this since Bug 1464722 landed, but interestingly, the
> new style applies to all scrollbars except for 2:
> - main scrollbar in options panel
> - main scrollbar in markup view
> 
> For these two the scrollbars keep the default color. So far I have no idea
> where this different behavior comes from. 

If you can upload a wip patch and point me to the place, I can have a look.

> Also when there are both a horizontal and a vertical scrollbar with custom
> colors, the corner between the two is transparent.

Are you using webrender? If so, that's probably bug 1471766.

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Julian Descottes [:jdescottes][:julian] from comment #1)
> I tried to implement this since Bug 1464722 landed, but interestingly, the
> new style applies to all scrollbars except for 2:
> - main scrollbar in options panel
> - main scrollbar in markup view

Actually, it sounds like viewport scrollbars are unstyled again :/

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1471162 - Stop using fake floating scrollbar in OSX dark theme;

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

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

Ah... Ok, I know why... I'm silly :/

Comment 7

[Julian Descottes [:jdescottes]]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #6)
> Ah... Ok, I know why... I'm silly :/

Thanks for having a look so quickly! Attached a patch, but looks like you don't need it for the moment :)

Comment 8

[Julian Descottes [:jdescottes]]

Comment on attachment 8988460 [details]
Bug 1471162 - Stop using fake floating scrollbar in OSX dark theme;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/253738/diff/1-2/

Comment 9

[Julian Descottes [:jdescottes]]

All scrollbars are now properly styled! However I still have the "transparent" (or blank sometimes) corner issue.
The easiest way to reproduce it is, with my patch:
- go to about:newtab
- open devtools 
- switch to dark theme
- open Style Editor panel in devtools

It should normally be showing contentSearchUI.css, which has pretty long lines, and therefore makes it easy to trigger both horizontal and vertical scrollbars.

I checked I have all the webrender preferences turned off.

Comment 10

[Xidorn Quan [:xidorn] UTC+10]

I used Browser Toolbox to checked that position, and it turns out that the scrollbars are not provided natively at all. They are controlled by CodeMirror.

It's not clear to me how do you make CodeMirror use <scrollbar> element for the scrollbars, but apparently the corner is just a div.CodeMirror-scrollbar-filler and doesn't use <scrollcorner> which is what is supposed to be styled.

You can probably either put an xul <scrollcorner> into the element, or apply "-moz-appearance: scrollcorner" to it to make it styled.

Comment 11

[Julian Descottes [:jdescottes]]

You are right, sorry about that! I have no idea how this is setup either, but we do have custom styles already for this.

  - debugger's css: https://searchfox.org/mozilla-central/rev/d2966246905102b36ef5221b0e3cbccf7ea15a86/devtools/client/debugger/new/dist/debugger.css#251-253

  :root.theme-dark .CodeMirror-scrollbar-filler {
    background: transparent;
  }

  which translates into the transparent corner I was talking about for the debugger. I believe this one can be completely removed, CodeMirror seems to take care of hiding the corner now when real floating scrollbars are used.

  - devtools' codemirror.css: https://searchfox.org/mozilla-central/rev/d2966246905102b36ef5221b0e3cbccf7ea15a86/devtools/client/sourceeditor/codemirror/lib/codemirror.css#20-22

  .CodeMirror-scrollbar-filler, .CodeMirror-gutter-filler {
    background-color: white; /* The little square between H and V scrollbars */
  }

  which is responsible for the white corner in the styleeditor. Here we can apply the fix you suggested and force -moz-appearance: scrollcorner (or rather do it in the mozilla.css file containing our codemirror overrides)

Comment 12

[Julian Descottes [:jdescottes]]

Attachment: Bug 1471162 - Style codemirror scrollbar corners correctly in dark-theme;

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

Comment 13

[Julian Descottes [:jdescottes]]

Comment on attachment 8988460 [details]
Bug 1471162 - Stop using fake floating scrollbar in OSX dark theme;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/253738/diff/2-3/

Comment 14

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8988728 [details]
Bug 1471162 - Style codemirror scrollbar corners correctly in dark-theme;

https://reviewboard.mozilla.org/r/253936/#review260662

Comment 15

[Patrick Brosset <:pbro>]

Comment on attachment 8988460 [details]
Bug 1471162 - Stop using fake floating scrollbar in OSX dark theme;

https://reviewboard.mozilla.org/r/253738/#review260664

Tested on macOS just now. Seems to work fine (I made sure layout.css.scrollbar-colors.enabled was false, and I forced actual scrollbar in system preferences).

::: devtools/client/shared/theme-switching.js:93
(Diff revision 3)
>        devtoolsStyleSheets.get(newThemeDef).push(styleSheet);
>        loadEvents.push(loadPromise);
>      }
>  
> -    if (os !== "win") {
> -      // Windows always uses native scrollbars, other platforms still use custom floating
> +    if (os !== "win" && os !== "mac") {
> +      // Windows & Mac always use native scrollbars, Linux still use custom floating

nit: s/use/uses

Comment 16

[Patrick Brosset <:pbro>]

Oh, and I tested with both patches applied too, so part 2 seems fine too.

Comment 17

[Julian Descottes [:jdescottes]]

Comment on attachment 8988460 [details]
Bug 1471162 - Stop using fake floating scrollbar in OSX dark theme;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/253738/diff/3-4/

Comment 18

[Julian Descottes [:jdescottes]]

Comment on attachment 8988728 [details]
Bug 1471162 - Style codemirror scrollbar corners correctly in dark-theme;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/253936/diff/1-2/

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/72a5ab3371fa
Stop using fake floating scrollbar in OSX dark theme;r=pbro
https://hg.mozilla.org/integration/autoland/rev/9839a5ebf84b
Style codemirror scrollbar corners correctly in dark-theme;r=gl

Comment 20

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/72a5ab3371fa
https://hg.mozilla.org/mozilla-central/rev/9839a5ebf84b