874570 - Making small scrollbars actually look smaller than regular ones

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, enhancement)

Comment 1

[André Reinald]

Attachment: Split the original patch from bug 873010

This patch makes the small scrollbars actually smaller than regular ones.

Comment 2

[André Reinald]

Attachment: Split the original patch from bug 873010

Modified previous patch to avoid duplicate calls to GetParentScrollbarFrame.

Comment 3

[Markus Stange [:mstange]]

Comment on attachment 752453 [details] [diff] [review]
Split the original patch from bug 873010

I shouldn't review this because I wrote most of it myself.
And, looking at this again, I think it would be better to implement small scrollbars by actually making the scrollbar frame smaller, and not only the drawing. I'll write a new patch.

Comment 4

[André Reinald]

(In reply to Markus Stange from comment #3)
> 
> I think it would be better to implement small
> scrollbars by actually making the scrollbar frame smaller, and not only the
> drawing. I'll write a new patch.

That would be nice. I'm currently looking at bug 873012 whose fix will likely affect the same source file. Maybe the patch you want to write will give me hints.

Comment 5

[Alex Keybl [:akeybl]]

tracking, but wouldn't block on this.

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

Too late for FF23, wontfixing.

Comment 7

[Stephen A Pohl [:spohl]]

Attachment: Patch

I think this does all we need, but adding mstange as feedback? to make sure. The scrollbars appear correctly (smaller) in the list boxes in bugzilla, for example. This is the page I used for my testing:
https://bugzilla.mozilla.org/enter_bug.cgi?product=Firefox

Note that when hovered, we expect 'small' scrollbars to grow by the same amount as regular ones.

Comment 8

[Markus Stange [:mstange]]

Comment on attachment 784456 [details] [diff] [review]
Patch

Oohh, this looks nice and simple.

Comment 9

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 784456 [details] [diff] [review]
Patch

Looks fine to me.

Comment 10

[Stephen A Pohl [:spohl]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/82adb6106f7a

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/82adb6106f7a

Comment 12

[Stephen A Pohl [:spohl]]

Comment on attachment 784456 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 636564
User impact if declined: Overlay scrollbars that are supposed to appear smaller than regular ones wouldn't appear smaller.
Testing completed (on m-c, etc.): Has been on m-c for 5 days.
Risk to taking this patch (and alternatives if risky): very low
String or IDL/UUID changes made by this patch: none

Comment 13

[Stephen A Pohl [:spohl]]

Adding Jim to check whether this is an issue on Windows too. I'm not sure about the sizing of scrollbars there. We can probably address in a separate bug if it is a problem there.

Comment 14

[Alex Keybl [:akeybl]]

Comment on attachment 784456 [details] [diff] [review]
Patch

Do we want to get this into a beta as well?

Comment 15

[Stephen A Pohl [:spohl]]

Attachment: Patch

Right, I should have asked for approval for both aurora and beta. I took this opportunity to update the patch to what was landed on m-c. Here is the approval request comment again for completeness:

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 636564
User impact if declined: Overlay scrollbars that are supposed to appear smaller than regular ones wouldn't appear smaller.
Testing completed (on m-c, etc.): Has been on m-c for 5 days.
Risk to taking this patch (and alternatives if risky): very low
String or IDL/UUID changes made by this patch: none

Comment 16

[bhavana bajaj [:bajaj]]

Comment on attachment 787086 [details] [diff] [review]
Patch

Approving on beta.Patch is already landed on FX25 , hence clearing the aurora nomination.

Comment 17

[Stephen A Pohl [:spohl]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a73c5aef86cf

Comment 18

[u279076]

Stephen, can you please outline the circumstances where small scrollbars exist so QA can verify this is fixed?

Comment 19

[Stephen A Pohl [:spohl]]

Sure:
1. In System Preferences > General, make sure that "When scrolling" is selected for "Show scroll bars:".
2. Go to https://bugzilla.mozilla.org/enter_bug.cgi?product=Firefox

The "component" and "version" boxes should show small(er) scrollbars.

Comment 20

[Paul Silaghi, QA [:pauly]]

Now, after the fix, the scrollbars are thinner.
Verified fixed FF 24, 25b3 Mac OS X 10.8.4