Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Consider using CoreUI to draw unified title/toolbars and statusbars

RESOLVED FIXED in mozilla8

Status

()

Core
Widget: Cocoa
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: mstange, Assigned: mstange)

Tracking

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

Trunk
mozilla8
All
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 2 obsolete attachments)

(Assignee)

Description

6 years ago
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/
Attachment #542791 - Flags: feedback?(joshmoz)
(Assignee)

Comment 1

6 years ago
Created attachment 542796 [details]
minimal test app

Comment 2

6 years ago
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 …
(Assignee)

Comment 3

6 years ago
(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

6 years ago
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.
(Assignee)

Comment 5

6 years ago
(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

6 years ago
(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

6 years ago
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+
(Assignee)

Updated

6 years ago
Blocks: 672050

Comment 8

6 years ago
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
Attachment #543695 - Attachment is obsolete: true

Comment 9

6 years ago
CoreUI is a Private Framework and will prevent Firefox's inclusion in the Mac App Store.

Firefox in Mac App Store Bugs:  641085, 636841
(Assignee)

Comment 10

6 years ago
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).
(Assignee)

Comment 11

6 years ago
Created attachment 549327 [details] [diff] [review]
v2
Assignee: nobody → mstange
Attachment #542791 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #549327 - Flags: review?(joshmoz)

Comment 12

6 years ago
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.

Updated

6 years ago
Blocks: 675445
(Assignee)

Updated

6 years ago
Blocks: 595711

Updated

6 years ago
Attachment #549327 - Flags: review?(joshmoz) → review+
(Assignee)

Comment 13

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/93c610640952
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/93c610640952
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla8

Comment 15

6 years ago
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)
Depends on: 678002

Comment 16

6 years ago
(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.

Updated

6 years ago
Depends on: 678039
(Assignee)

Updated

6 years ago
Depends on: 678091
(Assignee)

Updated

6 years ago
Depends on: 678184
(Assignee)

Updated

6 years ago
Depends on: 693690
Depends on: 764176
You need to log in before you can comment on or make changes to this bug.