Closed Bug 1045217 Opened 7 years ago Closed 7 years ago

[10.10] Tooltips have an incorrect color: yellow, but should be grey

Categories

(Core :: Widget: Cocoa, defect)

x86
macOS
defect
Not set
normal
Points:
1

Tracking

()

VERIFIED FIXED
mozilla35
Iteration:
35.3
Tracking Status
firefox33 --- wontfix
firefox34 + verified
firefox35 --- verified

People

(Reporter: bugzilla, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Whiteboard: [yosemite])

Attachments

(2 files, 1 obsolete file)

Build ID:Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:34.0) Gecko/20100101 Firefox/34.0

STR:
Hover any tab to see a tooltip with the website's full title/name

Actual results:
Tooltips are yellow

Expected results:
Tooltips should be grey like the rest of the tooltips in OS X 10.10
Summary: [10.10] Tooltips have the incorrect color: yellow, but should be grey → [10.10] Tooltips have an incorrect color: yellow, but should be grey
Component: General → Themes
Product: Firefox → Toolkit
Component: Themes → Widget: Cocoa
Product: Toolkit → Core
In my VM, the background is #f0f0f0, but I don't know whether the tooltips are using vibrancy/transparency, as the VM doesn't show any. In any case, the code that needs updating is here: http://mxr.mozilla.org/mozilla-central/source/widget/cocoa/nsNativeThemeCocoa.mm#2327 .

We could also do with slightly more horizontal padding on the tooltips.
This should use the behind-window vibrancy on OS X. I expect the simplest will be to just update the -moz-appearance here:

http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/osx/global/popup.css#101

although I wonder if instead we should not change -moz-appearance: tooltip to use behind-window-vibrancy in cocoa instead... That seems cleaner and would also mean other apps/themes/things-with-moz-appearance-tooltip automatically benefit from the update. I'd be happy to look at this myself - Markus, could you give a pointer or two on how to do that, or is that trickier than it sounds? :-)
(In reply to :Gijs Kruitbosch from comment #2)
> I'd be happy to look at this myself -
> Markus, could you give a pointer or two on how to do that, or is that
> trickier than it sounds? :-)
Flags: needinfo?(mstange)
FYI, we don't do more than just paint the yellow background-color for "-moz-appearance: tooltip;"

-----------------------------------
2374     case NS_THEME_TOOLTIP:
2375       CGContextSetRGBFillColor(cgContext, 0.996, 1.000, 0.792, 0.950);
2376       CGContextFillRect(cgContext, macRect);
2377       break;
(In reply to Stefan [:stefanh] from comment #4)
> FYI, we don't do more than just paint the yellow background-color for
> "-moz-appearance: tooltip;"
> 
> -----------------------------------
> 2374     case NS_THEME_TOOLTIP:
> 2375       CGContextSetRGBFillColor(cgContext, 0.996, 1.000, 0.792, 0.950);
> 2376       CGContextFillRect(cgContext, macRect);
> 2377       break;

Yes, I know - I tried to point out as much in comment #1 . But as you can see from the screenshot, this is no longer "just" transparency, but there's some blurring involved as well. To get this right, we should use the behind-window vibrancy implemented in bug 1055614.
(In reply to :Gijs Kruitbosch from comment #2)
> Markus, could you give a pointer or two on how to do that, or is that
> trickier than it sounds? :-)

It's not - should be rather simple, actually. In nsNativeThemeCocoa, don't paint anything for NS_THEME_TOOLTIP if VibrancyManager::SystemSupportsVibrancy(), and add NS_THEME_TOOLTIP handling near the places NS_THEME_MAC_VIBRANCY_LIGHT is used in nsDisplayList.cpp and nsChildView.mm.
Flags: needinfo?(mstange)
Attached patch vibrancy for tooltips, (obsolete) — Splinter Review
Per discussion on IRC, this is a start, but we need to (at a minimum) tell OS X that we want vibrancy even when the tooltip (window) doesn't have focus.
Attachment #8488605 - Flags: feedback?(mstange)
Markus, do you have time to look at this "soon" ? If not, shall I put up a patch to at least adjust the color to a transparent grey so it doesn't look quite so harshly out of place, that we can uplift for 34? :-)
Flags: needinfo?(mstange)
Last time I looked at this I wasn't able to make tooltips use vibrancy, no matter what I tried.
So yes, adjusting the color would be the right short-term fix.
Flags: needinfo?(mstange)
This looks right to me on my actual 10.10 machine in terms of transparency/shade of gray.
Attachment #8498171 - Flags: review?(mstange)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Comment on attachment 8498171 [details] [diff] [review]
fix 10.10 tooltips to use grey instead of yellow,

That's a very white-looking grey you have there.
Attachment #8498171 - Flags: review?(mstange) → review+
Yeah, checked in with more grey-ish grey (but still pretty non-grey).

remote:   https://hg.mozilla.org/integration/fx-team/rev/124797ab4a5c


Twiddling lots of flags for backlog things. Marco, can you add this? (bug 1063714 is currently blocked, and I worked on this (important for yosemite stuff) when I didn't know I was going to need to back out bug 1057166 - I'll get back to that tonight / tomorrow, as this is done now)

I'll file a separate bug for making this use proper translucency.
Iteration: --- → 35.3
Points: --- → 1
Flags: qe-verify+
Flags: needinfo?(mmucci)
Flags: in-testsuite-
Flags: firefox-backlog+
See Also: → 1075653
Added to IT 35.3
Flags: needinfo?(mmucci)
https://hg.mozilla.org/mozilla-central/rev/124797ab4a5c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
QA Contact: catalin.varga
Verified as fixed using the following environment:

FF 35
Build Id: 20141002030202
Os: Mac Os X 10.10
Status: RESOLVED → VERIFIED
Comment on attachment 8498171 [details] [diff] [review]
fix 10.10 tooltips to use grey instead of yellow,

Approval Request Comment
[Feature/regressing bug #]: yosemite
[User impact if declined]: really bright yellow tooltips in a gone-grey-and-translucent OS
[Describe test coverage new/current, TBPL]: nope
[Risks and why]: pretty low; uses a different color somewhere in widget instead of the one it uses now
[String/UUID change made/needed]: nope

NB: please transplant the landed patch instead of the one on the bug.
Attachment #8498171 - Flags: approval-mozilla-aurora?
Comment on attachment 8498171 [details] [diff] [review]
fix 10.10 tooltips to use grey instead of yellow,

Aurora+
Attachment #8498171 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
[Tracking Requested - why for this release]:
Verified as fixed using:

FF34
Build Id: 20141009004002
OS: Mac Os X 10.10
Attachment #8488605 - Attachment is obsolete: true
Attachment #8488605 - Flags: feedback?(mstange)
You need to log in before you can comment on or make changes to this bug.