User-Agent: Opera/9.80 (Windows NT 5.1; U; en) Presto/2.2.15 Version/10.10 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.5) Gecko/20091121 Thunderbird/3.0 Outlook 2007 quietly ignores the <blockquote> tag when it renders email messages. This is an issue when reading messages created in Thunderbird because the tag is used to highlight quoted text. As such, it becomes impossible to read replies made in Usenet (in-line) style. So, the easiest way to alleviate the problem is to add additional highlights to the <blockqoute> attributes. The attached patch places a new "style" attribute into quoted text based on the existence of this preference: mail.reply_quote_style The net result is something like this: <blockquote cite=3D"mid:1FE1BE61C474674F8....." type=3D"cite" style="color: darkblue; background-color: #F0F0F0; border-left : 3px dotted #000; padding-left: 10px;"> So I would say this (it's also based on some of our discussions): blah blah blah </blockquote> This shows a reasonable highlight in Outlook, even though the actual bar and space on the left are eaten up by the app. Reproducible: Always Steps to Reproduce: 1. Reply to any message 2. compose a reply in HTML

This patch was made in comm-central/mozilla against whatever "trunk" is called in Mercurial :)

The code builds and runs correctly on Win32. I have no means of making a Linux build, and hope that everything will continue to work. Unfortunately the change belongs "libeditor" as opposed to "mailnews"... Could someone review please?

The patch applies correctly to the "v3" branch and works well - tested as v3.0.2pre.

Bumped severity. This issue impacts Outlook users - they are just unable to tell the quoted text apart from inline reply.

(In reply to comment #0) > User-Agent: Opera/9.80 (Windows NT 5.1; U; en) Presto/2.2.15 > Version/10.10 > Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.5) > Gecko/20091121 Thunderbird/3.0 > > Outlook 2007 quietly ignores the <blockquote> tag when it renders email > messages. This is an issue when reading messages created in Thunderbird because > the tag is used to highlight quoted text. As such, it becomes impossible to > read replies made in Usenet (in-line) style. > > So, the easiest way to alleviate the problem is to add additional highlights to > the <blockqoute> attributes. The attached patch places a new "style" attribute > into quoted text based on the existence of this preference: > > mail.reply_quote_style > > The net result is something like this: > > <blockquote > cite=3D"mid:1FE1BE61C474674F8....." > type=3D"cite" > style="color: darkblue; background-color: #F0F0F0; border-left : 3px dotted > #000; padding-left: 10px;"> > So I would say this (it's also based on some of our discussions): blah blah > blah > </blockquote> > > This shows a reasonable highlight in Outlook, even though the actual bar and > space on the left are eaten up by the app. > Did You really test it in Outlook 2007? I do. O2007 uses Word engine to render HTML, and this engine do many things it its own way. I found that this engine will silently ignore most styles in <blockquote>, either assigned through style atrib or <style> tag. In my mails I use HTML template, heavy styled to show bloksquotes and signatures in way like TB shows them. It works fine in most MUA, even in Outlook prior 2007. But not in Outlook 2007! For myself, and my extension I still looking for a way to make Outlook 2007 render <blockquote> correctly...

Ludo, I'm not an editor module owner or peer - but Neil is.

Comment on attachment 426530 [details] [diff] [review] Patch v1 : inserts this optional new style into every quoted block > #include "nsIPrefBranch.h" > #include "nsIPrefService.h" > #include "nsIContentFilter.h" > #include "nsEventDispatcher.h" > >+#include "nsIPrefBranch.h" >+#include "nsIPrefService.h" Did you mean to duplicate these? >- res = DeleteSelectionAndCreateNode(NS_LITERAL_STRING("blockquote"), getter_AddRefs(newNode)); >+ res = DeleteSelectionAndCreateNode(NS_LITERAL_STRING("blockquote"), >+ getter_AddRefs(newNode)); If you are going to rewrap lines, please don't use tabs. >+ style = ToNewUnicode(str); ToNewUnicode allocates a string, which is probably not what you want. It doesn't even do UTF8->UTF16 conversion, although I guess CSS is mostly ASCII anyway. The documented method of getting a Unicode string from preferences is to use GetComplexValue, but if you only want to support ASCII then you should use one of the NS_Convert functions that returns a string that you can pass to SetAttribute.

(In reply to comment #5) > Did You really test it in Outlook 2007? > I do. > O2007 uses Word engine to render HTML, and this engine do many things it its > own way. I found that this engine will silently ignore most styles in > <blockquote>, either assigned through style atrib or <style> tag. > In my mails I use HTML template, heavy styled to show bloksquotes and > signatures in way like TB shows them. > It works fine in most MUA, even in Outlook prior 2007. But not in Outlook 2007! > > For myself, and my extension I still looking for a way to make Outlook 2007 > render <blockquote> correctly... Of course I tested with Outlook 2007, I have it on my machine. The bar for the blockquote is stripped but colors and indentation stays. It good enough.

Neal, thanks for the quick review. Let me address these issues and re-test. I'll submit the next patch in a couple of days.

Changes: - removed tabs - removed duplicate #includes - changed string conversion

Neal, any work on the second version of the patch? Thanks.

Sorry, I get lots of requests, and it's hard to keep up. Your patch looks OK although I haven't tested it yet. Just a note: 10 of your lines contain trailing whitespace.

Comment on attachment 429247 [details] [diff] [review] patch v2 Well, the patch appears to perform as intended. Note that you didn't manage to fix all the tabs either...

(In reply to comment #13) > Note that you didn't manage to fix all the tabs either... argh... Let me do a GUI diff and replace those tabs...

Just realized that I can hack tabs out manually :)

Hey Neil, how do I get the code in? Could you commit it into whatever dev branch is appropriate please?

You get the code checked in by adding the "checkin-needed" keyword. This might need a unit test to land though.

Hmm... it's been 6 months... Can someone commit the code please?

This needs to have a unit test before it can land on mozilla-central.

Hmm... this is a very small change that literally does two things: - gets a string from preferences - adds it to "quote" elements as a "style" attribute Is there a framework for testing such a change? Do you have an example of reply/composition coupled with attribute value verification?

(In reply to comment #20) > Hmm... this is a very small change that literally does two things: > - gets a string from preferences > - adds it to "quote" elements as a "style" attribute Sure. We require tests for all changes on mozilla-central though to make sure that future changes do not regress the existing behavior. > Is there a framework for testing such a change? Do you have an example of > reply/composition coupled with attribute value verification? You probably want to write a mochitest <https://developer.mozilla.org/en/Mochitest>. We don't have mailnews composition tests in mozilla-central, but you can issue whatever commands that you need using execCommand. Also, see the tests here for samples of tests written for other bugs <http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/html/tests/>.

OK, I think I see how to exercise the feature - it would come down to "pasting as quotation" and then verifying the attributes. However, the test framework is really something... the build is broken in comm-1.9.1; comm-1.9.2 builds but the test refuses to run; comm-central builds and runs, but I cannot figure out how to get the output from the test... Running the test comes down to this, I think: cd $(OBJDIR)/mozilla python _tests/testing/mochitest/runtests.py --autorun --close-when-done --console-level=INFO --log-file=./mochitest-plain.log --file-level=INFO --symbols-path=./dist/crashreporter-symbols --test-path=editor/libeditor/html/tests/test_bug480972.html Now, it is supposed to leave the python-based HTTP server running? If not, where does the output go? mochitest-plain.log is not even created... Thanks!

This needs tests, and the final patch needs to be approved, so it's not ready for check-in.