1374708 - Style and color quotes in messages

Status: RESOLVED FIXED

(SeaMonkey :: Themes, enhancement)

Description

[Frank-Rainer Grahl (:frg)]

Bug 1374018 might remove some css for quotes from toolkit which is used in SeaMonkey message display and composition. Thunderbird already has messageQuotes.css for this and can easily port it then. 

We are missing it and I also notice that quotes are displayed inconsistently. The SM Mailer part will show a gray line in preview and a blue line in edit mode. TB display looks much nicer here but also has right borders which I think we shouldn't show.

Comment 1

[Frank-Rainer Grahl (:frg)]

Attachment: 1374708-messagequotes.patch

Tested on Windows only so far.

Comment 2

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8879622 [details] [diff] [review]
1374708-messagequotes.patch

Could you check it out for Linux and OSX.

Comment 3

[rsx11m]

Comment on attachment 8879622 [details] [diff] [review]
1374708-messagequotes.patch

Ad-hoc comments:

>+++ b/suite/installer/allowed-dupes.mn
>+extensions/[e-mail]/chrome/modern/skin/modern/messenger/messageQuotes.css
>+chrome/classic/skin/classic/messenger/messageQuotes.css

Why is this needed? There doesn't seem to be any messageQuotes.css other than the mail/ ones which aren't packed.

>+++ b/suite/themes/modern/messenger/messageQuotes.css
>+/* ::::: Colorize block quote borders. We only go 5 levels deep. ::::: */
>+blockquote[type=cite] {
>+  border-color: rgb(114,159,207); /* Sky Blue 1 */
>+}

I'm a bit concerned colorizing the bars in modern as well, people who use it may be less tolerant to colorful changes breaking with the traditional appearance of this theme...

Should work on Linux in theory, but I'll double-check.

Comment 4

[Stefan [:stefanh]]

Might not be able to get to this until sunday (earliest), but if it works for linux it should work for Mac (assuming you copied the thunderbird mac styles to the mac version of the file).

Comment 5

[rsx11m]

Comment on attachment 8879622 [details] [diff] [review]
1374708-messagequotes.patch

>+++ b/suite/themes/classic/mac/messenger/messageQuotes.css
Seems identical to mail/themes/osx/mail/messageQuotes.css

Comment 6

[Frank-Rainer Grahl (:frg)]

> Why is this needed? There doesn't seem to be any messageQuotes.css other than the mail/ ones which aren't packed.

messageQuotes.css for modern and classic are currently identical.

> I'm a bit concerned colorizing the bars in modern as well, people who use it may be less tolerant to colorful changes breaking with the traditional appearance of this theme...

Indifferent here. We could either use different shades of gray here or use the good old header gray for all. This would also make the file different and the dupe could be removed.

I copied the OSX files but additional changes may be needed. Didn't have time for an OSX build yesterday. I will check it myself too over the weekend but a second opinion would be welcome.

Looks really good with class in Windows 7 imho.

Comment 7

[Frank-Rainer Grahl (:frg)]

"with classic" of course.

Comment 8

[Stefan [:stefanh]]

Comment on attachment 8879622 [details] [diff] [review]
1374708-messagequotes.patch

I've only tested the "blockquote[type="cite"] and I think it's OK.

--- /dev/null
+++ b/suite/themes/classic/mac/messenger/messageQuotes.css
.
.
.

+blockquote[type=cite] {
+  color: blue !important;
+  border-color: blue !important;

Do you really need the "!important" here (and elsewhere)? You don't seem to need it in the windows file. Interestingly, looking at the css you get the impression that all borders are displayed (right, top, left, bottom) since there's no style rules setting the right, top and bottom borderwidths to 0 as in the windows file. But I only see a left border (haven't digged deeper into it but DOMi shows that only the left border has a positive width).

Comment 9

[Frank-Rainer Grahl (:frg)]

>> Do you really need the "!important" here (and elsewhere)? 

Not sure. There were a lot of default style removals in toolkit recently. Copied over from TB so will check it out.

Comment 10

[Stefan [:stefanh]]

Might be that someone wanted to be on the safe side when overriding in mozilla/layout/style/res/html.css.

Comment 11

[Stefan [:stefanh]]

Without the "in", of course

Comment 12

[Stefan [:stefanh]]

Comment on attachment 8879622 [details] [diff] [review]
1374708-messagequotes.patch

>diff --git a/suite/themes/modern/jar.mn b/suite/themes/modern/jar.mn
>--- a/suite/themes/modern/jar.mn
>+++ b/suite/themes/modern/jar.mn
>@@ -398,16 +398,17 @@ modern.jar:
>   skin/modern/messenger/filterDialog.css                           (messenger/filterDialog.css)
>   skin/modern/messenger/folderMenus.css                            (messenger/folderMenus.css)
>   skin/modern/messenger/folderPane.css                             (messenger/folderPane.css)
>   skin/modern/messenger/folderPaneExtras.css                       (messenger/folderPaneExtras.css)
>   skin/modern/messenger/threadPaneExtras.css                       (messenger/threadPaneExtras.css)
>   skin/modern/messenger/messageBody.css                            (messenger/messageBody.css)
>   skin/modern/messenger/virtualFolderListDialog.css                (messenger/virtualFolderListDialog.css)
>   skin/modern/messenger/messageHeader.css                          (messenger/messageHeader.css)
>+  skin/modern/messenger/messageQuotes.css                          (messenger/messageQuotes.css)  

Oh, and I now see that you added some whitespace at the end of the line above.

Comment 13

[Frank-Rainer Grahl (:frg)]

Attachment: 1374708-messagequotes-V2.patch

Tested on OSX and Windows. For Windows under Modern and Classic.

Comment 14

[Frank-Rainer Grahl (:frg)]

Attachment: Current on Mac

OSX old

Comment 15

[Frank-Rainer Grahl (:frg)]

Attachment: Mac with patch applied

OSX new

Comment 16

[Stefan [:stefanh]]

Comment on attachment 8884546 [details] [diff] [review]
1374708-messagequotes-V2.patch

+  skin/modern/messenger/messageQuotes.css                          (messenger/messageQuotes.css)  

r=me for the Mac parts, but please fix the whitespace at the end of the above line.

Comment 17

[Stefan [:stefanh]]

Btw, frg pointed out that we only use 3 levels on MacOS. We should probably use 5, but I think that's a separate discussion.

Comment 18

[Ian Neal]

Comment on attachment 8884546 [details] [diff] [review]
1374708-messagequotes-V2.patch

LGTM, can we have a spin off to address extra levels for OSX?
r=me

Comment 19

[Frank-Rainer Grahl (:frg)]

Attachment: Modern theme

For the record screenshot of the modern levels.

Comment 20

[Frank-Rainer Grahl (:frg)]

https://hg.mozilla.org/comm-central/rev/34a2b1f74bf9a8d1812d40fdd1521981decb4357

Comment 21

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8884546 [details] [diff] [review]
1374708-messagequotes-V2.patch

[Approval Request Comment]
Regression caused by (bug #): none
User impact if declined: quotes do not look so nice
Testing completed (on m-c, etc.): c-b c-esr52
Risk to taking this patch (and alternatives if risky): none
String changes made by this patch: none

Comment 22

[Ian Neal]

Comment on attachment 8884546 [details] [diff] [review]
1374708-messagequotes-V2.patch

a=me for both beta and esr52

Comment 23

[rsx11m]

Comment on attachment 8886837 [details]
Modern theme

Sorry, I didn't get around to eventually test this on Linux, but the fading effect while maintaining the same color as such looks nice for Modern.

I'm making the attachment descriptions a little more descriptive than just plain "Screen Shot" and "Capture" to have at least /something/ contributed...  ;-)

Comment 24

[Jorg K (CEST = GMT+2)]

This is all suite/ - you want me to uplift this or you'll do it?

Comment 25

[Frank-Rainer Grahl (:frg)]

> This is all suite/ - you want me to uplift this or you'll do it?

Thanks. Thought I let it sit a week or so in Nightly before checking it in. Have it already in my folder for it. No Beta planned and ESR is not yet ready either.

Comment 26

[Frank-Rainer Grahl (:frg)]

https://hg.mozilla.org/releases/comm-beta/rev/b3081f23868e936bf7836dc31f707c387d32081e
https://hg.mozilla.org/releases/comm-esr52/rev/ca73775434ebf5ab15ff28861ea37acedcaac3e5