581382 - Persona doesn't apply to sidebar

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[Pavel Cvrcek [:JasnaPaka]]

Attachment: SeaMonkey 2.1 with sidebar

Persona should be used on sidebars too. At this moment I just see Persona on the right border. It doesn't look well.

Comment 1

[Pavel Cvrcek [:JasnaPaka]]

Attachment: Firefox 4.0 NB with Persona on sidebar

Comment 2

[Robert Kaiser]

Attachment: fix sidebar header(s)

Here's a patch that should make sidebar look much better with personas, making the main header by styled with the persona, and removing the shadow from the individual headers.

Comment 3

[[e-mail]]

Comment on attachment 459824 [details]
Firefox 4.0 NB with Persona on sidebar

Where's firefox's splitter? I can see the splitter in the SeaMonkey screen shot, and IMHO it looks really bad with a transparent background (I don't know why toolkit does that when Personas are active). What do you think?

Comment 4

[[e-mail]]

I see now, Firefox manages to give its splitter a solid colour, but our splitter is transparent, so it shows bits of the underlying Persona. I'll file a separate bug for that.

Comment 5

[Robert Kaiser]

Well, from what I see in the attached screen shot, their splitter is grippy-less 1px-wide version, so it's pretty different. Here on Linux, I see our slitter having a slid color even with my patch applied, but the effect in the screen shot is really looking strange, yes. If that's the common Windows experience with lwthemes, I'd be happy if someone on Windows can do a patch for that, as I can't test this here.

Comment 6

[[e-mail]]

Comment on attachment 476140 [details] [diff] [review]
fix sidebar header(s)

>-#sidebar-panel-picker {
>+#sidebar-panel-picker:not(:-moz-lwtheme) {
This change isn't necessary, since the picker has to inherit the colour (rather than defaulting to -moz-dialogtext) whether or not a Persona is applied. r=me with that fixed.

Comment 7

[Stefan [:stefanh]]

Comment on attachment 476140 [details] [diff] [review]
fix sidebar header(s)

This patch is missing the mac classic file.

Comment 8

[Robert Kaiser]

(In reply to comment #7)
> Comment on attachment 476140 [details] [diff] [review]
> fix sidebar header(s)
> 
> This patch is missing the mac classic file.

Well, I know Mac sidebar styling is a bit different but I can't test if the same changes there would work. Can you try and tell me?

I still need to investigate Neil's comment #6 anyhow, as I remember I had a distinct reason for putting this on there, IIRC it actually had a wrong color in lwtheme, but I need to re-check. I'm using a dark lwtheme with light fonts for testing, that shows up font color problems. It could be that actually inheriting the light color on non-lwthemed headers is bad, but let me verify.

Comment 9

[Robert Kaiser]

(In reply to comment #6)
> Comment on attachment 476140 [details] [diff] [review]
> fix sidebar header(s)
> 
> >-#sidebar-panel-picker {
> >+#sidebar-panel-picker:not(:-moz-lwtheme) {
> This change isn't necessary, since the picker has to inherit the colour (rather
> than defaulting to -moz-dialogtext) whether or not a Persona is applied. r=me
> with that fixed.

Actually, this is necessary, or else we'd need to do our own overrides of this for hover/active states. If we exclude this from lwtheme, then an inherit is set there and correctly overridden from there to not apply on hover/active when native styling takes over again. I added a comment to make that clear.

Comment 10

[Robert Kaiser]

Attachment: fix sidebar header(s), v1.1

Re-requesting review because I both didn't address your nit (see comment) and added Mac support (untested as I have no Mac) by just porting the same changes there. If there's anything more to do, I'll leave that to Mac people in a followup.

Comment 11

[[e-mail]]

(In reply to comment #9)
> Actually, this is necessary, or else we'd need to do our own overrides of this
> for hover/active states. If we exclude this from lwtheme, then an inherit is
> set there and correctly overridden from there to not apply on hover/active when
> native styling takes over again. I added a comment to make that clear.
OMG that looks so fugly. How do Linux Firefox users cope with the fail?

Comment 12

[Stefan [:stefanh]]

Comment on attachment 477999 [details] [diff] [review]
fix sidebar header(s), v1.1

+.box-texttab:-moz-lwtheme {
+  text-shadow: none;
+}
+

On mac, there's no text-shadow to override, so please remove the above rule.

Comment 13

[Robert Kaiser]

(In reply to comment #12)
> On mac, there's no text-shadow to override, so please remove the above rule.

Interesting, I thought lwtheme applied it everywhere to ensure some contrast of fonts over background images (just that those box-texttab items have no background image). But then, that's why having someone on Mac look at it is important, you guys have done everything differently when it comes to theming, I guess ;-)

Pushed as http://hg.mozilla.org/comm-central/rev/42683c5b4db3

Comment 14

[Stefan [:stefanh]]

I was wrong about comment #12, so I filed bug 599621 (discovered another thing also).