Closed Bug 1055018 Opened 6 years ago Closed 6 years ago

[10.10] Make widgets drawn with CoreUI use the Yosemite style

Categories

(Core :: Widget: Cocoa, defect)

All
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: mstange, Assigned: mstange)

References

Details

Attachments

(1 file, 5 obsolete files)

Our toolbars and toolbar buttons, among other things, look like on 10.9 and haven't adapted the 10.10 look. This is because when calling CUIDraw we pass NULL to the CoreUIRenderer argument.

On 10.10 we can use -[NSAppearance _drawInRect:context:options:] on [NSAppearance appearanceNamed:NSAppearanceNameAqua], which will call CUIDraw with the correct CoreUIRenderer and give us the 10.10 look. It works for everything except scrollbars, as far as I could tell. (Scrollbar thumbs are too thick when drawn through NSAppearance, I don't know why.)

I have patches.
Attached patch wip (obsolete) — Splinter Review
Attached patch v1 (obsolete) — Splinter Review
Attachment #8474830 - Attachment is obsolete: true
Attachment #8475159 - Flags: review?(smichaud)
Attached patch v2 (obsolete) — Splinter Review
Oops, forgot a .size in the max area checks.
Attachment #8475159 - Attachment is obsolete: true
Attachment #8475159 - Flags: review?(smichaud)
Attachment #8475184 - Flags: review?(smichaud)
Comment on attachment 8475184 [details] [diff] [review]
v2

Looks like I need to keep using [NSWindow coreUIRenderer] on 10.6 at least. Tryserver is all orange.
Attachment #8475184 - Flags: review?(smichaud)
Attached patch v3 (obsolete) — Splinter Review
Attachment #8475184 - Attachment is obsolete: true
Attachment #8475828 - Flags: review?(smichaud)
Markus, does this patch depend on those for bug 1051522?  If not I'll review this patch first.  I hope to get to both bugs by the end of this week.
No, this patch does depend on any other patches.
Comment on attachment 8475828 [details] [diff] [review]
v3

This mostly looks fine to me.  But I did notice what seems to be a serious problem:

In a class-dump of the AppKit framework on OS X 10.9 (both 10.9.1 and 10.9.4), I found the following three _drawRect methods of the NSAppearance class:

- (void)_drawInRect:(struct CGRect)arg1 context:(struct CGContext *)arg2
  options:(id)arg3 inWindow:(id)arg4;
- (void)_drawInRect:(struct CGRect)arg1 context:(struct CGContext *)arg2
  options:(id)arg3 inView:(id)arg4;
- (void)_drawInRect:(struct CGRect)arg1 context:(struct CGContext *)arg2
  options:(id)arg3 delegate:(id)arg4;

Notice that all of them have four parameters.  But the definition in your patch has only three.

You do check that your three-parameter method exists before you call it ... but I think that just means that you *never* call it.
Comment on attachment 8475828 [details] [diff] [review]
v3

Oops, forgot to check on Yosemite.  When I did (on Yosemite DP6, the current DP) I found your three-parameter _drawInRect method (and no others).  So the problem isn't as serious as I first thought.  But you're still not using NSAppearance on OS X 10.9.  Is this intentional?
Comment on attachment 8475828 [details] [diff] [review]
v3

> But you're still not using NSAppearance on OS X 10.9.  Is this intentional?

Not using NSAppearance on 10.9 isn't a big deal.  If you change your mind later, we can deal with that then.
Attachment #8475828 - Flags: review?(smichaud) → review+
(In reply to Steven Michaud from comment #9)
> But you're still not using
> NSAppearance on OS X 10.9.  Is this intentional?

Yes, this is intentional. I'll add a comment about that.
Attached patch v4 (obsolete) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=39aa79c801d8
Attachment #8475828 - Attachment is obsolete: true
Attached patch v5Splinter Review
Interestingly, NSAppearance and +[NSAppearance appearanceNamed:] are both available on 10.8, but [NSAppearance appearanceNamed:@"NSAppearanceNameAqua"] throws an NSInternalInconsistencyException on 10.8. I've added an "if (nsCocoaFeatures::OnYosemiteOrLater())" to GetAquaAppearance().
I've also restored the titlebar top line overdrawing in DrawNativeTitlebar that the previous patches were removing. It's still needed.
Attachment #8479871 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/0eb0a00238ed
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.