1283637 - [10.12] Overlay scrollbar thumbs are wide / fat even when the scrollbar is not hovered

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Markus Stange [:mstange]]

They should be thin, until hovered.

Comment 1

[Markus Stange [:mstange]]

Attachment: Bug 1283637 - Don't specify the 'state' key in the CoreUI options for overlay scrollbars unless the scrollbar is actually hovered.

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

Comment 2

[Markus Stange [:mstange]]

I'll need to test this some more, but I think the idea is right.

Comment 3

[Stephen A Pohl [:spohl] (OOO until 2/27)]

Comment on attachment 8766980 [details]
Bug 1283637 - Don't specify the 'state' key in the CoreUI options for overlay scrollbars unless the scrollbar is actually hovered.

https://reviewboard.mozilla.org/r/61688/#review59522

Just one question below, making sure that there wasn't an oversight during the refactor of RenderWithCoreUILegacy into RenderWithCoreUI.

::: widget/cocoa/nsNativeThemeCocoa.mm:2751
(Diff revision 1)
>            }
>            macRect.size.width -= 4;
>          }
>        }
>        const BOOL isOnTopOfDarkBackground = IsDarkBackground(aFrame);
> -      // Scrollbar thumbs have a too high minimum width when rendered through
> +      NSMutableDictionary* options = [NSMutableDictionary dictionaryWithObjectsAndKeys:

Is it no longer the case that scrollbar thumbs have "a too high minimum width when rendered through NSAppearance on 10.10"? Is this due to the change to |GetWidgetBorder|?

Comment 4

[Markus Stange [:mstange]]

There never was a "too high minimum width" - what was happening was that all scrollbars were rendering as hovered if rendered through NSAppearance, because the "state" key was present in the dict. Now that we no longer set the "state" key for non-hovered scrollbars, we correctly draw them at their thin size even through NSAppearance.

Comment 5

[Stephen A Pohl [:spohl] (OOO until 2/27)]

Ah, makes sense. Thanks!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3bb5c1b4fc5d
Don't specify the 'state' key in the CoreUI options for overlay scrollbars unless the scrollbar is actually hovered. r=spohl

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3bb5c1b4fc5d

Comment 8

[Markus Stange [:mstange]]

Comment on attachment 8766980 [details]
Bug 1283637 - Don't specify the 'state' key in the CoreUI options for overlay scrollbars unless the scrollbar is actually hovered.

Approval Request Comment
[Feature/regressing bug #]: macOS 10.12 Sierra
[User impact if declined]: Scrollbars will look fat
[Describe test coverage new/current, TreeHerder]: none
[Risks and why]: low; just small change to how we ask the system to render scrollbars + tweaking of internal spacing
[String/UUID change made/needed]: none

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8766980 [details]
Bug 1283637 - Don't specify the 'state' key in the CoreUI options for overlay scrollbars unless the scrollbar is actually hovered.

Improve 10.12 support, taking it.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing.

Comment 11

[Markus Stange [:mstange]]

Attachment: patch for beta

Comment 12

[Markus Stange [:mstange]]

Comment on attachment 8777484 [details] [diff] [review]
patch for beta

Approval Request Comment: See comment 8. This was approved for aurora but failed to land before the merge to beta.

Comment 13

[Gerry Chang [:gchang]]

Comment on attachment 8777484 [details] [diff] [review]
patch for beta

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

Per comment #9, improve 10.12 support, take it in 49 beta.

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4e5e1d4c6b04

Comment 15

[Alexandru Simonca [:asimonca], Release Desktop QA]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:48.0) Gecko/20100101 Firefox/48.0

I have reproduced this issue on MacOS Sierra 10.12 Beta 8 (16A313a) using Firefox DevEdition 48.0a2 (id: 20160530004020).

This is VERIFIED FIXED on the latest Nightly (id: 20160831030224) and on the latest Beta (id: 20160829102229) builds.