Closed Bug 874570 Opened 7 years ago Closed 6 years ago

Making small scrollbars actually look smaller than regular ones

Categories

(Core :: Widget: Cocoa, enhancement)

All
macOS
enhancement
Not set

Tracking

()

VERIFIED FIXED
mozilla25
Tracking Status
firefox21 --- unaffected
firefox22 --- unaffected
firefox23 + wontfix
firefox24 --- verified
firefox25 --- verified
firefox-esr17 --- unaffected

People

(Reporter: areinald.bug, Assigned: spohl)

Details

(Whiteboard: [lion-scrollbars=])

Attachments

(1 file, 3 obsolete files)

No description provided.
Assignee: nobody → areinald
Whiteboard: [lion-scrollbars+]
This patch makes the small scrollbars actually smaller than regular ones.
Attachment #752329 - Flags: review?(mstange)
Modified previous patch to avoid duplicate calls to GetParentScrollbarFrame.
Attachment #752329 - Attachment is obsolete: true
Attachment #752329 - Flags: review?(mstange)
Attachment #752453 - Flags: review?(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.
Attachment #752453 - Flags: review?(mstange)
(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.
tracking, but wouldn't block on this.
Whiteboard: [lion-scrollbars+] → [lion-scrollbars=]
Too late for FF23, wontfixing.
Attached patch Patch (obsolete) — Splinter Review
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.
Assignee: mstange → spohl.mozilla.bugs
Attachment #752453 - Attachment is obsolete: true
Attachment #784456 - Flags: review?(smichaud)
Attachment #784456 - Flags: feedback?(mstange)
Comment on attachment 784456 [details] [diff] [review]
Patch

Oohh, this looks nice and simple.
Attachment #784456 - Flags: feedback?(mstange) → feedback+
Comment on attachment 784456 [details] [diff] [review]
Patch

Looks fine to me.
Attachment #784456 - Flags: review?(smichaud) → review+
https://hg.mozilla.org/mozilla-central/rev/82adb6106f7a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
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
Attachment #784456 - Flags: approval-mozilla-aurora?
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 on attachment 784456 [details] [diff] [review]
Patch

Do we want to get this into a beta as well?
Attachment #784456 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attached patch PatchSplinter Review
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
Attachment #787086 - Flags: review+
Attachment #787086 - Flags: feedback+
Attachment #787086 - Flags: approval-mozilla-beta?
Attachment #787086 - Flags: approval-mozilla-aurora?
Attachment #784456 - Attachment is obsolete: true
Comment on attachment 787086 [details] [diff] [review]
Patch

Approving on beta.Patch is already landed on FX25 , hence clearing the aurora nomination.
Attachment #787086 - Flags: approval-mozilla-beta?
Attachment #787086 - Flags: approval-mozilla-beta+
Attachment #787086 - Flags: approval-mozilla-aurora?
Stephen, can you please outline the circumstances where small scrollbars exist so QA can verify this is fixed?
Flags: needinfo?(spohl.mozilla.bugs)
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.
Flags: needinfo?(spohl.mozilla.bugs)
Keywords: verifyme
Now, after the fix, the scrollbars are thinner.
Verified fixed FF 24, 25b3 Mac OS X 10.8.4
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.