313388 - Make toolkit winstripe theme match Windows Classic

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[James Ross]

This bug is a sub-bug of bug 243078, for dealing with making the Toolkit
'winstripe' theme (menu, toolbar and popups only) match the Windows Classic
appearance when using pure CSS. This is the fallback position for when no
theming is available.

BTW, this component sucks, I want a Toolkit: Themes component. :-)

Please do NOT comment on this bug unless you have a technical contribution, or
it will become impossible to work on this bug.

Comment 1

[James Ross]

Attachment: Phase 1 - XP Classic using only CSS

This patch turns off the native code that was checked-in against my wishes some time ago, and corrects the CSS to visually match the Windows XP Classic appearance.

Just to pre-empt one question: the EM-based values are used where the spacing is based on the font-size. This means that the sizes of various gaps continue to match with native menus when using large fonts (or simply different font sizes).

Picking randomly for first-review flag.

Comment 2

[Kevin Gerich]

After applying your patch on the trunk, I see that classic menu styles are applied even when the Luna theme is selected. James, I'm having trouble following the story here. Please bear with me. Is this part of a larger patch that you're working on to get the correct appearance on both Luna and Classic, or do you intend this patch to go in by itself?

Comment 3

[James Ross]

This is phase 1, as I've stated time and again. This patch needs to go in on its own so regressions with it can be found before I do the native theming. See:
  http://twpol.dyndns.org/weblog/2005/11/01/01

Comment 4

[Mike Connor [:mconnor]]

The plan is ok, its trunk only, please make sure stuart knows this is coming

Comment 5

[James Ross]

(In reply to comment #4)
> The plan is ok, its trunk only, please make sure stuart knows this is coming

Who exactly and why?

And what other review shenanigans are needed?

Comment 6

[Bryan]

(In reply to comment #4)
> The plan is ok, its trunk only, please make sure stuart knows this is coming

Checkin has been approved.  Can someone please check this in so we can get this moving?

~B

Comment 7

[James Ross]

I'm waiting for someone to answer the question about reviews, so I know if I *can* check it in or not. I'm not sure it's worth my effort if people don't answer critical (and supposedly easy) questions, though. *sigh*

Comment 8

[Steffen Wilberg]

> (In reply to comment #4)
> > The plan is ok, its trunk only, please make sure stuart knows this is coming
> Who exactly and why?
That would be Stuart Parmenter, which is one of the guys working on the Cairo graphics backend. I'm not sure how this would affect him, but I've cc'ed him, so now he knows :-)

> And what other review shenanigans are needed?
Kevin has reviewed the patch and mconnor says the plan is ok. That's enough.

Comment 9

[James Ross]

Ok, this has landed on trunk.

Any regressions or issues that show up as a result should be filed as "depends on" bugs to keep tracking sane.

I'm going to leave this bug open /for the moment/ as I've not dealt with bug 313391 yet (not sure what the status of it is with Places, either).

Comment 10

[Ben Goodger (use ben at mozilla dot org for email)]

This has caused a regression on the trunk... on XP-luna there's a white line underneath the toolbar. 

Comment 11

[James Ross]

Phase 1 complete. Phase 2 in bug 337771.

Comment 12

[[e-mail]]

Comment on attachment 217713 [details] [diff] [review]
Phase 1 - XP Classic using only CSS

>-  background-color: Menu;
>   min-width: 1px;
>+  background: Menu;
Why this change?