Closed
Bug 865806
Opened 11 years ago
Closed 11 years ago
[10.7] Make lion-style scrollbars brighter on dark background
Categories
(Core :: Widget: Cocoa, defect)
Tracking
()
VERIFIED
FIXED
mozilla24
Tracking | Status | |
---|---|---|
firefox21 | --- | unaffected |
firefox22 | --- | unaffected |
firefox23 | + | verified |
firefox24 | --- | verified |
firefox-esr17 | --- | unaffected |
People
(Reporter: spohl, Assigned: ehsan.akhgari)
References
(Depends on 1 open bug, )
Details
(Whiteboard: [lion-scrollbars+])
Attachments
(1 file, 2 obsolete files)
6.33 KB,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
Broken out of bug 636564. The lion-style scrollbars need to be brighter on dark background, for example on: http://www.klack.de/fernsehprogramm/2015-im-tv/0/free.html
Comment 1•11 years ago
|
||
Swap the gutter and indicator colors?
Reporter | ||
Comment 2•11 years ago
|
||
The complexity here is to detect dark backgrounds. See: http://mxr.mozilla.org/mozilla-central/source/widget/cocoa/nsNativeThemeCocoa.mm#2310 http://mxr.mozilla.org/mozilla-central/source/widget/cocoa/nsNativeThemeCocoa.mm#2345
Comment 3•11 years ago
|
||
WebKit people are defining it by the "plain" css background color on or inherited by the scrollable element: if the sum of the RGB values is under 384, it's dark. (So yes, a solid black image or black-to-black CSS gradient wouldn't count, but most web developers specify a backup background color in case an image doesn't load immediately or gradients aren't supported.) IMHO, this is worth implementing, if only as a first step toward a more fail-proof solution.
Assignee | ||
Comment 5•11 years ago
|
||
Assignee | ||
Comment 6•11 years ago
|
||
Comment on attachment 751265 [details] [diff] [review] Patch (v1) Hmm actually this heuristic is wrong, as aFrame is the button box frame for the scrollbar thumb!
Attachment #751265 -
Flags: review?(roc)
Assignee | ||
Comment 7•11 years ago
|
||
This seems to work fine.
Attachment #751265 -
Attachment is obsolete: true
Attachment #751354 -
Flags: review?(roc)
Attachment #751354 -
Flags: review?(roc) → review+
Assignee | ||
Comment 8•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=e0f37c1f4c7e
Assignee | ||
Comment 9•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b097bd9fd24c
Assignee | ||
Comment 10•11 years ago
|
||
Backed out because of Windows build bustage: https://hg.mozilla.org/integration/mozilla-inbound/rev/70898c96cceb
Assignee | ||
Comment 11•11 years ago
|
||
Relanded with clobber: https://hg.mozilla.org/integration/mozilla-inbound/rev/8aef76919ec0
Updated•11 years ago
|
Comment 12•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8aef76919ec0
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Comment 13•11 years ago
|
||
Comment on attachment 751354 [details] [diff] [review] Patch (v1) Review of attachment 751354 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/xpwidgets/nsNativeTheme.cpp @@ +691,5 @@ > + nsStyleContext* bgSC; > + if (nsCSSRendering::FindBackground(frame, &bgSC)) { > + nscolor bgColor = bgSC->StyleBackground()->mBackgroundColor; > + // Consider the background color dark if the sum of the r, g and b values is > + // less than 384 in a semi-transparent docement. This heuristic matches what Good ol' docements. :)
Updated•11 years ago
|
Whiteboard: [lion-scrollbars+]
Updated•11 years ago
|
status-firefox21:
--- → unaffected
status-firefox22:
--- → unaffected
status-firefox23:
--- → affected
status-firefox24:
--- → fixed
status-firefox-esr17:
--- → unaffected
tracking-firefox23:
--- → ?
Updated•11 years ago
|
Assignee | ||
Comment 14•11 years ago
|
||
Rob, do you want me to nominate this for Aurora?
Flags: needinfo?(robert.bugzilla)
Assignee | ||
Comment 15•11 years ago
|
||
(In reply to Jared Wein [:jaws] from comment #13) > Comment on attachment 751354 [details] [diff] [review] > Patch (v1) > > Review of attachment 751354 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: widget/xpwidgets/nsNativeTheme.cpp > @@ +691,5 @@ > > + nsStyleContext* bgSC; > > + if (nsCSSRendering::FindBackground(frame, &bgSC)) { > > + nscolor bgColor = bgSC->StyleBackground()->mBackgroundColor; > > + // Consider the background color dark if the sum of the r, g and b values is > > + // less than 384 in a semi-transparent docement. This heuristic matches what > > Good ol' docements. :) Hehe, nice catch! Fixed in a follow-up: https://hg.mozilla.org/mozilla-central/rev/f9dc5e76d66b
Comment 16•11 years ago
|
||
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #14) > Rob, do you want me to nominate this for Aurora? Yes please and thank you!
Flags: needinfo?(robert.bugzilla)
Assignee | ||
Comment 17•11 years ago
|
||
Comment on attachment 751354 [details] [diff] [review] Patch (v1) [Approval Request Comment] Bug caused by (feature/regressing bug #): Lion scrollbars. User impact if declined: See the bug report? [1] Testing completed (on m-c, etc.): m-c, note that bug 874391 was also landed which fixes a crash in this patch, and I'll roll that in this patch when landing on Aurora. Risk to taking this patch (and alternatives if risky): Should not be very risky. String or IDL/UUID changes made by this patch: None. [1] <rant> the entries on this form are really really not appropriate for most bugs, as most of these questions have very obvious answers for most situations. I'm sometimes tempted to just clear this questionnaire where it doesn't make sense. :/ </rant>
Attachment #751354 -
Flags: approval-mozilla-aurora?
Comment 18•11 years ago
|
||
Comment on attachment 751354 [details] [diff] [review] Patch (v1) I hear your frustration but we (relman) get a lot of value from the form responses in triage because even though the questions/answers may seem obvious to you when you are familiar with a bug's history & origin, we are very aided by having a rollup of the info in a single comment. Also some of the questions are intended to prompt/remind the nominator of things they might overlook like UUID/string changes. That being said, if you have proposals for ways to request the info that would feel less aggravating please do reach out and propose them. We could certainly change the look/feel/length of the form to improve the reception.
Attachment #751354 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•11 years ago
|
Comment 20•11 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM][Out May 23-27] from comment #19) > https://hg.mozilla.org/releases/mozilla-aurora/rev/59f9f7a8bd8e I backed this out of aurora because it landed without the crashfix from bug 874391. I would have preferred to just land that fix, but it didn't have a+ yet, and when I noticed this, no approvers were around and I was told on IRC this cset ought to be backed out as approval should have been requested on a landable patch instead of this one. (Please don't shoot the messenger, I'm just trying to make sure we don't get really crashy aurora builds.)
Comment 21•11 years ago
|
||
Carrying over earlier r+'s from roc, pretty much the same approval request: [Approval Request Comment] Bug caused by (feature/regressing bug #): Lion scrollbars. User impact if declined: scrollbars are neigh-invisible on dark backgrounds Testing completed (on m-c, etc.): m-c, note that bug 874391 was also landed which fixes a crash in the original patch. This patch contains both the original and the crashfix. Risk to taking this patch (and alternatives if risky): Should not be very risky. String or IDL/UUID changes made by this patch: None.
Attachment #751354 -
Attachment is obsolete: true
Attachment #753126 -
Flags: review+
Attachment #753126 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 22•11 years ago
|
||
(In reply to lsblakk@mozilla.com [:lsblakk] from comment #18) > Comment on attachment 751354 [details] [diff] [review] > Patch (v1) > > I hear your frustration but we (relman) get a lot of value from the form > responses in triage because even though the questions/answers may seem > obvious to you when you are familiar with a bug's history & origin, we are > very aided by having a rollup of the info in a single comment. Also some of > the questions are intended to prompt/remind the nominator of things they > might overlook like UUID/string changes. That being said, if you have > proposals for ways to request the info that would feel less aggravating > please do reach out and propose them. We could certainly change the > look/feel/length of the form to improve the reception. Yeah, I totally understand how this is helpful to you guys, and I did not mean to be dismissive of that fact. However, I think it should be OK to drop the questions that are irrelevant. For example, just delete the UUID/string question instead of just saying no. Presumably that delivers the same amount of information to you guys. Or, just saying "trivial patch to fix a crasher" instead of filling in all of the existing questions to the same effect. Would that make sense?
Assignee | ||
Comment 23•11 years ago
|
||
Comment on attachment 753126 [details] [diff] [review] Patch including the crashfix Thanks for creating the rolled up patch, Gijs! But I don't think this beauacracy is really needed as I mentioned that I want to land the roll-up patch in comment 17 -- it just happened that RyanVM beat me to landing it. :-) So I'll just go ahead and assume a+.
Attachment #753126 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 24•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/ebb8f343fea6
Assignee | ||
Comment 25•11 years ago
|
||
I mistakenly landed this patch <https://hg.mozilla.org/integration/mozilla-inbound/rev/d921692be88f> which was really for bug 876129 with this bug #. Sorry!
Comment 27•11 years ago
|
||
User Agent :Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:23.0) Gecko/20100101 Firefox/23.0 Build ID: 20130711122148 Tested on FX 23-B5 using http://www.klack.de/fernsehprogramm/2015-im-tv/0/free.html For dark Background, I see light scroll-bar.
Comment 28•11 years ago
|
||
User Agent :Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:25.0) Gecko/20130712 Firefox/25.0 Build ID: 20130712030203 User Agent :Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:24.0) Gecko/20130712 Firefox/24.0 Build ID: 20130712004003 Tested on latest nightly and Aurora using http://www.klack.de/fernsehprogramm/2015-im-tv/0/free.html For dark Background, I see light scroll-bar.
Status: RESOLVED → VERIFIED
Comment 29•11 years ago
|
||
User Agent : Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:23.0) Gecko/20100101 Firefox/23.0 Build ID: 20130715155216 User Agent : Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:25.0) Gecko/20130718 Firefox/25.0 Build ID: 20130718030201 User Agent : Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:24.0) Gecko/20130718 Firefox/24.0 Build ID: 20130718004004 Tested on latest nightly and Aurora using http://www.klack.de/fernsehprogramm/2015-im-tv/0/free.html For dark Background, I see light scroll-bar.
You need to log in
before you can comment on or make changes to this bug.
Description
•