Closed Bug 668195 Opened 9 years ago Closed 9 years ago

Consider using CoreUI to draw unified title/toolbars and statusbars

Categories

(Core :: Widget: Cocoa, defect)

All
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla8

People

(Reporter: mstange, Assigned: mstange)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(5 files, 2 obsolete files)

Attached patch v1 (obsolete) — Splinter Review
Currently we draw our titlebars, unified toolbars and statusbars manually using hardcoded colors and custom gradients because neither Cocoa nor HITheme provide public APIs for these things.
We might want to think about using CoreUI here. CoreUI is a private framework without any compatibility promises, so this would increase our risk of being broken by OS X updates. Josh, Steven, what's your opinion?

The most pressing problem that would be simplified by switching to CoreUI is the 10.7 chrome style which uses a noise texture that I don't really want to draw manually in C++.

I've tested this patch on 10.5 and 10.6; I hope it also works on 10.7. Here's a tryserver build:
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mstange@themasta.com-02fac412f0d2/try-macosx64/firefox-7.0a1.en-US.mac.dmg

Notes on the implementation:
The entry point to CoreUI drawing is the CUIDraw function. The widget that is drawn (and its properties) is determined by a dictionary which is passed as an argument to CUIDraw. Values for this dictionary can be figured out by observing other programs. For example, this is the unified toolbar drawn by the preferences window of Address Book.app:

> (gdb) attach 5485
> Attaching to process 5485.
> Reading symbols for shared libraries . done
> Reading symbols for shared libraries ................................................................................................................................................................. done
> 0x00007fff8887829a in mach_msg_trap ()
> (gdb) break CUIDraw
> Breakpoint 1 at 0x7fff85a60e6e
> (gdb) c
> Continuing.
> 
> Breakpoint 1, 0x00007fff85a60e6e in CUIDraw ()
> (gdb) po $rdx
> {
>     kCUIWindowFrameUnifiedTitleBarHeightKey = 78;
>     state = normal;
>     widget = kCUIWidgetWindowFrame;
>     windowtype = regularwin;
> }

Here's a very short intro to CoreUI:
http://www.theregister.co.uk/2009/01/12/mac_secrets_coreui/
Attachment #542791 - Flags: feedback?(joshmoz)
Attached file minimal test app
I'm not sure if this is the way it is supposed to look like …
(In reply to comment #2)
> Created attachment 542815 [details]
> How patch v1 looks like on Lion

Thanks! That's what I expected it to look like; the Firefox main window obviously still needs some CSS work - and I wouldn't land this patch before that's done.
(It looks like this because in tabs-on-top mode the native unified toolbar is completely covered by a gradient image.)
The preferences and download windows don't have that problem, for example.
Attached image Patch v1 on Lion for TB (obsolete) —
Just to let you know, this is how your patch v1 looks on Lion for Thunderbird. And Thunderbird crashes 4 seconds after startup. Tested with Thunderbird 5 on Lion 11A511.
(In reply to comment #4)
> And Thunderbird crashes 4 seconds after startup.

And it doesn't without this patch? Do you have a stack trace?
(In reply to comment #5)
> (In reply to comment #4)
> > And Thunderbird crashes 4 seconds after startup.
> 
> And it doesn't without this patch? Do you have a stack trace?

Sorry, this crash has nothing to do with this patch. Must be something other, because also an absolutely unpatched TB6.0 build on 10.7 crashes for me.
Comment on attachment 542791 [details] [diff] [review]
v1

Review of attachment 542791 [details] [diff] [review]:
-----------------------------------------------------------------

I have no objection to this approach, thanks for looking into this!
Attachment #542791 - Flags: feedback?(joshmoz) → feedback+
Blocks: 672050
Attached image Patch v1 on Lion for TB
Now that I fixed the crash, here is the screenshot of TB with this patch on Lion 11A511
Attachment #543695 - Attachment is obsolete: true
CoreUI is a Private Framework and will prevent Firefox's inclusion in the Mac App Store.

Firefox in Mac App Store Bugs:  641085, 636841
(In reply to comment #9)
The problem is that there really is no public API for toolbar drawing. Here's a demo app that shows the contortions we'd have to go through if we only wanted to use public APIs: We'd have to create an invisible window, attach an NSToolbar with the right size to it, and render the window into an offscreen buffer. But even then we'd have to fake the window's activeness state through private methods (_hasKeyAppearance and _hasActiveAppearanceIgnoringKeyFocus).
Attached patch v2Splinter Review
Assignee: nobody → mstange
Attachment #542791 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #549327 - Flags: review?(joshmoz)
Good thing to point out about the App Store but I don't think we have a better option here, as Markus pointed out. I'd like to continue with this work.
Blocks: 675445
Blocks: 595711
Attachment #549327 - Flags: review?(joshmoz) → review+
http://hg.mozilla.org/mozilla-central/rev/93c610640952
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
I'm getting a white-ish (hex #f7f7f7) see image: http://i.imgur.com/p92mS.png This is somewhat distracting and no other app has this line. 

(I'm running Snow Leopard)
(In reply to d.a. from comment #15)
> I'm getting a white-ish (hex #f7f7f7) see image:
> http://i.imgur.com/p92mS.png This is somewhat distracting and no other app
> has this line. 
> 
> (I'm running Snow Leopard)

If you see this with today's nightly and not with an older version, please file a new bug on this (in "Product: Core" and "Component: Widget: Cocoa") and make it block this one. Comments in closed bugs tend to get lost.
Depends on: 678039
Depends on: 678091
Depends on: 678184
Depends on: 693690
You need to log in before you can comment on or make changes to this bug.