Last Comment Bug 855135 - Change link and signature colors in themes from hard-coded definitions to follow preferences
: Change link and signature colors in themes from hard-coded definitions to fol...
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Theme (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 23.0
Assigned To: rsx11m
:
Mentors:
Depends on: 845807 917906 920997 929006
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-26 17:14 PDT by rsx11m
Modified: 2013-10-29 13:09 PDT (History)
2 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
unaffected


Attachments
Proposed patch (2.93 KB, patch)
2013-03-27 07:25 PDT, rsx11m
bwinton: review+
bwinton: ui‑review+
standard8: approval‑comm‑aurora+
Details | Diff | Splinter Review
Comparison on Windows 7 (3.50 KB, image/png)
2013-03-28 16:59 PDT, rsx11m
no flags Details

Description rsx11m 2013-03-26 17:14:24 PDT
Now that bug 845807 has made the link colors and other color options (like using system colors or using/overriding colors defined by the document itself) available in the preferences UI, any hard-coded instances should be removed from the themes so that they actually follow the preference values. This specifically affects the display of plain-text messages and HTML messages which do not 

On Linux, all links are hard-wired to a light blue by a generic rule in http://mxr.mozilla.org/comm-central/source/mail/themes/linux/mail/messageBody.css#37

> /* ::::: message text, incl. quotes ::::: */
> 
> a {
>   color: rgb(32,74,135); /* Sky Blue 3 */
> }

Mac OSX hard-wires only signature text and link colors to gray and light blue, http://mxr.mozilla.org/comm-central/source/mail/themes/osx/mail/messageBody.css#77

> /* ::::: signature ::::: */
> 
> .moz-txt-sig,
> .moz-signature {
>   color: #505050;
> }
> 
> .moz-txt-sig > a,
> .moz-signature > a {
>   color: #7777FF; /* light blue */
> }

I didn't find any link-related rules in the windows/ theme.
Comment 1 Blake Winton (:bwinton) (:☕️) 2013-03-26 17:26:01 PDT
For the signature text and links in particular, I think it still makes sense to de-emphasize them a little.

Having said that, I would be happy with changing the Mac block to something like:
> > /* ::::: signature ::::: */
> > 
> > .moz-txt-sig,
> > .moz-signature {
> >   opacity: 0.6;
> > }
> > 
> > .moz-txt-sig > a,
> > .moz-signature > a {
> >   /* delete this rule */
> > }

(0.6 seems to get it close to the current values to me…)
Comment 2 rsx11m 2013-03-26 17:33:26 PDT
I just started to wonder how comes that the windows/ version gets away with not defining any specific font color for signatures here in the messageBody.css file, but then found that they are defined in the quoted content, http://mxr.mozilla.org/comm-central/source/mail/themes/windows/mail/messageQuotes.css#18

> .moz-txt-sig,
> .moz-signature {
>   color: rgb(136,138,133); /* Aluminum 4 */
> }
> 
> .moz-txt-sig > a,
> .moz-signature > a {
>   color: rgb(114,159,207); /* Sky Blue 1 */
> }

Since those are not restricted to blockquote constructs they should also apply to the main text. Similar definitions can also be found for the linux/ theme.
Comment 3 rsx11m 2013-03-26 19:26:56 PDT
Ok, so the reason that the signature is defined in messageQuotes.css for Linux and Windows but not for Mac OSX is that gMsgEditorCreationObserver uses it in the observe function to style the signature during message composition. For some reason, Mac OSX doesn't have that file and defines it in messageBody.css instead.

These should be enough pointers to come up with a patch, I'll give it a try but can only verify it for Linux and Windows, not Mac OSX (don't have one around).
Comment 4 rsx11m 2013-03-26 20:46:08 PDT
Looks good on Windows already, especially with the background color changed to something different than white. Basing the signature on opacity rather than that greyish font really helps in this case, and custom-colored links show up nicely.

I'll test the Linux version tomorrow and should have a patch up for your review after that, unless I run into any unexpected issues.
Comment 5 rsx11m 2013-03-27 07:25:38 PDT
Created attachment 730150 [details] [diff] [review]
Proposed patch

Here the patch for all platforms. This works fine on Linux as well, now the preference colors are obeyed without the "Allow content to choose its own colors" box unchecked, and the signature has opacity applied rather than a fixed color to also consider custom text colors.

Tested with both plain-text and HTML messages, including the signature on message composition (which doesn't use the display text and background colors though, but that's a different issue). In Simple HTML, the user-defined anchor color is chosen; in Original HTML, color specifications in the document are observed.

As said, I'm unable to test this on Mac OSX, but don't see a reason why it shouldn't work in a similar way.
Comment 6 rsx11m 2013-03-27 07:33:38 PDT
Note that you shouldn't see any opacity applied to the signature in message composition on Mac OSX given that it doesn't have a messageQuotes.css file (I don't see any bug report pending to add it, thus I can't tell if it was on purpose or by omission). The comment in the composition  code points to bug 517919 and bug 516322 for related discussion.
Comment 7 rsx11m 2013-03-28 16:59:10 PDT
Created attachment 730977 [details]
Comparison on Windows 7

Interesting, I assume that you determined this from Mac OSX?

The signature text color there is #505050 [rgb(114,159,207)] and #7777ff [rgb(119,119,255)] for the signature. In contrast, Windows and Linux define rgb(136,138,133) for the text and rgb(114,159,207) for links in signatures, thus are brighter than on Mac OSX for whatever reason.

Now, an opacity of 0.6 corresponds to rgb(101, 101, 101) for black, and 0.5 is rgb(127, 127, 127). As the latter is closer to the current shade (and that's what I've ended up using in bug 855684 for the SeaMonkey version of this patch, given that they currently use a matching 50% gray for the signature text), I've tried it as shown in this screenshot comparison. However, the link color is now lighter in Thunderbird (but not in SeaMonkey), thus it looks too pale now despite the gray text approximately matching the current shade.

Thus, unless you tell me to switch the opacity to 0.5 for Windows and Linux, I'd stick with 0.6 for all platforms given that it looks better for the in-signature links.
Comment 8 rsx11m 2013-03-28 17:03:26 PDT
(In reply to rsx11m from comment #7)
> Interesting, I assume that you determined this from Mac OSX?

That refers to:

(In reply to Blake Winton (:bwinton) from comment #1)
> (0.6 seems to get it close to the current values to me…)
Comment 9 Blake Winton (:bwinton) (:☕️) 2013-04-01 10:23:49 PDT
Comment on attachment 730150 [details] [diff] [review]
Proposed patch

Seems pretty nice on Mac.  ui-r=me.

>+++ b/mail/themes/linux/mail/messageBody.css
>@@ -29,20 +29,16 @@ mailattachcount {
> /* ::::: message text, incl. quotes ::::: */
> 
>-a {
>-  color: rgb(32,74,135); /* Sky Blue 3 */
>-}
>-

I guess my only review-question for this patch is "Why did you remove this line?"

r=me with that answered.  (I'm guessing it's to let the user's chosen colour show, but I wanted to double-check that.)

Thanks,
Blake.
Comment 10 rsx11m 2013-04-01 13:06:06 PDT
(In reply to Blake Winton (:bwinton) from comment #9)
> I guess my only review-question for this patch is "Why did you remove this line?"

That's how I've noticed that something was wrong in the first place. On Linux, that rule currently overrides browser.anchor_color, thus a change there doesn't affect anything at all. I'd assume that this was introduced initially for the same reason the default link color was changed in bug 845807, to make the blue a bit lighter. This has been taken care of now by globally changing the default, thus the rule can go for that reason as well.
Comment 11 rsx11m 2013-04-01 16:34:56 PDT
Comment on attachment 730150 [details] [diff] [review]
Proposed patch

Too bad, this has just missed the merge, thus requesting comm-aurora approval.

[Approval Request Comment]
Regression caused by (bug #): issue exposed by bug 845807
User impact if declined: fix for bug 845807 won't work on Linux and for signature links on all platforms
Testing completed (on c-c, etc.): developed and tested in 22.0 
Risk to taking this patch (and alternatives if risky): no impact on function, style changes only
Comment 12 Ryan VanderMeulen [:RyanVM] 2013-04-02 07:19:24 PDT
https://hg.mozilla.org/comm-central/rev/4ad03358510a
Comment 13 Mark Banner (:standard8) 2013-04-09 02:13:08 PDT
https://hg.mozilla.org/releases/comm-aurora/rev/085e380d8983

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