Closed Bug 1374708 Opened 7 years ago Closed 7 years ago

Style and color quotes in messages

Categories

(SeaMonkey :: Themes, enhancement)

enhancement
Not set
normal

Tracking

(seamonkey2.49esr fixed, seamonkey2.50 wontfix, seamonkey2.51 wontfix, seamonkey2.52 fixed, seamonkey2.53 fixed)

RESOLVED FIXED
seamonkey2.53
Tracking Status
seamonkey2.49esr --- fixed
seamonkey2.50 --- wontfix
seamonkey2.51 --- wontfix
seamonkey2.52 --- fixed
seamonkey2.53 --- fixed

People

(Reporter: frg, Assigned: frg)

References

(Blocks 1 open bug)

Details

Attachments

(4 files, 1 obsolete file)

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.
Attached patch 1374708-messagequotes.patch (obsolete) — Splinter Review
Tested on Windows only so far.
Comment on attachment 8879622 [details] [diff] [review]
1374708-messagequotes.patch

Could you check it out for Linux and OSX.
Attachment #8879622 - Flags: feedback?(stefanh)
Attachment #8879622 - Flags: feedback?(rsx11m.pub)
Comment on attachment 8879622 [details] [diff] [review]
1374708-messagequotes.patch

Ad-hoc comments:

>+++ b/suite/installer/allowed-dupes.mn
>+extensions/modern@themes.mozilla.org/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.
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 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
> 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.
"with classic" of course.
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).
Attachment #8879622 - Flags: feedback?(stefanh) → feedback+
>> 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.
Might be that someone wanted to be on the safe side when overriding in mozilla/layout/style/res/html.css.
Without the "in", of course
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.
Tested on OSX and Windows. For Windows under Modern and Classic.
Attachment #8879622 - Attachment is obsolete: true
Attachment #8879622 - Flags: feedback?(rsx11m.pub)
Attachment #8884546 - Flags: review?(stefanh)
Attachment #8884546 - Flags: review?(iann_bugzilla)
Attached image Current on Mac
OSX old
Attached image Mac with patch applied
OSX new
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.
Attachment #8884546 - Flags: review?(stefanh) → review+
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 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
Attachment #8884546 - Flags: review?(iann_bugzilla) → review+
Attached image Modern theme
For the record screenshot of the modern levels.
https://hg.mozilla.org/comm-central/rev/34a2b1f74bf9a8d1812d40fdd1521981decb4357
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.53
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
Attachment #8884546 - Flags: approval-comm-esr52?
Attachment #8884546 - Flags: approval-comm-beta?
Blocks: 1381317
Comment on attachment 8884546 [details] [diff] [review]
1374708-messagequotes-V2.patch

a=me for both beta and esr52
Attachment #8884546 - Flags: approval-comm-esr52?
Attachment #8884546 - Flags: approval-comm-esr52+
Attachment #8884546 - Flags: approval-comm-beta?
Attachment #8884546 - Flags: approval-comm-beta+
Attachment #8884547 - Attachment description: Screen Shot 2017-07-08 at 16.13.11.png → Current on Mac
Attachment #8884548 - Attachment description: Screen Shot 2017-07-08 at 16.53.45.png → Mac with patch applied
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...  ;-)
Attachment #8886837 - Attachment description: Capture.PNG → Modern theme
This is all suite/ - you want me to uplift this or you'll do it?
> 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.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: