Closed Bug 1062801 Opened 10 years ago Closed 10 years ago

[10.10] Vibrant regions turn black when "Reduce Transparency" is enabled under Accessibility -> General

Categories

(Core :: Widget: Cocoa, defect)

All
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35
Tracking Status
firefox33 --- wontfix
firefox34 + fixed
firefox35 --- fixed

People

(Reporter: mstange, Assigned: mstange)

References

Details

Attachments

(1 file)

We need to respect the pref and stop clearing the window background when it's set and instead paint a solid color.
We talked about this yesterday and I just spent some hours trying to figure this out, but didn't get very far.

The effectiveAppearance property is just the same vibrancylight thing we set it to, as best I can tell.

I then tried poking at the view, but all its flags are set identically in the "reduce transparency" and non-reduce-transparency cases, even "allowsVibrancy", which you kind of would expect would maybe toggle, but no.

The pref is stored in the ~/Library/Preferences/com.apple.universalaccess.plist, and is a bool called "reduceTransparency" (who would have thunk it)

However, I don't know if there's any notification sent by the pref pane that we can listen for to let us know the pref changed - Maybe decompiling the pref pane would help, but I don't know enough about how to do that - just poking at it with "strings" didn't get me very far...
(In reply to :Gijs Kruitbosch from comment #1)
> We talked about this yesterday and I just spent some hours trying to figure
> this out, but didn't get very far.
> 
> The effectiveAppearance property is just the same vibrancylight thing we set
> it to, as best I can tell.
> 
> I then tried poking at the view, but all its flags are set identically in
> the "reduce transparency" and non-reduce-transparency cases, even
> "allowsVibrancy", which you kind of would expect would maybe toggle, but no.
> 
> The pref is stored in the
> ~/Library/Preferences/com.apple.universalaccess.plist, and is a bool called
> "reduceTransparency" (who would have thunk it)
> 
> However, I don't know if there's any notification sent by the pref pane that
> we can listen for to let us know the pref changed - Maybe decompiling the
> pref pane would help, but I don't know enough about how to do that - just
> poking at it with "strings" didn't get me very far...

I thought that this would have to come in via the DistributedNotificationCenter, but apparently I was wrong. Using a combination of randomly-pulled-off-the-internet knowledge, I made an "OS X playground" in the latest version of XCode, and ran this Swift snippet:

// Playground - noun: a place where people can play
import XCPlayground;
import Cocoa;

XCPlayground.XCPSetExecutionShouldContinueIndefinitely(continueIndefinitely: true);

let notificationCenter = NSNotificationCenter.defaultCenter();
let mainQueue = NSOperationQueue.mainQueue();

notificationCenter.addObserverForName(nil, object: nil, queue: mainQueue) { note in
    println("Got notification " + note.name);
}

which doesn't output anything (extra) when toggling the reduce transparency toggle, whereas when I change the overall color scheme while this is running, I see:

Got notification NSSystemColorsWillChangeNotification
Got notification NSSystemColorsDidChangeNotification
Got notification NSControlTintDidChangeNotification
Got notification kCUINotificationAquaColorVariantChanged

So yeah. Wherever it is, not there...
(In reply to :Gijs Kruitbosch from comment #1)
> I then tried poking at the view, but all its flags are set identically in
> the "reduce transparency" and non-reduce-transparency cases, even
> "allowsVibrancy", which you kind of would expect would maybe toggle, but no.

I looked at the implementation of -[NSVisualEffectView drawRect:] and found out that it fills the view with the color returned from -[NSVisualEffectView _currentFillColor], and - good news! - this fill color becomes opaque when the "reduce transparency" pref is set.
So we can just draw that color when painting our -moz-appearance:-moz-mac-vibrancy-light/dark background.

I haven't found out yet how to be notified of dynamic changes, or how to properly react to them. But switching window focus repaints the vibrant areas anyway, so we'll look bad only until our window regains focus after the pref switch.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Attachment #8499476 - Flags: review?(smichaud)
Comment on attachment 8499476 [details] [diff] [review]
Fill with _currentFillColor during -moz-appearance drawing

Looks fine to me.
Attachment #8499476 - Flags: review?(smichaud) → review+
[Tracking Requested - why for this release]:
Like bug 1077358, yosemite-only, but pretty glaring, and so it'd be sad if we shipped this for 34.
Ditto to bug 1077358, tracking for 34, which is quickly becoming a Yosemite friendly release.
This is hitting non-unified bustage on inbound (wasn't visible earlier due to the other bustage that landed around the same time). I'm willing to give you some time to fix, but will need to backout if something doesn't land soon.
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=2823423&repo=mozilla-inbound
Flags: needinfo?(mstange)
This should fix it: https://hg.mozilla.org/integration/mozilla-inbound/rev/e1b7ef73bdae
Flags: needinfo?(mstange)
https://hg.mozilla.org/mozilla-central/rev/ad40e2248efa
https://hg.mozilla.org/mozilla-central/rev/e1b7ef73bdae
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
This should be in the latest nightly, should it? I've tested with 35.0a1 (2014-10-08), downloaded just a minute ago, on 10.10 14A386a, and for me the region still turns black if I enable "Reduce Transparency".
(In reply to Nomis101 from comment #12)
> This should be in the latest nightly, should it? I've tested with 35.0a1
> (2014-10-08), downloaded just a minute ago, on 10.10 14A386a, and for me the
> region still turns black if I enable "Reduce Transparency".

Even after refocusing the window?
(In reply to :Gijs Kruitbosch from comment #13)
> (In reply to Nomis101 from comment #12)
> > This should be in the latest nightly, should it? I've tested with 35.0a1
> > (2014-10-08), downloaded just a minute ago, on 10.10 14A386a, and for me the
> > region still turns black if I enable "Reduce Transparency".
> 
> Even after refocusing the window?

Interesting. It's gone after I refocus the window. Initially if I enable "Reduce Transparency" it turns black. Than I put Firefox in the foreground and after that the black it gone. Even after I put Firefox back in the background.
(In reply to Nomis101 from comment #14)
> (In reply to :Gijs Kruitbosch from comment #13)
> > (In reply to Nomis101 from comment #12)
> > > This should be in the latest nightly, should it? I've tested with 35.0a1
> > > (2014-10-08), downloaded just a minute ago, on 10.10 14A386a, and for me the
> > > region still turns black if I enable "Reduce Transparency".
> > 
> > Even after refocusing the window?
> 
> Interesting. It's gone after I refocus the window. Initially if I enable
> "Reduce Transparency" it turns black. Than I put Firefox in the foreground
> and after that the black it gone. Even after I put Firefox back in the
> background.

See comment #3, this is expected. We should file a followup bug for fixing this, but I don't expect people to be changing this pref every other moment, so I don't think it's the most important thing to address right now.
Need to uplift this, right? Can you request approval?
Flags: needinfo?(mstange)
Comment on attachment 8499476 [details] [diff] [review]
Fill with _currentFillColor during -moz-appearance drawing

Approval Request Comment
[Feature/regressing bug #]: 10.10 support / vibrant sidebars (bug 1051522 + bug 1059278)
[User impact if declined]: vibrant sidebars are black if vibrancy is disabled
[Describe test coverage new/current, TBPL]: none
[Risks and why]: low to medium, but leaving vibrancy in without this fix would be riskier
[String/UUID change made/needed]: none
Attachment #8499476 - Flags: approval-mozilla-aurora?
Flags: needinfo?(mstange)
Comment on attachment 8499476 [details] [diff] [review]
Fill with _currentFillColor during -moz-appearance drawing

I'm approving for 34 as we want to support 10.10 in this release. As I don't expect our Beta audience to provide much coverage for this OS, I would like to see a QE test plan around the release if one doesn't already exist. cc lizzard

Aurora+
Attachment #8499476 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(lhenry)
Flags: needinfo?(lhenry)
You need to log in before you can comment on or make changes to this bug.