496248 - content/multimessageview.css should not have font-specific info -- move to skin/multimessageview.css

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[CatThief]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090603 Shiretoko/3.5pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090603 Shredder/3.0b3pre

Lucida Grande is specified as the font-family for #buttonbox inside messenger/content/multimessageview.css.  It is not possible to override this font with a theme's messenger/skin/multimessageview.css regardless of how high the code goes within this object's tree structure.

Reproducible: Always




Existing code:
#buttonbox {
  font-family: Lucida Grande;
}

Attempted override:
#multimessageview > #headingwrappertable > #headingwrapper > #buttonbox {
  font-family: sans-serif !important;
}

Please remove the "font-family: Lucida Grande;" entry from messenger/content/multimessageview.css in order to allow a user's default font to appear as it does by default for other buttons.

Comment 1

[Phil Ringnalda (:philor)]

I presume that this will be fixed by Andreas' forthcoming CSS tweaks, that they'll be moving the 95% which belongs into themes/ where it belongs, and removing the 5% that doesn't belong at all?

Comment 2

[David Ascher (:davida)]

The font choice should definitely be moved to theme, sorry about that. 

I'm a bit confused by comment #0, as the cross-theme bits are included by the theme-specific files, so why can't a new theme simply not @import that file?

Phil: The intent of the current structure was to avoid code duplication across the themes, but to allow themes to override specific stuff as needed to accomodate specific platform requirements, and or ignore the shared theme (in particular for third-party themes).  What's not right with that plan?

Comment 3

[Phil Ringnalda (:philor)]

Sigh. I keep thinking I'll learn not to go off half-cocked without actually looking at the whole thing, but I never do.

That plan should work just fine, and the comment 0 override should both be working and be massively more than is needed, assuming CatThief is doing the same thing the default themes are: I'm currently punishing myself with just

+#buttonbox {
+  font-family: fantasy;
+}

at the bottom of mail/themes/pinstripe/mail/multimessageview.css, which works "perfectly" to give me hideous button text, since the content multimessageview.css is @imported by the skin one, which means rules in the skin one come last, so they only need to be of equal specificity to win. CatThief, can we see a copy of your theme, to see why it's failing for you?

(However, we really shouldn't be using an unquoted Lucida Grande without a fallback - I'd think either just specifying sans-serif, or one of the more roundabout -moz- values that winds up being just sans-serif anyway, would be a safer choice.)

Comment 4

[Alfred Kayser]

Re comment #2: This scheme is also used by songbird, and my experience there is that this is actually very frustrating for themers, as one often need to override settings that one doesn't want, with lots of !important's.
The end-result is that one needs to override so much that it is easier to just copy the default theme files, and modify those, than trying to overload the application (/content/) styles. 
Luckely, for multimessageview.css one could indeed also remove the '@import'.

Other complaint: the scrollbar is higher than the actual scrollable part, due to the display:absolute.

Comment 5

[David Ascher (:davida)]

(In reply to comment #4)
> Re comment #2: This scheme is also used by songbird, 

I doubt very much that multimessage.css is used by songbird, no?

> and my experience there is
> that this is actually very frustrating for themers, as one often need to
> override settings that one doesn't want, with lots of !important's.

I'm not sure what you mean by "this" in the "this is very frustrating".  Can you clarify?

> The end-result is that one needs to override so much that it is easier to just
> copy the default theme files, and modify those, than trying to overload the
> application (/content/) styles. 
> Luckely, for multimessageview.css one could indeed also remove the '@import'.

Right, that's on purpose =)

> Other complaint: the scrollbar is higher than the actual scrollable part, due
> to the display:absolute.

True, but a separate bug.  I may see if I can fix that w/ some overflow: magic, but let's discuss that elsewhere.

Renaming this bug to clarify the bits that I think are broken.

Comment 6

[CatThief]

(In reply to comment #3)
> That plan should work just fine, and the comment 0 override should both be
> working and be massively more than is needed, assuming CatThief is doing the
> same thing the default themes are: I'm currently punishing myself with just
> 
> +#buttonbox {
> +  font-family: fantasy;
> +}

Yes, what I specified is indeed massively more than should be needed.  I just wanted to illustrate how nothing I was trying would change the font.  The simplified code you used would not work as I began my attempt.

> CatThief, can we see a copy of your theme, to see why it's failing for you?

Certainly.  You can download it from here:
http://www.tom-cat.com/mozilla/downloads/mostlycrystal_tb_30.jar

Comment 7

[Phil Ringnalda (:philor)]

(In reply to comment #6)
> http://www.tom-cat.com/mozilla/downloads/mostlycrystal_tb_30.jar

Works for me: I installed your theme on Win7, changed my default font for sans-serif to Comic Sans so I'd be sure to know what I was getting, restarted because Gecko's bad about caching font choices, and got the archive button in a collapsed thread summary labeled in Comic Sans (yet another thing I don't really recommend, for people with weak stomachs).

(In reply to comment #4)
> Re comment #2: This scheme is also used by songbird, and my experience there is
> that this is actually very frustrating for themers, as one often need to
> override settings that one doesn't want, with lots of !important's.

Then that's not *this* scheme, or Gecko's CSS parser is grotesquely broken and nobody bothers to file bugs on it.

Once I finally actually looked at it, *this* scheme is:

* only one bit of CSS is applied to the thread summary page, your skin's multimessage.css (well, and global/skin/, of course) - anything that's there is because you've put it there, one way or another

* if you choose to do what the default themes do, and @import the content multimessage.css in your skin multimessage.css, then you do not need or want to use !important - @import is just like copy-pasting the contents into that spot, so anything you do later in the file with the exact same selector will just replace rules from the imported sheet, just like they would if you have the same selector twice in a file.

Dunno what fastload caching bug or font choice caching bug or whatever CatThief hit, but you *can* override anything in the content CSS, without needing an !important, and you're not at all required to use it and override it, if you don't want to - it's no different than if that same CSS was copy-pasted into all three default skin CSS files (other than that you'll pick up changes to it as they're made, for good or ill, without having to watch for checkins to all three skin files).

Comment 8

[Alfred Kayser]

Note, even with this method, one still has to monitor theme related  (and even theme unrelated) changes, as for example adding or remove elements (such as the .content div) can impact one's theme...

Comment 9

[CatThief]

I tested the removal of the @import entry from my theme's multimessage.css and the button font is now picking up my system default. Additional code from messenger/content/multimessage.css now has to be added in order to style other elements, but that should be simple enough.

I initially included the @import entry because that is what appears in the default theme.  I'm glad to hear it isn't needed.

Thank you all for your attention, and solution, to this issue.

Comment 10

[Pardal Freudenthal (:ShareBird)]

Sorry, but I don't agree with this approach (moving styles to content). It makes no sense for me and is against all I've believed as good programming practices (separation from content and appearance). It makes things more difficult and confusing and doesn't solve problems from third party themes (if an author doesn't track the changes, the theme will break...)

I don't want to override the rules from default theme, I want to fully substitute the default theme.

There is already a method to just override rules from the default theme with the advantage if the third party theme doesn't include for example the multimessageview.css file it won't be broke at all.
I've described this method in this article:
http://www.tudobom.de/articles/yatt/#light_weight

Comment 11

[David Ascher (:davida)]

Pardal: I don't understand your objection -- the default theme (in skin/) imports files from content, but if you substitute the default theme, your theme won't.

Comment 12

[David Ascher (:davida)]

I think we're fine here, unless I'm missing something.

Comment 13

[Pardal Freudenthal (:ShareBird)]

(In reply to comment #11)
> Pardal: I don't understand your objection -- the default theme (in skin/)
> imports files from content, but if you substitute the default theme, your theme
> won't.

David, I'm sorry for needing so long to write here...
Actually my concern has not much to do with *my* theme (it is clear to me that I don't need to import the code from content), but a matter of principle. The separation between content and appearance has demonstrated many benefits.

Unfortunately I have not found documents discussing the advantages of this approach, but I can well imagine the convenience from sharing code between the three themes, qute, pinstripe and gnomestripe. 
I even use a similar method to ensure compatibility between different operating systems, applications and versions of applications (my theme works in Firefox and Thunderbird). My experience is that this approach brings also some disadvantages. It is more difficult to debug code in this way. We can easily run into a "code salad" and lose track of issues involving the code.

Just as example, we have already some issues on multimessageview.css:
Double declaration of padding at http://mxr.mozilla.org/comm-central/source/mail/base/content/multimessageview.css#55
Value for color property declared twice at content for class .snippet: http://mxr.mozilla.org/comm-central/source/mail/base/content/multimessageview.css#115 and http://mxr.mozilla.org/comm-central/source/mail/base/content/multimessageview.css#192
A try to overwrite this declaration on the three themes with this selector: .message > .header > .snippet (e.g. http://mxr.mozilla.org/comm-central/source/mail/themes/qute/mail/multimessageview.css#18), but I'm not sure if this selector will ever fire, since .header is not a child from .message. AFAICS there is a .row between them (?)...

At a third-party perspective it will be more difficult and confusing.
Although I use to think about the job on writing themes like giving appearance to some markup (as we do on web designing), me too use to look at the default theme to begin the work. The first thing I do is to out comment the rules that "break" my theme. After this I look at the DOM for available objects and selectors to give the appearance I want.
In this specific case from multimessageview, I've "imported" the .css in content "per hand" (it means copy and paste) into the .css in skin and tried to figure out which rules I can use and which I have to delete. That's why I've ran into the issues I've described above...

Maybe I'm going paranoid and feeling a "movement" in the direction placing more styles at content. I'm really afraid one day the default theme will live at content. This will be a big PITA for theme developers.
I would like to see exactly the opposite, e.g. removing hardcoded images, using css instead of some properties like flex, direction, pack, etc...

Comment 14

[David Ascher (:davida)]

I'm in full agreement w/ the separation of content and style, assuming the technology doesn't get in the way.  This approach IMO is simply about reusing shared code. If there was another place in /skin that was useable across platforms, then I'd happily use it.

Right now, there are _huge_ numbers of messy bits in CSS because it's so hard for a single developer to make things look right on all platforms.  Keeping three CSS files that differ a bit because of e.g. font variants, but are mostly shared, in sync, is hugely error prone.  @import solves that very neatly, IMO.

Maybe that's another bug we could file, to somehow allow cross-platform CSS?

Comment 15

[Pardal Freudenthal (:ShareBird)]

In the way we are shipping themes now, I'm also not seeing other approach to share code between platforms inside the skin, since we ship several themes separately.

But, I can imagine another solution for it. I have no clue about the Mozilla's building system, so, I'm not sure if it's feasible at all. 

If we ship only *one* skin for *all* platforms, we could build a system that would have a "core" code, which would import just the files related to the specific OS.

I've described the method here:
http://forums.mozillazine.org/viewtopic.php?p=4753105#p4753105

The clue on this would be to register a "only skin" package and "switch" between different files using the manifest flags. The structure from the theme would be practically the same plus another folder containing OS specific files, I've called it os_target on my theme:

os_target
  linux
    files and images linux related
  macos
    files and images mac os related
  windows
    ....
  aero
    ...
  .
  .
  .
communicator
editor
global
.
.
.
  
The chrome manifest could be:
skin    os_target   classic/1.0   chrome/os_target/linux/     os=linux
skin    os_target   classic/1.0   chrome/os_target/macos/     os=Darwin

and so on.

A file, e.g, multiviewmessage.css inside the chrome://messenger/skin/ would have a @import like:
@import url("chrome://os_target/skin/multiviewmessage.css");

The manifest flags "switch" the desirable file...

Maybe this could be a bug to file??