Last Comment Bug 579739 - Make lightweight themes / personas work in composer windows
: Make lightweight themes / personas work in composer windows
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Themes (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1b2
Assigned To: Jens Hatlak (:InvisibleSmiley)
:
Mentors:
Depends on: SM-lwtheme
Blocks:
  Show dependency treegraph
 
Reported: 2010-07-18 08:46 PDT by Robert Kaiser (not working on stability any more)
Modified: 2010-11-08 15:17 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
basic lwthemes support (1.15 KB, patch)
2010-11-02 07:03 PDT, Jens Hatlak (:InvisibleSmiley)
no flags Details | Diff | Review
basic lwthemes support v1a (2.60 KB, patch)
2010-11-03 05:21 PDT, Jens Hatlak (:InvisibleSmiley)
no flags Details | Diff | Review
basic lwthemes support v1b (4.03 KB, patch)
2010-11-03 14:49 PDT, Jens Hatlak (:InvisibleSmiley)
stefanh: review+
neil: superreview+
Details | Diff | Review
basic lwthemes support v1c [Checkin: comment 13] (6.64 KB, patch)
2010-11-04 16:57 PDT, Jens Hatlak (:InvisibleSmiley)
stefanh: review+
neil: superreview+
Details | Diff | Review

Description Robert Kaiser (not working on stability any more) 2010-07-18 08:46:36 PDT
We should also make lwthemes work in composer windows once they work in browser.
Comment 1 Jens Hatlak (:InvisibleSmiley) 2010-11-02 06:52:24 PDT
The minimum is to add these two lines to the <window> of the XUL, cf. bug 579738:
        lightweightthemes="true"
        lightweightthemesfooter="status-bar"
Comment 2 Jens Hatlak (:InvisibleSmiley) 2010-11-02 07:03:59 PDT
Created attachment 487572 [details] [diff] [review]
basic lwthemes support

I think in the case of Composer, styling the bottom tabs with text shadow is OK, so I currently see no reason to add specific styles.
Comment 3 neil@parkwaycc.co.uk 2010-11-03 04:30:27 PDT
(In reply to comment #2)
> I think in the case of Composer, styling the bottom tabs with text shadow is
> OK, so I currently see no reason to add specific styles.
But the background of the bottom tabs becomes the lightweight theme solid background colour, which isn't so helpful. (It would be nice if it could be merged with the status bar, but unfortunately the sidebar gets in the way.)
Comment 4 Jens Hatlak (:InvisibleSmiley) 2010-11-03 05:21:43 PDT
Created attachment 487878 [details] [diff] [review]
basic lwthemes support v1a

OK, so no styling for the tabs. Also found that the formatting menu should have some transparency for the sake of consistency.
Comment 5 neil@parkwaycc.co.uk 2010-11-03 14:24:12 PDT
Comment on attachment 487878 [details] [diff] [review]
basic lwthemes support v1a

>+#EditModeTabs:-moz-lwtheme {
I don't think this is the correct element; there's still a bit of custom colour leaking through. Try the EditModeToolbar instead.

>+  -moz-appearance: none;
Don't think we need this.
Comment 6 Stefan [:stefanh] 2010-11-03 14:40:45 PDT
Note that there is an editor.css in classic/mac...
Comment 7 Jens Hatlak (:InvisibleSmiley) 2010-11-03 14:49:02 PDT
Created attachment 488015 [details] [diff] [review]
basic lwthemes support v1b
Comment 8 Stefan [:stefanh] 2010-11-04 12:12:50 PDT
Comment on attachment 488015 [details] [diff] [review]
basic lwthemes support v1b

I can't seem to get Composer working in my trunk build, it just hangs (and it's not this patch that causes it of course). I do manage to get a view of the lwtheming, though.

+toolbar menulist:-moz-lwtheme /* formatting menu */ {
+  opacity: .8;
+}

Looks like you could put this in toolbar.css (also for mac ;-) ). I might re-visit the mac part since I'm undecidable on the opacity here and atm I can't test the color/bg-color thing.

Thanks!
Comment 9 Jens Hatlak (:InvisibleSmiley) 2010-11-04 16:57:21 PDT
Created attachment 488369 [details] [diff] [review]
basic lwthemes support v1c [Checkin: comment 13]

(In reply to comment #8)
> +toolbar menulist:-moz-lwtheme /* formatting menu */ {
> +  opacity: .8;
> +}
> 
> Looks like you could put this in toolbar.css (also for mac ;-) ).

Good idea; that also obsoletes part of the MailNews patch. :-)

> I might re-visit the mac part since I'm undecidable on the opacity here
> and atm I can't test the color/bg-color thing.

After the address book patch review I thought I might just as well remove the color definitions for Mac here, too. I mean, since you cannot test it and will have to revisit that anyway... If you disagree, speak up (r-).
Comment 10 neil@parkwaycc.co.uk 2010-11-04 17:13:39 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > +toolbar menulist:-moz-lwtheme /* formatting menu */ {
> > +  opacity: .8;
> > +}
> > Looks like you could put this in toolbar.css (also for mac ;-) ).
> Good idea; that also obsoletes part of the MailNews patch. :-)
What about the compose window, which has menulists in the addressing widget?
Comment 11 Jens Hatlak (:InvisibleSmiley) 2010-11-04 17:26:54 PDT
(In reply to comment #10)
> What about the compose window, which has menulists in the addressing widget?

If and when (!) this becomes an issue there we can fix it there (i.e. write a rule specific to that). I think doing the general case with a general rule and specific cases with specific rules is the way to go, and the compose window one might actually be the only exception, while we have at least two places that make use of the general rule.
Comment 12 Stefan [:stefanh] 2010-11-05 02:12:48 PDT
Comment on attachment 488369 [details] [diff] [review]
basic lwthemes support v1c [Checkin: comment 13]

I'm pretty sure the color is needed, so it's probably better to leave both color rules in.
Comment 13 Jens Hatlak (:InvisibleSmiley) 2010-11-08 15:17:03 PST
Comment on attachment 488369 [details] [diff] [review]
basic lwthemes support v1c [Checkin: comment 13]

(In reply to comment #12)
> I'm pretty sure the color is needed, so it's probably better to leave both
> color rules in.

Fixed upon checkin: http://hg.mozilla.org/comm-central/rev/c60c611cea9a

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