Extract native Vista toolbar styles and expose to Themes

RESOLVED FIXED

Status

()

Core
Widget: Win32
P2
normal
RESOLVED FIXED
10 years ago
7 years ago

People

(Reporter: faaborg, Assigned: vlad)

Tracking

({dev-doc-complete})

Trunk
x86
Windows Vista
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
We would like to use the silver toolbar appearance on Vista for the main window of Firefox and the media toolbar appearance (black) in the Library window.

Vlad wrote an app to explore what parts of the native OS theme we can extract, so he will be able to provide more specific details.
Flags: blocking-firefox3?
Component: Theme → Widget: Win32
Flags: blocking-firefox3?
Product: Firefox → Core
QA Contact: theme → win32
Flags: blocking1.9?
What we want are rebars, not toolbars. The right theme to use depends on the appearance:
media -> "Media::Rebar"
communcations -> "Communcations::Rebar"
alternate -> "Rebar" or "Alternate::Rebar"

part: RP_BACKGROUND
state: 0

So how should we expose this to themers?
Yep; we need to define new appearance properties.  I think right now the 'toolbox' appearance is the base rebar; we should define something like media-toolbox and communications-toolbox.  The nice thing is that "Media::Rebar" falls back to "Rebar" if the specific subclass isn't found, so we don't have to do anything special for windows.  For now, we can ignore those properties on other platforms I think, or just treat them the same as a normal 'toolbox'.
Assignee: nobody → vladimir
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
Created attachment 305902 [details] [diff] [review]
add new appearance types

Adds three new appearance types, media-toolbox, communications-toolbox, and browsertabbar-toolbox.  Note that these will all fall back to the normal 'Rebar' style if the specific subclass is not available.
Attachment #305902 - Flags: review?(roc)
How many of those Vista things are there? Will we eventually want to add a huge number of theme constants to cover them all?
Nope, should just be these three, at least for 1.9.  If we have time i'd like to revisit how we do native theme stuff for moz2+.
I'd like to hear your thoughts about that, I don't have any :-)

Shouldn't you be adding something to nsNativeThemeWin::CloseData?
Created attachment 306204 [details] [diff] [review]
updated patch

Updated patch, with something now done in CloseTheme().  Also got rid of a bogus hunk that snuck in here that should've been done in the other theme patch (and now is).
Attachment #305902 - Attachment is obsolete: true
Attachment #306204 - Flags: review?(roc)
Attachment #305902 - Flags: review?(roc)
Flags: blocking1.9+
Flags: tracking1.9+
Priority: P3 → P2
Comment on attachment 306204 [details] [diff] [review]
updated patch

+#define CLOSE_THEME(_x)  if (_x) { closeTheme(_x); _x = NULL; }

Macros suck, would you mind "static void CloseTheme(HANDLE* aTheme)"?
Attachment #306204 - Flags: superreview+
Attachment #306204 - Flags: review?(roc)
Attachment #306204 - Flags: review+
Comment on attachment 306204 [details] [diff] [review]
updated patch

One more to request approval on..

roc, I'd rather not do a function since then I can't assign NULL back to the pointer (unless I do a ref to a pointer, but ick).  I mean, I can expand out the macro, just seems cleaner this way.  (Really, I -almost- made a little helper class so you could do  WindowsThemeData mButtonTheme("Button");, but decided that would probably be overkill for now :)
Attachment #306204 - Flags: approval1.9b4?
Comment on attachment 306204 [details] [diff] [review]
updated patch

a1.9b4=beltzner
Attachment #306204 - Flags: approval1.9b4? → approval1.9b4+
Checked in.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Blocks: 420232
Unless its something to come later, I am not seeing the 'black' bar in the Library on my Vista HP.  

Today's build:
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b4pre) Gecko/2008030106 Minefield/3.0b4pre Firefox/3.0 ID:2008030106
Its not hooked up yet, but the patch works with some userchrome.

On another note, while this degrades nicefully in XP, it doesn't theme the text color on the toolbox, meaning that you have to use special theme overrides for XP and Vista to fix it anyway. Is that possible to extract from the theme? Or maybe theres an easier to way to make sure the text is visible that I don't know of.

Comment 14

10 years ago
From what I can tell, Communications::Rebar has a text colour defined in the theme, while Media::Rebar doesn't... 

There's also the issue that the "Media" and "Communications" subclasses could disappear entirely in the next version of Windows, so if you do then explicitly set the text colour to another colour, it might look a bit odd on newer versions of windows.
(Reporter)

Comment 15

10 years ago
>There's also the issue that the "Media" and "Communications" subclasses could
>disappear entirely in the next version of Windows

They are encouraging other applications to use these toolbar styles in their user experience guidelines, so they probably will keep them around for legacy applications with Windows 7.  If not we will have plenty of time to create an update while it is in beta.

Updated

10 years ago
Depends on: 427045

Updated

10 years ago
Depends on: 431309
Depends on: 427173

Updated

10 years ago
Keywords: dev-doc-needed

Updated

10 years ago
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.