Last Comment Bug 668195 - Consider using CoreUI to draw unified title/toolbars and statusbars
: Consider using CoreUI to draw unified title/toolbars and statusbars
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Cocoa (show other bugs)
: Trunk
: All Mac OS X
: -- normal with 4 votes (vote)
: mozilla8
Assigned To: Markus Stange [:mstange]
:
Mentors:
Depends on: 693690 678002 678039 678091 678184 764176
Blocks: 595711 667476 672050 675445
  Show dependency treegraph
 
Reported: 2011-06-29 06:10 PDT by Markus Stange [:mstange]
Modified: 2014-02-07 08:43 PST (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (20.45 KB, patch)
2011-06-29 06:10 PDT, Markus Stange [:mstange]
jaas: feedback+
Details | Diff | Review
minimal test app (3.07 KB, text/plain)
2011-06-29 06:21 PDT, Markus Stange [:mstange]
no flags Details
How patch v1 looks like on Lion (116.53 KB, image/png)
2011-06-29 07:30 PDT, Dominic Spitaler
no flags Details
Patch v1 on Lion for TB (22.94 KB, image/jpeg)
2011-07-03 13:31 PDT, Nomis101
no flags Details
Patch v1 on Lion for TB (77.68 KB, image/jpeg)
2011-07-24 09:57 PDT, Nomis101
no flags Details
window frame drawing using almost only public apis (4.02 KB, text/plain)
2011-07-28 04:15 PDT, Markus Stange [:mstange]
no flags Details
v2 (20.70 KB, patch)
2011-07-29 02:16 PDT, Markus Stange [:mstange]
jaas: review+
Details | Diff | Review

Description Markus Stange [:mstange] 2011-06-29 06:10:03 PDT
Created attachment 542791 [details] [diff] [review]
v1

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/
Comment 1 Markus Stange [:mstange] 2011-06-29 06:21:47 PDT
Created attachment 542796 [details]
minimal test app
Comment 2 Dominic Spitaler 2011-06-29 07:30:45 PDT
Created attachment 542815 [details]
How patch v1 looks like on Lion

I'm not sure if this is the way it is supposed to look like …
Comment 3 Markus Stange [:mstange] 2011-06-29 07:37:00 PDT
(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.
Comment 4 Nomis101 2011-07-03 13:31:59 PDT
Created attachment 543695 [details]
Patch v1 on Lion for TB

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.
Comment 5 Markus Stange [:mstange] 2011-07-03 13:55:13 PDT
(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?
Comment 6 Nomis101 2011-07-09 05:01:35 PDT
(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 7 Josh Aas 2011-07-09 12:22:08 PDT
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!
Comment 8 Nomis101 2011-07-24 09:57:09 PDT
Created attachment 548025 [details]
Patch v1 on Lion for TB

Now that I fixed the crash, here is the screenshot of TB with this patch on Lion 11A511
Comment 9 David Doukidis 2011-07-27 20:15:31 PDT
CoreUI is a Private Framework and will prevent Firefox's inclusion in the Mac App Store.

Firefox in Mac App Store Bugs:  641085, 636841
Comment 10 Markus Stange [:mstange] 2011-07-28 04:15:17 PDT
Created attachment 549066 [details]
window frame drawing using almost only public apis

(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).
Comment 11 Markus Stange [:mstange] 2011-07-29 02:16:04 PDT
Created attachment 549327 [details] [diff] [review]
v2
Comment 12 Josh Aas 2011-07-29 08:42:22 PDT
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.
Comment 14 :Ehsan Akhgari (busy, don't ask for review please) 2011-08-09 08:52:31 PDT
http://hg.mozilla.org/mozilla-central/rev/93c610640952
Comment 15 d.a. 2011-08-10 09:43:07 PDT
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)
Comment 16 Stefan [:stefanh] 2011-08-10 13:28:12 PDT
(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.

Note You need to log in before you can comment on or make changes to this bug.