Closed Bug 1471162 Opened 4 years ago Closed 4 years ago

Get rid of fake floating scrollbars on OSX

Categories

(DevTools :: Framework, enhancement, P3)

Unspecified
Windows
enhancement

Tracking

(firefox63 fixed)

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

Attachments

(3 files)

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

Stop using fake floating scrollbars on OSX when Bug 1464722 lands.
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.
Attached image transparent-corner.png
(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.
(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 :/
Ah... Ok, I know why... I'm silly :/
Depends on: 1471866
(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 :)
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
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.
Flags: needinfo?(xidorn+moz)
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.
Flags: needinfo?(xidorn+moz)
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 on attachment 8988728 [details]
Bug 1471162 - Style codemirror scrollbar corners correctly in dark-theme;

https://reviewboard.mozilla.org/r/253936/#review260662
Attachment #8988728 - Flags: review?(gl) → review+
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
Attachment #8988460 - Flags: review?(pbrosset) → review+
Oh, and I tested with both patches applied too, so part 2 seems fine too.
Pushed by jdescottes@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/72a5ab3371fa
https://hg.mozilla.org/mozilla-central/rev/9839a5ebf84b
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.