Make lightweight themes / personas work in mailnews windows

RESOLVED FIXED in seamonkey2.1b2

Status

SeaMonkey
Themes
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Robert Kaiser, Assigned: InvisibleSmiley)

Tracking

Trunk
seamonkey2.1b2
Dependency tree / graph

SeaMonkey Tracking Flags

(seamonkey2.1 wanted)

Details

Attachments

(2 attachments, 7 obsolete attachments)

(Reporter)

Description

7 years ago
We should also make lwthemes work in mailnews windows once they work in browser.
(Assignee)

Comment 1

7 years ago
xref Bug 563101 / attachment 442999 [details] [diff] [review] / attachment 443425 [details] (personas.css; SEAMONKEY_ID case CSS part from personas.js). Requesting 2.1 status triage.
status-seamonkey2.1: --- → ?
Version: unspecified → Trunk

Updated

7 years ago
status-seamonkey2.1: ? → wanted
(Assignee)

Comment 2

7 years ago
Created attachment 487074 [details] [diff] [review]
basic lwthemes support

First I tried using Stylish to check the effects of the CSS from the bug mentioned in my previous comment. Even changing [persona] to [lwtheme="true"] made no difference. Then I found the two "lightweightthemes" attributes on the <window> in navigator.xul and the matching rules in communicator.css. After I had applied those changes it just worked without any further CSS, just had to apply the changes to a few more XUL files. :-)

The result already looks quite promising, though the header pane in the standalone message window should probably not have a background, and the same is debatable for the compose window formatting toolbar.

What do you think?

[Not taking the bug since I'm not sure I'm the best person to actually determine where changes need to be done in the MailNews UI and how to do it in CSS.]
Attachment #487074 - Flags: feedback?(mnyromyr)

Comment 3

7 years ago
Comment on attachment 487074 [details] [diff] [review]
basic lwthemes support

(In reply to comment #2)
> Created attachment 487074 [details] [diff] [review]
> basic lwthemes support

> What do you think?

Cool. :)

> The result already looks quite promising, though the header pane in the
> standalone message window should probably not have a background,

True, especially since the header pane will usually "outheight" the top image. 
BTW, the standalone window status-bar seems to be broken with your patch?

> and the same is debatable for the compose window formatting toolbar.

Yes, that looks a bit strange; same for the address toolbar - too much height, again, the Persona images are usually too narrow.
Attachment #487074 - Flags: feedback?(mnyromyr) → feedback+
(Assignee)

Comment 4

7 years ago
Created attachment 487190 [details] [diff] [review]
patch v2
Attachment #487190 - Flags: superreview?
Attachment #487190 - Flags: review?(mnyromyr)
(Assignee)

Updated

7 years ago
Attachment #487190 - Flags: superreview? → superreview?(neil)
(Assignee)

Updated

7 years ago
Attachment #487074 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Assignee: nobody → jh
Status: NEW → ASSIGNED
(Assignee)

Comment 5

7 years ago
(In reply to comment #3)
> BTW, the standalone window status-bar seems to be broken with your patch?

Yeah, somehow the lightweightthemesfooter attribute breaks it. Removing it didn't seem to make any difference other than unbreaking it. :-/

BTW: TB (mail/) seems to add the attributes to messenger.xul.

You may want to cross-check the TB styles and those from bug 563101.

Comment 6

7 years ago
Comment on attachment 487190 [details] [diff] [review]
patch v2

>+        lightweightthemes="true"
>         windowtype="mail:messageWindow">
The standalone message window still has a status bar...

>+#msgHeaderView:-moz-lwtheme,
>+#messengerWindow[lwtheme="true"] treecols {
Everyone needs to be using :-moz-lwtheme now.
(sadly we have one [lwtheme="true"] hanging around...)
Attachment #487190 - Flags: superreview?(neil) → superreview-
(Reporter)

Comment 7

7 years ago
Right, :-moz-lwtheme should work on all elements, AFAIK, so we should use that as it should be faster than walking the cascade up to the window.
(Assignee)

Comment 8

7 years ago
Created attachment 487328 [details] [diff] [review]
patch v2a

(In reply to comment #6)
> >+        lightweightthemes="true"
> >         windowtype="mail:messageWindow">
> The standalone message window still has a status bar...

So? It has one with or without the patch. If you're referring to the lightweightthemesfooter attribute, see comment 5. I have no intention to check what's going wrong there; nothing suspicious in the JS console and without the attribute everything is right as far as I can see.

If anyone wants to check, the above attribute seems to only be checked by this code:
http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/content/LightweightThemeConsumer.jsm#90
Attachment #487190 - Attachment is obsolete: true
Attachment #487328 - Flags: superreview?(neil)
Attachment #487328 - Flags: review?(mnyromyr)
Attachment #487190 - Flags: review?(mnyromyr)

Comment 9

7 years ago
(In reply to comment #8)
> I have no intention to check what's going wrong there
Well fortunately for you I did check, and it's the mMsgNotificationBar which tries to retrieve the element at an unsafe time (i.e. while the document is loading). I'm not seeing the lightweight theme on the main 3 pane status bar though; it's not the same cause, so as yet I'm not sure what the issue is.
(In reply to comment #9)
> (In reply to comment #8)
> > I have no intention to check what's going wrong there
> Well fortunately for you I did check, and it's the mMsgNotificationBar which
> tries to retrieve the element at an unsafe time (i.e. while the document is
> loading). I'm not seeing the lightweight theme on the main 3 pane status bar
> though; it's not the same cause, so as yet I'm not sure what the issue is.
In this case it's gMessengerBundle that's the culprit. Similarly in the compose window it's sComposeMsgsBundle and sBrandBundle.

Updated

7 years ago
Attachment #487328 - Flags: superreview?(neil) → superreview-
(Assignee)

Updated

7 years ago
Attachment #487328 - Flags: review?(mnyromyr)
(Assignee)

Comment 11

7 years ago
(In reply to comment #10)
> In this case it's gMessengerBundle that's the culprit. Similarly in the compose
> window it's sComposeMsgsBundle and sBrandBundle.

Thanks for the analysis, but that's above my head. Feel free to take.
Assignee: jh → nobody
Status: ASSIGNED → NEW

Comment 12

7 years ago
+#searchInput:-moz-lwtheme,
+.tabmail-tab:-moz-lwtheme,
+toolbar menulist:-moz-lwtheme /* View and Folder menus */ {
+  opacity: .8;
+}

I think you might want to put the tab styling in mailWindow1.css since that's the only file where tab styles are. Also, this doesn't handle the mac part.
(Assignee)

Comment 13

7 years ago
OK, so I basically already threw the towel, and the reason for that hasn't changed, but I can offer to add lwthemes support for the main window only. IIUC, comment 10 implies that the main window is not affected, and it's also the only window that TB is currently styling. It's not the full solution, but better than nothing. The rest could either be done in this bug or a follow-up.
(In reply to comment #13)
> IIUC, comment 10 implies that the main window is not affected
No, I see that all mail windows are affected. I'll file a separate bug on avoiding accessing DOM elements before the document loads.

Also, the Advanced button needs to be given some opacity. (The search and go buttons in the browser window have their own rules for this but it might be worthwhile creating a shared rule to cover this.)

Updated

7 years ago
Depends on: 609877
(In reply to comment #9)
> (In reply to comment #8)
> > I have no intention to check what's going wrong there
> Well fortunately for you I did check, and it's the mMsgNotificationBar
Speaking of which, it needs to have its text shadow and colours fixed ;-)
(Assignee)

Comment 16

7 years ago
Created attachment 488583 [details] [diff] [review]
patch v3

Since Neil was kind enough to address the blocker, let's move on. Notes:
- patch is on top of the one from bug 579739, which I'll check in once the tree reopens
- .msgNotificationBar already sets colors -> no need for an override
- this is basically just another lwthemes bug, so I'll skip Karsten's review here and continue with the Stefan/Neil dream team
- of course this can only land once bug 609877 is ready.
Assignee: nobody → jh
Attachment #487328 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #488583 - Flags: superreview?(neil)
Attachment #488583 - Flags: review?(stefanh)
Comment on attachment 488583 [details] [diff] [review]
patch v3

>+toolbar menulist:-moz-lwtheme,
>+toolbar button:-moz-lwtheme {
>   opacity: 0.8;
Nit: can remove rules from navigator.css now.

>+.tabmail-tab:-moz-lwtheme {
>+  opacity: 0.8;
Except for the selected tab (see navigator.css).

>+  -moz-appearance: none;
I don't think we need this anywhere.

>+#searchInput:-moz-lwtheme {
>+  opacity: .8;
Except when it's focused. (More fuel for communicator.css? Also, should we remove opacity from focused menulists?)

sr=me with these fixed.
Attachment #488583 - Flags: superreview?(neil) → superreview+
(Assignee)

Comment 18

7 years ago
Created attachment 488983 [details] [diff] [review]
patch v3a
Attachment #488583 - Attachment is obsolete: true
Attachment #488983 - Flags: superreview+
Attachment #488983 - Flags: review?(stefanh)
Attachment #488583 - Flags: review?(stefanh)

Comment 19

7 years ago
Comment on attachment 488983 [details] [diff] [review]
patch v3a

--- a/suite/themes/classic/mac/communicator/communicator.css
+++ b/suite/themes/classic/mac/communicator/communicator.css
@@ -105,12 +105,13 @@ grippy {
 
 .sidebarTree {
   border: none;
   margin: 0px !important;
 }
 
 /* ::::: lightweight themes ::::: */
 
-toolbar menulist:-moz-lwtheme {
+toolbar button:-moz-lwtheme,
+toolbar menulist:-moz-lwtheme:not([open="true"]),
+toolbar textbox:-moz-lwtheme:not([focused="true"]) {
   opacity: 0.8;
 }

After some though, I think we only want the textbook styling here (buttons never had it on mac, and the menulist styling looks a bit weird (might re-visit that, but my current opinion is that the styling makes it look 50% disabled). Note that it should be 0.9 for the textbox (as it was for the urlbar!)

+#MsgHeadersToolbar menulist:-moz-lwtheme,
+#FormatToolbar menulist:-moz-lwtheme {
+  opacity: 1;
+}
The above means that you can get rid of this in the mac version of messengercompose.css ;-)

One question: Do we plan to make users be able to move them to the main toolbar and/or put other menulists in those toolbars? The reason I ask is that the above descendant selectors are the most expensive style rules you can have and you only want these if there are no other possibilities. Just asking. 

--- a/suite/themes/classic/mac/messenger/mailWindow1.css
+++ b/suite/themes/classic/mac/messenger/mailWindow1.css
@@ -236,8 +236,16 @@
 .tabmail-tab[type="message"] .tab-icon {
   margin-top: -2px;
 }
 
 .tabmail-tab[type="folder"][NewMessages="true"],
 .tabmail-tab[type="folder"][IsServer="true"] {
   font-weight: bold;
 }
+
+.tabmail-tab:-moz-lwtheme:not([selected="true"]) {
+  opacity: 0.8;
+}
+
+.tabmail-tab:-moz-lwtheme {
+  text-shadow: none;
+}

Don't need/want this. Most stuff is taken care of in tabbrowser.css and we also don't remove the shadow in tabbrowser, so I think we're fine having it in tabmail.

The current patch lacks some style rules in the mac version of mailWindow1.css to make it complete - for example, right now, the search bar and the tabmail-strip isn't lw-themed. IOTW:

1) You need to get rid of the background-image/color in #searchToolbar ("background-image: none", and "background-color: transparent" will do fine)
2) The .tabmail-strip needs a transparent backgrund
3) the #messagepane border is transparent and the #messagepanebox gets the lwtheme image as a background, so it shines through to the border. I'd suggest just setting a -moz-dialog background to the messagepanebox.

Minusing, mainly because I think it's too much to fix upon check-in.
Attachment #488983 - Flags: review?(stefanh) → review-

Comment 20

7 years ago
Comment on attachment 488983 [details] [diff] [review]
patch v3a

+toolbar textbox:-moz-lwtheme:not([focused="true"]) 

forgot to mention that i guess this should make the addressbook searchbox styling obsolete.
(Assignee)

Comment 21

7 years ago
Created attachment 489316 [details] [diff] [review]
patch v3b [Checkin: comment 22]

(Needless to say that the Mac changes are totally untested by me. I depend on you telling me exactly what you need; I hope I got it about right.)

(In reply to comment #19)
> One question: Do we plan to make users be able to move them to the main toolbar
> and/or put other menulists in those toolbars? The reason I ask is that the
> above descendant selectors are the most expensive style rules you can have and
> you only want these if there are no other possibilities. Just asking. 

Bug 606683 will make the formatting toolbar customizable, so you'll then be able to drag the formatting drop-down to some other customizable toolbar.

(In reply to comment #20)
> forgot to mention that i guess this should make the addressbook searchbox
> styling obsolete.

Good catch. I'm assuming Neil is OK with that, so I'm carrying over his sr+.
Attachment #488983 - Attachment is obsolete: true
Attachment #489316 - Flags: superreview+
Attachment #489316 - Flags: review?(stefanh)

Updated

7 years ago
Attachment #489316 - Flags: review?(stefanh) → review+
(Assignee)

Comment 22

7 years ago
Comment on attachment 489316 [details] [diff] [review]
patch v3b [Checkin: comment 22]

http://hg.mozilla.org/comm-central/rev/cc4bf8588a7e
Attachment #489316 - Attachment description: patch v3b → patch v3b [Checkin: comment 22]
(Assignee)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b2
(In reply to comment #17)
> Also, should we remove opacity from focused menulists?
I noticed you went for open menulists. An interesting choice, but I'm not 100% happy with it. I'll have to see whether I get used to it.
The Compose window Subject box needs an override :-(
(Assignee)

Comment 25

7 years ago
Created attachment 490402 [details] [diff] [review]
fix compose subject opacity
Attachment #490402 - Flags: superreview?(neil)
Attachment #490402 - Flags: review?(neil)
Bah, I failed to notice that the addressing widget is also affected :-(
(Assignee)

Comment 27

7 years ago
Created attachment 490449 [details] [diff] [review]
fix compose opacity
Attachment #490402 - Attachment is obsolete: true
Attachment #490449 - Flags: superreview?(neil)
Attachment #490449 - Flags: review?(neil)
Attachment #490402 - Flags: superreview?(neil)
Attachment #490402 - Flags: review?(neil)
Comment on attachment 490449 [details] [diff] [review]
fix compose opacity

>+#msgSubject:-moz-lwtheme,
>+#MsgHeadersToolbar textbox:-moz-lwtheme,
Doesn't the subject field live in the message header toolbar anyway?
(Assignee)

Comment 29

7 years ago
Created attachment 490450 [details] [diff] [review]
fix compose opacity v1a [Checkin: comment 30]
Attachment #490449 - Attachment is obsolete: true
Attachment #490450 - Flags: superreview?(neil)
Attachment #490450 - Flags: review?(neil)
Attachment #490449 - Flags: superreview?(neil)
Attachment #490449 - Flags: review?(neil)

Updated

7 years ago
Attachment #490450 - Flags: superreview?(neil)
Attachment #490450 - Flags: superreview+
Attachment #490450 - Flags: review?(neil)
Attachment #490450 - Flags: review+
(Assignee)

Comment 30

7 years ago
Comment on attachment 490450 [details] [diff] [review]
fix compose opacity v1a [Checkin: comment 30]

http://hg.mozilla.org/comm-central/rev/e43941867366
Attachment #490450 - Attachment description: fix compose opacity v1a → fix compose opacity v1a [Checkin: comment 30]

Updated

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