Closed Bug 136502 Opened 22 years ago Closed 9 years ago

Provide prefs. setting to turn off Options > Delivery Format > Auto-Detect (especially to avoid Auto-Downgrade mystery/surprise)

Categories

(MailNews Core :: Composition, enhancement)

enhancement
Not set
normal

Tracking

(thunderbird45 fixed)

RESOLVED FIXED
Thunderbird 45.0
Tracking Status
thunderbird45 --- fixed

People

(Reporter: dave, Assigned: thomas8)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [GS] Workaround < TB45: https://addons.mozilla.org/de/thunderbird/addon/always-html/)

Attachments

(3 files, 8 obsolete files)

840 bytes, patch
BenB
: review+
Details | Diff | Splinter Review
16.19 KB, image/png
Details
9.13 KB, patch
mkmelin
: review+
Details | Diff | Splinter Review
Mozilla 0.9.9 seems to have a feature where it automatically downconverts email
and sends it in "text only" mode if it does not detect any formatting (such as
bold, colours etc.). There is no apparent way to disable this feature.

It would be better if the Formatting menu had an option to turn this behaviour
off and **always** send HTML (e.g. always send multipart) c.f. the default
behaviour in Netscape, Outlook, Notes, Eudora, .... - since the vast majority of
recipients are likely to have an HTML-aware client it would be nice to send HTML
so that the message appears in a proportional font, etc.
QA Contact: gayatri → esther
Select the account settings and mark "compose messages in HTML format".
WFM.
I tried this again with Mozilla 1.1a / Linux (Talkback build) and it's still
doing it ... here's the raw copy of the style of emails it's actually sending,
which I will also forward to Alfonso. Both of these were written by hitting
"Compose" and just typing the text, then selecting the word "bold" and changing
it to bold.


--- First test - no formatting -----------------------------------------------

From - Sun Jul 21 12:30:56 2002
X-UIDL: 3d3aefd000000001
X-Mozilla-Status: 0001
X-Mozilla-Status2: 00000000
Return-Path: <dave@convio.com>
Received: from convio.com (alba.convio.com [63.91.81.6])
	by dcc.vu (8.11.6/8.9.3) with ESMTP id g6LHUP427634
	for <dave@dcc.vu>; Sun, 21 Jul 2002 12:30:25 -0500
Received: (from dave@localhost)
	by convio.com (8.9.3/8.9.3) id MAA05581
	for dave@dcc.vu; Sun, 21 Jul 2002 12:44:05 -0500
Received: from dcc.vu (cs242730-185.austin.rr.com [24.27.30.185])
	by convio.com (8.9.3/8.9.3) with ESMTP id MAA05577
	for <dave@convio.com>; Sun, 21 Jul 2002 12:44:04 -0500
Received: from convio.com (localhost [127.0.0.1])
	by dcc.vu (8.11.6/8.9.3) with ESMTP id g6LHUN427630
	for <dave@convio.com>; Sun, 21 Jul 2002 12:30:23 -0500
Message-ID: <3D3AEFAE.9090601@convio.com>
Date: Sun, 21 Jul 2002 12:30:22 -0500
From: David Crooke <dave@convio.com>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1a) Gecko/20020610
X-Accept-Language: en-us, en
MIME-Version: 1.0
Subject: Mozilla HTML Downconvert Test 1
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
Status:   


This email is being written in Mozilla's HTML email composer. The 
setting "Compose messages in HTML format" is active. Since it contains 
no overt formatting, Mozilla will downconvert it and send only plain text.




---- Second test; just one pair of <B></B> tags ------------------------------

From - Sun Jul 21 12:34:16 2002
X-UIDL: 3d3af09800000001
X-Mozilla-Status: 0001
X-Mozilla-Status2: 00000000
Return-Path: <dave@convio.com>
Received: from convio.com (alba.convio.com [63.91.81.6])
	by dcc.vu (8.11.6/8.9.3) with ESMTP id g6LHYF427644
	for <dave@dcc.vu>; Sun, 21 Jul 2002 12:34:15 -0500
Received: (from dave@localhost)
	by convio.com (8.9.3/8.9.3) id MAA05679
	for dave@dcc.vu; Sun, 21 Jul 2002 12:47:55 -0500
Received: from dcc.vu (cs242730-185.austin.rr.com [24.27.30.185])
	by convio.com (8.9.3/8.9.3) with ESMTP id MAA05675
	for <dave@convio.com>; Sun, 21 Jul 2002 12:47:54 -0500
Received: from convio.com (localhost [127.0.0.1])
	by dcc.vu (8.11.6/8.9.3) with ESMTP id g6LHYD427640
	for <dave@convio.com>; Sun, 21 Jul 2002 12:34:13 -0500
Message-ID: <3D3AF094.1020403@convio.com>
Date: Sun, 21 Jul 2002 12:34:12 -0500
From: David Crooke <dave@convio.com>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1a) Gecko/20020610
X-Accept-Language: en-us, en
MIME-Version: 1.0
To: dave@convio.com
Subject: Mozilla HTML Downconvert Test 2
Content-Type: multipart/alternative;
 boundary="------------070209050400030204000700"
Status:   


--------------070209050400030204000700
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit


This email is being written in Mozilla's HTML email composer. The 
setting "Compose messages in HTML format" is active. Since it contains a 
single word in *bold*, Mozilla will send it in MIME multipart with both 
plain text and HTML.



--------------070209050400030204000700
Content-Type: text/html; charset=us-ascii
Content-Transfer-Encoding: 7bit

<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
  <title></title>
</head>
<body>
<br>
This email is being written in Mozilla's HTML email composer. The setting
"Compose messages in HTML format" is active. Since it contains a single word
in <b>bold</b>, Mozilla will send it in MIME multipart with both plain text
and HTML.<br>
<br>
<br>
</body>
</html>

--------------070209050400030204000700--



 
Alfonso pointed out a setting in the Email Compose dialog "Options -> Format ->
Auto-Detect" - changing this to "Plain and Rich (HTML) Text" disables the
downconvert. However this has to be done for every message and it appears there
is no way to default to the latter behaviour.

Amending summary accordingly.
Summary: Provide option to *not* down-convert email to plain text → Provide prefs. setting to turn off Options -> Format -> Auto-Detect
Please change Hardware and OS to All.

Prog.
Shosh, could you please confirm this bug?

This is the bug that causes Hebrew HTML emails with dir="rtl" to be converted to
plain text (which in turn get displayed as LTR).

Thanks,

Prog.

PS.
For what it's worth, I'm currently using the following workaround:
1. Add HTML formatting (an italicized space, a horizontal rule and so on...)
2. Save as Template.
3. Compose future email using that template.
And if I didn't mention it, this bug effects both Thunderbird 0.4 and Mozilla
Mail&News 1.6b.

Prog.
prog, what element is the dir= attribute assigned to?
Assignee: mscott → mscott
Status: UNCONFIRMED → NEW
Ever confirmed: true
Some further info ...

1. The behaviour I opened the initial BZ about has not changed at least up to
Mozilla 1.4 (Gecko 20030624) which I just reproduced it against.

2. I was not aware it was actively breaking content in non-8859 langauges, I was
looking at it merely as an aesthetic control issue.

3. I think the auto-downconvert is a good feature for e.g. people on dialup, but
I'd prefer not to have it, and I'm not alone, so it would be nice to have a way
to permanently run it off (rather than having to do so manually for each
individual email).

4. There is (now) a partial workaround - if a domain is explicitly specified as
an "HTML domain" under Edit -> Preferences -> Send Format then this behaviour
does not occur. This is suitable for e.g. making sure intra-company email always
goes in multipart (I have our own domain in mine) but is not a general solution.
Prog, do you want to spin off a new bug for this dir=rtl issue? I'll attach
this patch there and check it in under your new bug.

This fix is really orthogonal to what's being asked for in this bug.
Scott, I just opened Bug 228720 for the direction issue.

Your patch is going to make Mozilla a much bigger contender to Outlook and
Outlook Express among BiDi users. Until now, most Hebrew users didn't consider
Mozilla as a serious alternative, but with some advocacy and lobbying, this is
now something that will be easier to change :-)

Thanks again!

Prog.
the fix for prog's issue has been checked into 1.7a
Comment on attachment 137499 [details] [diff] [review]
Fix for Bug 228720 (see Prognathous' comment 5 here): Don't down-convert msg if dir attribute set [checked in]

First, the formalities: this patch has r/sr=bienvenu under Bug 228720

I request this fix to land in 1.6, as it makes Mozilla Mail&News a much more
effective and competitive mail client for BiDi users. No longer do the
recipients of the mail have to face wrong text alignment and directionality.

As for risks of regressions, they are very small. I have tested a 1.6-20040101
build with this patch applied (and another, unrelated patch for bug 162242).
The results were excellent, under WinXP, Win98, and Win2k (the latter was
tested by two other users). Anyone can download this build at
http://www.xslf.com/mozilla_bin.exe

In all cases, Mail prompted the user to choose a format, allowing HTML or
HTML+Plain Text mail to be sent, in turn solving the direction and alignment
problems that so many BiDi users suffer when plain-text Hebrew or Arabic emails
are opened in common mail clients.

Prog.
Attachment #137499 - Flags: approval1.6?
Comment on attachment 137499 [details] [diff] [review]
Fix for Bug 228720 (see Prognathous' comment 5 here): Don't down-convert msg if dir attribute set [checked in]

a=chofmann for 1.6 but it needs to get on the branch quickly.
Attachment #137499 - Flags: approval1.6? → approval1.6+
the patch is now in the 1.6 branch
Comment on attachment 137499 [details] [diff] [review]
Fix for Bug 228720 (see Prognathous' comment 5 here): Don't down-convert msg if dir attribute set [checked in]

for clarity I'm clearing the 1.6 flag on this bug and putting it in Bug #228720
which is specifically for the BIDI issue and has been marked as fixed.
Attachment #137499 - Flags: approval1.6+
*** Bug 157346 has been marked as a duplicate of this bug. ***
What do we do with the people who want to send HTML whenever they use
formatting, without being bothered by a dialog, and plaintext otherwise?
Answering my own question: See bug 44494.
Product: MailNews → Core
QA Contact: esther → backend
Assignee: mscott → nobody
Product: Core → MailNews Core
Any chance this could get fixed for 3.0?  I run into this issue a lot.  It doesn't just mess up formatting in my reply.  It messes up the whole email.

Is anyone with enough privileges to modify this bug monitoring this thing?  Could someone please respond to Comment 4, as this is not a platform-dependent bug.

Probably can't get blocking on this, but worth a try. 8^)

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090201 Lightning/1.0pre
Flags: blocking1.9.1?
> It messes up the whole email.

I don't think it can. We always ask during send, if there's any formatting. Even if it's just bold and only in the quote. I just tested it again to make sure. The only case when we don't ask for HTML is if there's no formatting at all.
OS: Linux → Other
Hardware: x86 → All
> (comment #20) We always ask during send, if there's any formatting.

Actually, we don't: Since bug 466674, the message is silently sent as plain-text plus HTML as new default, thus there is no indication whether or not auto-detect considers it "plain enough" to send without HTML part.
Correction per bug 466674 comment #13, this was only changed for Thunderbird. Nevertheless, it would be good to have an off switch for auto-detect to allow the advanced user to decide explicitly how to send out the message...
(In reply to comment #20)
> We always ask during send, if there's any formatting. Even if it's just bold and only in the quote.

By the way, that prompt on sending has a huge annoyance factor--at least for me-- because the detection is doesn't work well and shows up even when replying to messages that were clearly formatted HTML.  I ended up going to Tools --> Options... --> Composition --> Send Options... and setting "Send the message in HTML anyways."

That said, I just tested the sending and it turns out the message was sent with the right font but was messed up on-screen when clicking reply. I'm now starting to think that this auto-detection code is the root cause of Shredder's myriad font display issues.
> I'm now starting to think that this auto-detection code is the root cause
> of Shredder's myriad font display issues.

Please don't guess. The auto-detection code simply looks at which HTML tags are used. If fonts are used, HTML is offered. The detect code doesn't doesn't change any fonts.


I think this is simply a dup of bug 44494.
Nope, it isn't. bug 44494 comment 0 makes it sound like in 2000, there was a pref to do what this bug wants, which is to send "Hi." in HTML or HTML+text, but in that bug you are talking about always making the same decision for nsIMsgCompConvertible.Yes and nsIMsgCompConvertible.Altering and nsIMsgCompConvertible.No, while this is about always making the same decision, a decision other than  nsIMsgCompSendFormat.PlainText, about nsIMsgCompConvertible.Plain.
Flags: blocking1.9.1?
OS: Other → All
The difference between having one pref for all cases and one pref for each of the 3 cases is not big (enough).
These remain two different issues though: First auto-detect determines if formatting is present that would warrant sending as HTML (the subject here), only then a selection how to proceed is asked for (bug 44494, eliminating the dialog box), but we would never reach that stage unless auto-detect can be switched off. Consequently, this is a boolean yes-or-no option on auto-detect, followed by an enumeration what to do if the presence of formatting is assumed.
I must admit: I am shocked! Aghast! Terrified! I have been fighting with this bug in TB 3.0 all day trying to use HTML templates. And because of a 7 !!!!! year ignorance of simply enabling a SIMPLE !!!!!! option in about:config or whereever, so anybody for whom this is a nuisance can switch it off, people are STILL (2009!!!) forced to waste their time ...

Honestly, I do not have the ever-so-slightest understanding for such a negligence. Shame on you who could implement this switch EASILY. I know TB is a free product, but no matter if free or not, whoever codes something has responsibility for it.

Please guys, stop philosophizing and pencil-pushing and darn just SOLVE THIS FOR GOOD ONCE AND FOR ALL !!!!!

Thanks (for waisting mine and many many many other people's time). The people that found this posting are just the very tip of the iceberg)
Lee Binder, first and last warning for serious violations of etiquette (and normal human friendliness to people who gave you a free gift out of mere fun).
Stops being about fun where there is a serious motivation to try and control user's experiences. Actually DICTATE is a much better word for what has been happening here.

Apart from that, the work-around is to include e.g. <b></b>, i.e. at the very bottom of templates. This works with TB as well as ExternalTemplateLoader templates and also with Quicktext snippets. The latter have to be inserted AS HTML in the configurator and also be FORMATTED with <p></p> and/or <br> + the required HTML tag like i,b,em,strong etc.

Since enough politics has been displayed in this thread during 7 years, I guess you will now also say something pragmatic about solving the issue itself at its actual root?

Thanks.
PS: another workaround might be to include one of the mentioned tags in a signature, because if one uses them in the message body they might get deleted unintentionally ...
PS2: the trick with the signature works, <b>&nbsp;</b> is enough.

I will leave this pitifully sad thread now. May the force (finally) be with you! It's hardly ever too late.
Per forum request, I'm posting here a little hack which I'm applying for each release to get the default changed from "Auto Detect" to "Plain and Rich Text" (I'm routinely composing in plain text and open it in HTML only when I mean it, so that hack allows me to easily select the send mode with the Shift key when opening or replying).

Download and copy the patch into the "chrome" directory of your Thunderbird installation and perform the following sequence of instructions there, using
a command shell (bash, csh, etc.):

  cp -p messenger.jar messenger.jar.orig
  unzip messenger.jar \
    content/messenger/messengercompose/messengercompose.xul \
    content/messenger/messengercompose/MsgComposeCommands.js
  patch -p0 -b < bug136502tb3hack.patch
  zip -0v messenger.jar \
    content/messenger/messengercompose/messengercompose.xul \
    content/messenger/messengercompose/MsgComposeCommands.js

This patch is for Thunderbird 3.0.5, the modifications are identical for SeaMonkey. As a fallback, just open messenger.jar with the ZIP tool of your choice and edit these two files manually.
Thanks rsx11m. I just compared the Mac and the Windows messenger.jar, they are different. MsgComposeCommands.js are identical, messengercompose.xul not. I am using one shared account folder from Ubuntu, Mac OS X and Windows 7 (on my NTFS Data partition), so your approach means I would have to patch each TB install in each OS after each update (at least when the messenger.jar got changed in an update).

Do you know what needs to be installed in Windows and OS X so your diff patch can be applied? (I am aware I can always work-around by applying your patch in Ubuntu to also the other OSes messengercompose.xul but that's a bit awkward). 

Thanks,
Lee
> (comment #34) so your approach means I would have to patch each TB install
> in each OS after each update (at least when the messenger.jar got changed in an
> update).

Yes, when an update is applied (and a partial update would likely fail after having modified the JAR file, but should fall back to a full update), you'll have to run this again. Thus, it's best done as a shell script which you can run without having to think about it.

> Do you know what needs to be installed in Windows and OS X so your diff patch
> can be applied?

While the patch was created on Linux, it should equally apply to Windows and Mac OSX versions (I've tested on Linux and Windows only). Ideally, you would
edit the source and then recompile/repackage everything as needed, in which
case any preprocessing for differences in platforms would be automatically considered. For this patch, the code modified in messengercompose.xul is far enough from any #ifdef platform-specific in- or exclusions, thus the patch program will figure out the offset and just report it (3 lines for Windows).
Oops, I just figured that I've misunderstood the second part of your question
the moment I've hit the "Commit" button... I don't know how to get the "patch" package for Mac OSX, but for Windows I'd recommend http://www.cygwin.com for those. It's a rather powerful environment, thus you should go through the list and check "zip" and "patch" packages. BTW: It also contains Mercurial and CVS, which you'd need to get the current complete sources and compile it yourself.
;) I was just getting ready to lube my fingertips. I remember from some time before there are pre-compiled binaries of CYGWin. I had it installed in Vista.
Yet another workaround was mentioned in that forum discussion, just set a color for the background in the Composition preferences. This will trick Auto-Detect into HTML even without a signature (at least in 3.0). However, even if you make it a very light gray, I may be confusing if the recipients sees a bit of a shadow on his or her print-outs of your messages, thus not optimum either.
Re comment 33:
Wouldn't such a patch (for Options -> Format -> "Plain text and HTML" as default) mean, that the user's settings (Tools -> Options-> Composition -> General -> Send Options -> Text Format -> e. g. "Send the message only in plain text") are always ignored, since the patch makes somethings else the default via the compose window ?

--

So what about the following ideas:
1.) Rename the menu item "Options -> Format" into "Options -> One-off Bypass for User's Default-Send-Format", to make clear that this settings here are overrding the user's default-send-format-settings under "Tools -> Options-> Composition -> General -> Send Options -> Text Format -> ...".
2.) a) Remove Auto-detect completely.   Or
    b) Leave the current Auto-detect function as it is.
       Just rename it into "use hmtl only if formatted text is present".
       But if we leave this option alive here, we should also think about offering it under "Tools -> Options-> Composition -> General -> Send Options -> Text Format -> ...".
3.) Introduce a new/additional format option: "Options -> Format -> Apply user's default-send-format".
User's default-send format is "Tools -> Options-> Composition -> General -> Send Options -> Text Format -> ...".
!!! Make this new option "Apply user's default-send-format" the DEFAULT under "Options -> Format" !!!!!!!!!!!!!!!!!!!!!

EXAMPLE:
This new default option (3.) would mean: An unformatted message would then e. g. be sent as HTML and plaintext, if this is the user's choice/send option in TB ("Tools -> Options-> Composition -> General -> Send Options -> Text Format -> ...").

In my opinion having auto-detect as default doesn't make sense, because it makes the user's settings ("Tools -> Options-> Composition -> General -> Send Options -> Text Format -> ...") absolutely useless.

PS: my requests for renaming menue items/options are just suggestions. I know that the wording is bad. But for the moment helpful (for the discussion).
(In reply to comment #40)
> Re comment 33:
> Wouldn't such a patch (for Options -> Format -> "Plain text and HTML" as
> default) mean, that the user's settings (Tools -> Options-> Composition ->
> General -> Send Options -> Text Format -> e. g. "Send the message only in plain
> text") are always ignored, since the patch makes somethings else the default
> via the compose window ?

Yes, they would be ignored. Just for clarification, the patch isn't supposed to be a solution for the issue here, it's just a workaround for people who want to hard-wire the menu default to a given value. A "real" solution would switch off auto-detect by a preference and then follow the usual chain for HTML messages based on address book and domain-specific or global settings.
Further to comment 40:

Another possible solution:
Instead of introducing a new option (3.) "Options -> Format -> Apply
user's default-send-format", just check which of the available options under "Options -> Format" equal the user's default-send-format under "Tools -> Options-> Composition -> General -> Send Options -> Text Format -> ..." for the CURRENT message and hightlight this option (= make it the DEFAULT for THIS message) under "Options -> Format".
YEAIH, great - with folks like you, rsx11m and Herr Paulchen Panther (Danke) life's fun !!
Thanks for the encouragement. ;-)

Re comment #40 and comment #42, I agree that some menu items should probably be relabeled for clarification. For example, there are both Format > xxx for HTML formatting and Options > Format for the send format in the composition menu.

The main problem with your suggestion to have Send Options selection as the default for sending messages, after bug 466674, this would imply that all messages are sent with both HTML and Plain Text parts. While the default in general should probably stay at "Auto-Detect", there should be an easy way to disable or bypass it.

Without too much impact on the existing UI (an not considering bug 44494), there seem to be two major alternatives:

 - Add a checkbox to Tools > Options > Composition > General > Send Options
   saying "allow messages without formatting to be sent as plain text" and
   have it checked by default. If the user unchecks it, the auto-default
   algorithm always returns "formatting found" and proceeds with address book
   and other criteria. As a paradox, this would leave the Options menu default
   in the composition window on Auto-Detect even if it's disabled, but allows
   the user to override address-book or domain settings on a per-message basis.

 - Let the checkbox say "make this my default sending mode" and leave it
   unchecked by default. If the user checks that box, something similar to
   the hack in comment #33 kicks in and the default Options > Format setting
   defaults to the value of mail.default_html_action. This would override any
   address-book or per-domain settings unless the user switches the menu back
   to "Auto Detect" for a specific message.

My two cents...
Whiteboard: [gs]
http://gsfn.us/t/15003 is not this bug. The original poster there specifically says that some words are in bold, in which case the HTML pref would be active.

The only case when we send plaintext without asking is when there's no formatting *whatsoever* in the message.
Whiteboard: [gs]
Thanks Ben, you are right! Even simplest formatting (such as bold or italic) will now be sufficient now to trigger an auto-detect="HTML" response. I've just verified this with the 3.1 RC2, and indeed it contains an HTML part now in this case where prior versions were only sending the plain-text version with those pseudo-format characters. So, that's a major improvement.
> Even simplest formatting (such as bold or italic)
> will now be sufficient now to trigger an auto-detect="HTML" response.

It's always (since 1999 or so) been like this.

Some background info:
There are 3 cases which the "auto-detect" recognizes:
- No formatting at all (apart from whitespace)
  Converts to plaintext (more specifically f=f) without asking.   
- Some structural formatting that we can represent in a different form
  in plaintext. E.g. bold, lists, links.
  Conversion dialog comes up, with a label recommending plaintext.
- Other formatting, esp. colors or similar
  Conversion dialog comes up, no explicit recommendation.

Now, what broke some time ago (might a few years by now) is that some other code added a color to the body, by default. That broke the above logic and now always triggers case 3. That needs to be fixed.

Even then, any formatting (bold, lists) would trigger the dialog.

If you set the pref mentioned here (, 
If the pref Prefs | Compose | Send Options... | "When sending messages in HTML format and one or more recipients are not listed as being able to receive HTML" is set to "Send the message in both plaintext and HTML", we skip the dialog and send as selected.
But if there's no formatting *at all*, we have no reason to send HTML, so we send plaintext only.
(In reply to comment #45)
> The only case when we send plaintext without asking is when there's no
> formatting *whatsoever* in the message.

I've just tested this with 3.1 final on XP after running into another thread mentioning font choices lost. Specifically, I made E-mails with 3-4 lines and changed the fonts in some of the lines. Here the results:

 - Explicit font choices (Arial, Courier) triggered HTML + plain-text, ok.

 - Switching the center line to "Preformat" but leaving the neighboring lines
   at "Body Text" did *not* trigger HTML, it was sent in plain-text only.

 - Switching from "Variable" (my default) to "Fixed" inserted <tt> in the
   HTML source during editing, but nevertheless resulted in plain text only.

Thus, looks there are still some bugs in the algorithm. Are those filed already or otherwise on your list?
rsx, these are not bugs, because Plaintext is inherently Preformatted and (assumed to be and typically rendered as) Fixed font.
In fact, I think do we explicitly honor Preformatted in the HTML->Plaintext converted.
Maybe, but if I have Variable Width as composition default and mark some part as Fixed Width (or preformat) on purpose, I'd expect this to be recognized as a format change by auto-detect, especially when composing in Variable Width by default, as it intuitively is an explicit format change... ;-)

I found bug 414299 on this, so it's covered. Obviously the discussion is getting a bit philosophical when and why format should be considered format
for the purpose of auto-detect. Guess I'll stick with my hack in comment #33
to have an explicit control over in which format to compose and to send.
As another workaround, the MailTweak extension offers a Send HTML tweak, http://mailtweak.mozdev.org/tweaks.html#forcehtml which essentially does
the same as attachment 449323 [details] [diff] [review] by changing the Format default.

This add-on provides various other tweaks which you may or may not want, but
just to add another alternative to hacking the code or tricking auto-detect.
(In reply to comment #45)
> The only case when we send plaintext without asking is when there's no
> formatting *whatsoever* in the message.

That is not my experience in Thunderbird 3.1.4, I made specific formatting (of links and html style "reply quoting") and they were silently thrown away and the message was converted to plain text. See description here:
http://getsatisfaction.com/mozilla_messaging/topics/sometimes_messages_composed_in_html_get_changed_to_plain_text_when_sent
Here's a workaround in TB 3.1.7 (running on Win7):

Tools - Options - Compose - General - HTML Options - Font: e. g. "Arial"
(instead of "Variable Width" [= default])

Then my mails are always sent in HTML and plaintext (as set under html send options) and auto-dect does not downgrade my mails to plaintext any more :-)
I like rsx11m's suggestions in Comment 44.
Component: Backend → Composition
QA Contact: backend → composition
Whiteboard: [patchlove][has draft patch]
> Wayne Mery (:wsmwk) changed:
>   Status Whiteboard|                            |[patchlove][has draft patch]

I wouldn't call attachment 449323 [details] [diff] [review] a draft patch, it more has
the nature of a "hack", but yes - it provides the necessary pointers.

The procedure in comment #33 will have to be changed slightly for TB 3.3 since those files are now held in a single omni.jar file (bug 601573).
Comment on attachment 137499 [details] [diff] [review]
Fix for Bug 228720 (see Prognathous' comment 5 here): Don't down-convert msg if dir attribute set [checked in]

This patch looks good to me. r=BenB with style issues fixed
Comment on attachment 137499 [details] [diff] [review]
Fix for Bug 228720 (see Prognathous' comment 5 here): Don't down-convert msg if dir attribute set [checked in]

(In reply to comment #10)
> Scott, I just opened Bug 228720 for the direction issue.

... and it was checked in there already, I think wsm was talking about the post-install hack as being the draft patch.
Attachment #137499 - Attachment description: the fix for prog's issue. Don't down convert msg if dir attribute set → the fix for prog's issue. Don't down convert msg if dir attribute set [Bug 228720]
Interesting, that patch still "applies" even though it was definitely checked in:

patching file nsMsgCompose.cpp
Hunk #1 succeeded at 5422 with fuzz 2 (offset 1165 lines).

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/mailnews/compose/src&command=DIFF_FRAMESET&file=nsMsgCompose.cpp&rev2=1.405&rev1=1.404

This is line 5097 on current trunk, so I think that we can assume this was taken care of for sure.
Hi folks,
I found another workaround for this issue.

In the config editor (Tools -> Options -> General -> Config Editor) search the parameter "msgcompose.text_color" and change the value to "black". This is equivalent to the default value "#000000", but it forces the message to be sent as HTML, given the send options are set accordingly.

The generated HTML text contains the element
<body bgcolor="#FFFFFF" text="black">
which is valid HTML.
(In reply to Peter Huber from comment #60)

Thanks Peter. We can do this in a login script, example here on StackOverflow: http://stackoverflow.com/questions/9668346/batch-file-to-modify-add-line-to-thunderbird-prefs-js-files
Other solution may be my extension, Stationery. 
This bug is problem for Stationery users too, so i add special code to disable plaint-text downgrade. Simply I override classifier function, telling TB that current main can not be downgraded, regardless of content.

It is very simple function, just put it in composer window. It will override already existing stock function.

Here is code:

function DetermineConvertibility() {
  if (gMsgCompose.composeHTML) return nsIMsgCompConvertible.No;
  return nsIMsgCompConvertible.Plain;
}
Getsatisfaction has more than enough evidence that the current interaction of settings to send HTML isn't transparent and predictable enough, and users who are confused that their HTML compositions do not get sent as HTML, but as plaintext.

http://getsatisfaction.com/mozilla_messaging/tags/bug_414299

And many users have realized that Format: Auto-Detect doesn't behave according to their expectations, resulting in the understandable request to just switch it off (this bug).

http://getsatisfaction.com/mozilla_messaging/tags/bug_136502
(currently 35 people have this problem)
(among these, http://gsfn.us/t/16j03)
Whiteboard: [patchlove][has draft patch] → [GS][patchlove][has draft patch]
Arivald created an extension to work around this (until this bug is fixed properly).
https://addons.mozilla.org/thunderbird/addon/always-html/
Attachment #137499 - Flags: review+
The solution here is to add a new option in prefs that says "Always HTML+Plaintext, even if no formatting is used", and leave the other options working as-is, just rename it to "Send HTML+Plaintext, if there is formatting".
(In reply to Ben Bucksch (:BenB) from comment #65)
> The solution here is to add a new option in prefs that says "Always
> HTML+Plaintext, even if no formatting is used", and leave the other options
> working as-is, just rename it to "Send HTML+Plaintext, if there is
> formatting".

Sorry, I don't understand. Could you provide some context, which "options in prefs" are you talking about? Rename what?
And what if I want "Always HTML only" (even if no formatting is used)?
Ben, with review+ for attachment 137499 [details] [diff] [review], are you saying that Scott's patch written in 2003 still applies cleanly to current trunk??
(In reply to Thomas D. from comment #67)
> Ben, with review+ for attachment 137499 [details] [diff] [review], are you
> saying that Scott's patch written in 2003 still applies cleanly to current
> trunk??

Ditto why has this not been checked in ?
A 9 years old patch? Applying cleanly? Are you serious? Highly unlikely.
My code review+ said that I am OK with this change, done in exactly this way, and I think the code change as presented is fine.
I didn't apply or test it.
(In reply to Ludovic Hirlimann [:Usul] from comment #68)
> (In reply to Thomas D. from comment #67)
> > Ben, with review+ for attachment 137499 [details] [diff] [review], are you
> > saying that Scott's patch written in 2003 still applies cleanly to current
> > trunk??
> 
> Ditto why has this not been checked in ?

Strangely, several comments in this bug and Bug 228720 Comment 2 all suggest that this little patch has already been checked in long ago, and it's only about dir=rtl problem of Bug 228720 which Prognathous mentioned in comment 5 here, hence the patch was attached here instead of bug 228720 where it belongs (and I suggest it should be moved there for ease of reference, and reduced confusion).

Be that as it may, attachment 137499 [details] [diff] [review] does not fix this bug, it's just another nit to tame Format:Auto-Detect into behaving less datalossy.
Ben, an answer to my comment 66 would be helpful to move this bug forward.
(In reply to Thomas D. from comment #70)
> (In reply to Ludovic Hirlimann [:Usul] from comment #68)
> > (In reply to Thomas D. from comment #67)
> > > Ben, with review+ for attachment 137499 [details] [diff] [review], are you
> > > saying that Scott's patch written in 2003 still applies cleanly to current
> > > trunk??
> > 
> > Ditto why has this not been checked in ?

It was checked in long ago yes - http://mxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgCompose.cpp#5184
(In reply to Thomas D. from comment #66)
> (In reply to Ben Bucksch (:BenB) from comment #65)
> > The solution here is to add a new option in prefs that says "Always
> > HTML+Plaintext, even if no formatting is used", and leave the other options
> > working as-is, just rename it to "Send HTML+Plaintext, if there is
> > formatting".
> 
> "options in prefs" are you talking about?

Preferences | Composition | Configure text format behaviour | Text Format
(When ... one or more recipients are not listed as being able to receive HTML)
Currently, it has the options:
1. Ask me what to do
2. Convert the message to plaintext
3. Send the message in HTML anyway
4. Send the message in both HTML and plaintext

Currently, this whole option is only used, if the message contains formatting in the first place. If "one or more recipients are not listed as being able to receive HTML" *and* the message contains no formatting, we take the safe route and send plaintext.

---

This bug complaints about this behavior. I therefore suggested to have the options:
1. Ask me what to do (if formatting is used)
2. Convert the message to plaintext (even if formatting is used)
3. Send as HTML and plaintext (if formatting is used), otherwise send as plaintext
4. Send as HTML and plaintext (even if no formatting is used)

---

Alternatively, we could make 2 dropdowns:

If the message can be sent as plaintext:
1. Send as plaintext
2. Send as HTML and plaintext

If formatting is used that cannot be expressed in HTML:
1. Ask me what to do
2. Convert the message to plaintext
3. Send as HTML and plaintext

I think we should go with the latter.
(In reply to Ben Bucksch (:BenB) from comment #73)
> Alternatively, we could make 2 dropdowns:
> 
> If the message can be sent as plaintext:
> 1. Send as plaintext
> 2. Send as HTML and plaintext
> 
> If formatting is used that cannot be expressed in HTML:
> 1. Ask me what to do
> 2. Convert the message to plaintext
> 3. Send as HTML and plaintext
> 
> I think we should go with the latter.

The advantage of that approach is that leaves the one dropdown and how it works untouched, and adds another (the first) dropdown for the case that we currently simply downconvert to plaintext. I.e. it's a direct fix for this bug, it's understandable, clear UI, and it's a straight-forward addition to the code, because we simply add handling for one new pref in one code place.
Attachment #137499 - Attachment description: the fix for prog's issue. Don't down convert msg if dir attribute set [Bug 228720] → Fix for Bug 228720 (see Prognathous' comment 5 here): Don't down-convert msg if dir attribute set [checked in]
(In reply to Ben Bucksch (:BenB) from comment #73)

Thanks for explanation, that's helpful!

> (In reply to Thomas D. from comment #66)
> > (In reply to Ben Bucksch (:BenB) from comment #65)
> > > The solution here is to add a new option in prefs that says "Always
> > > HTML+Plaintext, even if no formatting is used", and leave the other options
> > > working as-is, just rename it to "Send HTML+Plaintext, if there is
> > > formatting".
> > 
> > "options in prefs" are you talking about?
> 
> Preferences | Composition | Configure text format behaviour | Text Format
> (When ... one or more recipients are not listed as being able to receive
> HTML)

OK, on Windows, that's
Tools | Options | Composition | General | Configure text format behaviour [Send Options...]

The full caption for those 4 current options reads:
"When sending messages in HTML format and one or more recipients are not listed as being able to receive HTML:"

And there's a subtext, too:
"Note: Use the Address Book to specify preferred text formats for recipients."

1) I have some doubts if something so deeply buried in Options can solve the problem of this bug, but I need to give that more thought.

> Currently, it has the options:
> 1. Ask me what to do
> 2. Convert the message to plaintext
> 3. Send the message in HTML anyway
> 4. Send the message in both HTML and plaintext
> 
> Currently, this whole option is only used, if the message contains
> formatting in the first place. If "one or more recipients are not listed as
> being able to receive HTML" *and* the message contains no formatting, we
> take the safe route and send plaintext.

2) Unfortunately, current behaviour is more complex & confusing than that, and also broken. Try this:
- Mark recipient's address book card as "prefers plain text"; let's say his name is "Paul Plain"
- Check checkbox for: Tools > Account Settings > MyAccount > Composition & Adressing > [x] Compose messages in HTML format
- In Tools | Options | ... Send Options (see above), set "When sending messages in HTML format and one or more recipients are not listed as being able to receive HTML:" to
"Ask me what to do"
- From Tools | Options |... Send Options (see above), set HTML font to "Variable width" (which is default)
- Compose message to "Paul Plain"
- Set Options | Format to "Rich Text (HTML) only" (important!)
- type hello world, do not(!) apply any formatting
- send

Actual Result:
- Message goes out as HTML only (because user chose so from Options > Format, so that's correct behaviour);
- however, it goes out to an explicit "plain-text" recipient who is "...not listed as being able to receive HTML", so "Ask me what to do" should occur but it does not (bug).

> If "one or more recipients are not listed as being able to receive HTML" *and*
> the message contains no formatting, we take the safe route and send plaintext.
As shown above, that's not applicable for all cases. Perhaps it applies with the additional condition *if* Options | Format | Auto-Detect is ON; but this bug wants to turn that very setting OFF.

I don't have time to test all the possible combinations (plenty), but I take that as yet another indication that the current interaction of various settings with regards to plaintext vs. HTML is really not predictable nor transparent.

> we take the safe route and send plaintext.

3) ...only that it's no longer the safe route, because times have changed:

3a) Prefers to receive messages formatted as "Unknown" (which is default for new and collected AB entries) can safely be assumed as "is able to receive HTML", because the vast majority of mail readers whichever flavor are capable of that. For those few cases where plain-text is really *required*, well, I guess *they* can change that setting.

3b) Given that former bandwidth, HTML compatibility etc. limitations (56K modem & text-only mail reader) are widely and increasingly a thing of the past, the odds are much higher that sending plaintext will annoy user's if there's any formatting whatsoever getting lost on the way (for an incomplete list formats & styles dumped by Auto-Detect, see Bug 414299 Comment 108, and the testcases on that bug). Users rightly expect that message composition follows the principle of wysiwyg as far as possible, and the safest route to ensure consistent formatting between sender and recipient is HTML, like it or not.

> This bug complaints about this behavior. I therefore suggested to have the
> options:
> 1. Ask me what to do (if formatting is used)
> 2. Convert the message to plaintext (even if formatting is used)
> 3. Send as HTML and plaintext (if formatting is used), otherwise send as
> plaintext
> 4. Send as HTML and plaintext (even if no formatting is used)

4) If you ask me (with 5 years of TB/BMO experience), that's pretty hard to understand... Fortunately, even Ben himself does not favor the above "solution".

> Alternatively, we could make 2 dropdowns:
> 
> If the message can be sent as plaintext:
> 1. Send as plaintext
> 2. Send as HTML and plaintext
> 
> If formatting is used that cannot be expressed in HTML:

Formatting that *cannot* be expressed in *HTML*?
I suppose it's just a typo, and you really mean:
"If formatting is used that cannot be expressed in *plaintext*"?

> 1. Ask me what to do
> 2. Convert the message to plaintext
> 3. Send as HTML and plaintext
> 
> I think we should go with the latter.

Ben never answers all of my questions... :(
Here's the one which still needs answer:

5) Using the "2-dropdowns" proposal as outlined in Ben's comment 73...
...what if I want to just "Send HTML only" (and never plaintext, regardless of formatting used or not)?

More specifically, can you provide reasons for *not offering*:
[dropdown1]... 3. Send as HTML
and for *removing the existing option* of
[dropdown2]... 3. Send the message in HTML anyway?

6) First thoughts on alternative solutions:

Fwiw, given the complexity of options and scenarios involved, I'm still trying to more fully understand the current behaviour, and I cannot yet present a better solution.

6a) However, per current summary, this bug wants an option to "turn off Options > Format > Auto-Detect" (because Auto-Detect is too much dataloss(y) for a whole range of cases). I wonder if turning OFF Auto-Detect could just be achieved by enabling turning ON of another option from that very same menu. Iow, provide a way of making *any* of
> Auto-Detect / Plain Text only / HTML only / Plain Text AND HTML
the per-account default setting for *any* new mail.

Maybe someone can figure a more simple UI that is not buried somewhere in options to control that.

6b) Perhaps we could have something in per-Account Settings | Composition and Adressing?

Perhaps where we now have checkbox...
[x] compose messages in html format
... we could replace or somehow combine that with the following dropdown list? :

Default format for outgoing messages:
1. Auto-Detect
2. HTML
3. Plain Text
4. HTML and Plain Text

My vague assumption is that if the user can currently choose that for any given message, and we can currently handle it with respect to recipients preferences and related general options, then there should not be a problem about making any one of these the *default* setting per-account. If there *is* a problem, well then, I assume we already have that problem in the current behaviour, which is kinda likely. For a start, if we were to read prefers format "Unknown" more appropriately as "probably able to receive HTML", that would probably make things easier and work better with the default outgoing format dropdown I sketched above.

7) On second thoughts, I think what this boils down to is a *shift of paradigm*:
- move towards a predictable, stable, message-centric text format with reliable wysiwyg composition (probably realizable as a per-account default choice of text format)
- move away from the current *need* for recipient-centric AND repeated per-message user intervention if user wants to ensure sending wysiwyg aka HTML (while still providing recipient-centric options for users who need them)
- move away from Auto-Detect's current *datalossy conversion to plaintext*, based on the now obsolete assumption that plaintext is the superior format to avoid various problems

FTR: I am *not* claiming to know all the questions, *nor* to have "The solution" here... I am *not* calling for the removal of Auto-Detect either; I suppose I could actually like Auto-Detect if it behaved better and more predictable than now.
See Also: → 584363
Blocks: 1202227
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #75)
> (In reply to Ben Bucksch (:BenB) from comment #73)
> 2) Unfortunately, current behaviour is more complex & confusing than that,
> and also broken. Try this:
> - Mark recipient's address book card as "prefers plain text"; let's say his
> name is "Paul Plain"
> - Check checkbox for: Tools > Account Settings > MyAccount > Composition &
> Adressing > [x] Compose messages in HTML format
> - In Tools | Options | ... Send Options (see above), set "When sending
> messages in HTML format and one or more recipients are not listed as being
> able to receive HTML:" to
> "Ask me what to do"
> - From Tools | Options |... Send Options (see above), set HTML font to
> "Variable width" (which is default)
> - Compose message to "Paul Plain"
> - Set Options | Format to "Rich Text (HTML) only" (important!)
> - type hello world, do not(!) apply any formatting
> - send
> 
> Actual Result:
> - Message goes out as HTML only (because user chose so from Options >
> Format, so that's correct behaviour);
> - however, it goes out to an explicit "plain-text" recipient who is "...not
> listed as being able to receive HTML", so "Ask me what to do" should occur
> but it does not (bug).

Now this is a new feature and seems to go against all the other bugs I have seen yet. I.e. that just want the Send options honored and the user only asked if the "ask me/autodetect" options are set. He you want an explicit "Send HTML only" selection be disregarded in some cases. 

If you want to pursue this scenario and the proposal, please file it as a new bug, if there is none.
Bug 414299 comment 146 has a good idea of inferring the format from the composition mode of the compose window (either HTML or plain).

My proposal is this:
In the currect Options->Delivery format menu add a new option like "Composing mode", so:
"Autodetect"
"Composing mode"
-------------
"Plain text"
"HTML"
"Both"

Choosing one of the first 2 options would be preserved in a pref across compositions.
The latter 3 would not be preserved (as today), and act only on the current message.

The "composing mode" means, if you started in HTML mode, the message is automatically considered HTML and inconvertible to plain text. If you send to plain text recipients, the Send Options will be used to decide what to do (bug 1204379).

I think in this case both camps could be happy. People wanting automatic downgrade if message is deemend not HTML (even if composed in HTML mode, but using no formatting) would stick to "autodetect".
Everybody else fearing even simple HTML messages may loose some tiny bit of formatting, can force HTML just by starting in HTML mode (which can be preset in account options).

Opinions?
(In reply to :aceman from comment #79)
> Opinions?

I don't think "what many bug opners/comment posters actually want in this bug" is "default other than Delivery-Format=Auto-Detect".
I think "what bug opner/comment posters/complaints poster are requesting in this bug" is "Tb's behavior when Delivery-Format=Auto-Detect is forced is far different from his expectation(i.e. Tb silently sends his HTML mail as text/plain by unknown reason, even though he composed mail in HTML mode, even though he didn't request to send as text/plain to Tb), so please stop forcing Delivery-Format=Auto-Detect".

I think following is sufficient as first answer to bug opener/comment posters in this bug(and in many bugs relevant to Auto-Detect).
(a) Delivery-Format=Auto-Detect => Delivery Format=Determined by Send Options
(b) "When mail is sent in HTML..." in Send Options panel => "When mail is composed in HTML mode..."
(c)    if (allHtml) {Send as text/html; return;}
    -  if (aConvertible == nsIMsgCompConvertible::Plain || allPlain) {Send as text/plain; return;}
    +  if (allPlain) {Send as text/plain; return;}
       Send Options check follows, and Send Options is applied,
    Note: If above change, Send Options is applied regardless of aConvertible value
          when allHtml != true && allPlain!=true (===any mixed preference case).
     i.e. "Silently sent as text/plain because of aConvertible==nsIMsgCompConvertible::Plain" won't occur.
     i.e. "Send Options" is applied always when user composed his mail in HTML mode.
          ("allHtml, so Send as text/html" and "allPlain, so Send as text/plain")
          (can be considered "done by Send Options" by "Delivery Format=Determined by Send Options".)
Please don't forget that Send Optios already has options of Ask, Send Both, Send HTML only, Send Text only.
Please note that aConvertible value which is set by Auto-Detect feature can be freely utilized in action of Send Options any time, if needed or when Send Optios will be enhanced.
I believe majority of Tb users won't request other default than "Delivery Format=Determined by Send Options".
(In reply to :aceman from comment #79)
> Bug 414299 comment 146 has a good idea of inferring the format from the
> composition mode of the compose window (either HTML or plain).
> 
> My proposal is this:
> In the currect Options->Delivery format menu add a new option like
> "Composing mode", so:
> "Autodetect"
> "Composing mode"

I think that doesn't make sense because Options -> Delivery format is only available in the HTML editor... I also think the option itself does not make much sense, because it's obvious that when you compose in plaintext, you can only send plain because HTML has not been composed. It's also quite obvious that when you compose using HTML editor, the general expectation is to send HTML. I still find it weird why users should compose in HTML and expect to send plaintext. It's nonsense.
What isn't obvious is why auto-detect should involve auto-degrade; and historically (even to this day), the major purpose of auto-degrade seems to avoid "ASK" option of send options, which is no longer default. Auto-detect should just create the right combination of HTML, plain, or both, according to recipients preferences, and probably with a bias for "BOTH" even for plaintext recipients if there's any formatting. Otherwise observe send options, which might work reasonably well if we stop to downgrade beforehand. I'm still wondering if we need a new pref which defines if "unknown" translates to "prefers-text", prefers-html, or prefers-both. With that information, we could perhaps make auto-detect even more finetuned to the point and users preferences. IF a new option in delivery format menu really makes sense (I have doubts), then perhaps something like "Auto-Detect (prefer plaintext)" for auto-degrade+auto-detect.
Delivery format "Composing Mode" will also be a very confusing option for the majority of our users who have never in their life seen any other composing mode than HTML composer.
(In reply to WADA from comment #80)
> (In reply to :aceman from comment #79)
> > Opinions?
> 
> I don't think "what many bug opners/comment posters actually want in this
> bug" is "default other than Delivery-Format=Auto-Detect".
> I think "what bug opner/comment posters/complaints poster are requesting in
> this bug" is "Tb's behavior when Delivery-Format=Auto-Detect is forced is
> far different from his expectation(i.e. Tb silently sends his HTML mail as
> text/plain by unknown reason, even though he composed mail in HTML mode,
> even though he didn't request to send as text/plain to Tb), so please stop
> forcing Delivery-Format=Auto-Detect".

Yeah, that's probably right. Users want to switch off Auto-Detect mostly because it involves Auto-Degrade which is uncontrollable currently. If we remove Auto-Degrade, most users will probably be happy. We should try it.
Otherwise, so far I took this bug as allowing users to set a per-account format for autogoing messages, regardless of recipient preferences. Iow, be message-centric, not recipient-centric.

I mostly agree with WADA's plan listed in comment 80.
I don't agree that allPlain should automatically downconvert to plaintext without using send options.
Current wording of sent options suggests otherwise, and send options allow user to choose if he wants to be warned about dataloss, or just send something useful (like "both" formats) without asking.

The current crux of send options is that it mixes prefers-plain and prefers-unknown, which are two totally different animals. As I said before, I suspect we need to disentangle them so that send options are only for prefers-plaintext, and have separate options for prefers-unknown.

Or something else which is more intelligent and understandable than now.
But even WADA's proposal will go a long way to improve things, and doesn't make anything worse, so I'm not against it.
In that case, the patch in bug 1204379 will quite help here.
But there is still a difference to 1) "send plain if message is convertible and all recipients want plain" and 2) "if message is composed in HTML consider it HTML in any case and honor Send options for plain text recipients". There is a tiny window for messages that are correctly considered convertible by the algorithm and messages that are incorrectly considered convertible by the algorithm. Option 2 would be for users that think the window is too large and still some formatting is lost. E.g. the header of a replied plain text message.

I understand you guys want option 2) and ideally just remove option 1). But it seems Magnus wants option 1) for some users. Therefore I propose to implement both options if we can't find some middle ground.
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #81)
> It's also quite obvious
> that when you compose using HTML editor, the general expectation is to send
> HTML. I still find it weird why users should compose in HTML and expect to
> send plaintext. It's nonsense.

Well, in bug 1204379 you guys actually lean to that option IF the message composed in HTML is detected to be convertible.

The bug here wants a way to force HTML if composed in HTML regardless of what TB thinks about convertibility.
I offered to allow that. Also notice you can toggle HTML/plain mode of composition via the right hotkey before opening the compose window. So it is not that if a user has HTML mode set in account options, that he will always have the HTML composer available.
Removing myslef on all the bugs I'm cced on. Please NI me if you need something on MailNews Core bugs from me.
I think the patch in bug 1204379 does what WADA says in comment 80, with the addition that if message is NOT convertible and there are allPlain recipients, then we obey the Send options, because that is what they say. There is no saying that if recipients want plain, then we can never send them both formats. If that is wanted, we need a new UI bug for changing the logic and the strings in the prefs, as I already said somewhere.
Attached patch WIP patch (obsolete) — Splinter Review
This implements my proposal from comment 79. The setting is per-identity (so you can have it differently for e.g. news identities) and can only be set in Options->delivery format. Note, this only overrides the nsMsgCompose.bodyConvertible value and assumes the composition is HTML. If recipients are unknown Send options still determine outcome. If all plaintext, downgrading occurs, due to bug 1204379. I do not touch that here.

So this option is for users that think nsMsgCompose.bodyConvertible can never be 100% right and may loose some small bits of formatting. Which I do agree with, if some users want to keep pixel perfect rendering at recipient (which they will not always get, but why fight them).

(As a WIP, this patch must be applied on top of bug 584313).
Attachment #449323 - Attachment is obsolete: true
Attachment #8672345 - Flags: feedback?(m-wada)
Attachment #8672345 - Flags: feedback?(bugzilla2007)
Comment on attachment 8672345 [details] [diff] [review]
WIP patch

Thanks Aceman for driving this forward with real patches.

However, as I tried to explain in my Comment 81, I think the approach/design of this patch does not make sense and it will create more problems and confusion than it solves. Even the patch code looks very much like a hack, and we are perpetuating the nonsense of the current misdesign by adding more variables to ensure that HTML compositions actually get sent as HTML (wow!!!).

I haven't read the patch in all detail, but as you said it implements comment 79, so I'll take it from there.

Auto-Detect only exists in HTML editor, never in plaintext editor.
Menu with choices for Delivery Format only exists in HTML editor, never in plaintext editor. I understand your new "delivery format": "compose mode" promises this:

(In reply to :aceman from comment #79)
> Bug 414299 comment 146 has a good idea of inferring the format from the
> composition mode of the compose window (either HTML or plain).

When composing in HTML mode, send HTML (without Auto-Degrade, yeah).
When composing in Plaintext mode, send Plaintext.

Now that's trivial, and useless for plaintext editor which can only send plaintext.

But even for HTML editor, it's confusing:

> My proposal is this:
> In the currect Options->Delivery format menu add a new option like
> "Composing mode", so:
> "Autodetect"
> "Composing mode"
> -------------
> "Plain text"
> "HTML"
> "Both"

In HTML editor (the only editor where this menu occurs), "composing mode" is *always* HTML. So what's the difference between "Composing mode(==HTML)" and the existing menu "HTML"? I see none. The existing "HTML" command/option will actually also bypass "Auto-Downgrade". So we'd have two menu options which do exactly the same, which violates ux-minimalism. Even "Assume HTML" does not make it any better.

> Choosing one of the first 2 options would be preserved in a pref across
> compositions.
> The latter 3 would not be preserved (as today), and act only on the current
> message.

Why? That's very confusing, and not necessary.

> The "composing mode" means, if you started in HTML mode,

When you have a menu with "delivery format" options, containing the new "composing mode" option, you have *always* started in HTML editor mode. So there's no IF. The condition is always true. Plaintext editor doesn't care about any of this.

> the message is
> automatically considered HTML and inconvertible to plain text. If you send
> to plain text recipients, the Send Options will be used to decide what to do
> (bug 1204379).

I don't think that bug 1204379 is relevant here. Bug 1204379 was trying to land a dirty workaround for bypassing "Auto-Downgrade" by default, while allowing interested users to preserve the current behaviour. This bug 136502 is about bypassing the entire Auto-Downgrade and Auto-Detect conglomerate algorithm, and allowing users to choose any of the existing delivery options other than "auto-detect" as a *default* (as opposed setting the same manually for every message).
All options except Auto-Detect actually work as expected, and do NOT involve Auto-Downgrade.

> I think in this case both camps could be happy. People wanting automatic
> downgrade if message is deemend not HTML (even if composed in HTML mode, but
> using no formatting) would stick to "autodetect".
> Everybody else fearing even simple HTML messages may loose some tiny bit of
> formatting, can force HTML just by starting in HTML mode (which can be
> preset in account options).
> Opinions?

OK, here's what we can do for this bug, and it won't be too hard I guess/hope/pray :

Please have a look at my proposal of attachment 8672330 [details], page 1, Send options UI. This bug is about implementing the section with the caption:
*Default Message Delivery Format*

Technical requirements for the full feature:
1.) global pref with UI in send options (as depicted in proposal, use dropdown instead of bullets)
- defaults to Auto-Detect
2.) prefs for each identity (!) with UI in identity settings
- if not set, use global pref
- UI in identity settings > composition and addressing > composition >
below this option:
[ ] compose messages in HtML format
insert this option:
    Default Message Delivery Format: [Use global setting: #displaytext-of-current-global-setting# v] <- dropdownlist
- in identity settings UI, default to "Use global setting..." (which should NOT need/create the per-identity-pref)
3.) When starting new composition, get value of current sender's identity pref, and use that to execute/tick the respective command from delivery Format menu

Oh yes, this feels good...
Attachment #8672345 - Flags: feedback?(bugzilla2007) → feedback-
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #89)

> 3.) When starting new composition, get value of current sender's identity
> pref, and use that to execute/tick the respective command from delivery
> Format menu

You don't any other code to tweak convertible or internal auto-detect logic or such, because just executing/checking the delivery format commands from the delivery format menu should do the trick; only auto-detect involves auto-degrade; any other option just works as it should out of the box, without auto-degrade.
(In reply to :aceman from comment #88)
> WIP patch

Why such big change is needed?
I feel logic like following is sufficient(see bug 1210244 coment #60)
 var skip_auto_downgrade = prefs setting value of mailnews.compose.skip_auto_downgrade ;
 var force_auto_downgrade_if_Convertible = !skip_auto_downgrade && aConvertible==nsIMsgCompConvertible::Plain ;
 var force_auto_downgrade_if_Not_Convertible = !skip_auto_downgrade && aConvertible==nsIMsgCompConvertible::No ;
 if ( allhtml ) `{ regardless of aConverible value, send as text/html; } <= (A-1)/(A-2) == Auto-Detect
 else if( allPlain ) {
     if     (force_auto_downgrade_if_Convertible    ) { send as text/plain ; } <= (B-1) == Auto-downgrade
     else if(force_auto_downgrade_if_Not_Convertible) { send as text/plain ; } <= (B-2) == Auto-downgrade
     else { send as text/html ; }
  }
  else if( !allhtml && !allPlain ) {
     if     (force_auto_downgrade_if_Convertible    ) { send as text/plain ; } <= (C-1) == Auto-downgrade
     else if(force_auto_downgrade_if_Not_Convertible) { UseSendOptions     ; } <= (C-2) == Send Options
     else { send as text/html ; Or UseSendOptions as currently done in (C-2) ; }
     // send as text/html is beter because current Send Options panel is for (C-2) only
  }

Above is written as current statements change for explanation, and in case of that send as text/plain is needed when allPlain.
Following is perhaps better, because it's easy to understand.
  if (skip_auto_downgrade) { send as text/html; return; }
  // Do same code as current at here

By only simple logic change with a new prefs settig(minimum change), following is achieved.
If mailnews.compose.skip_auto_downgrade=false, absolutely same as current.
If mailnews.compose.skip_auto_downgrade=true, always sent as text/html, regardless of aConverible value, regardless of Prefers setting, regrdless of Send Options setting.
=> Identical to "kill auto-downgrade, ignore Send Options".
   Identical to forcing Delivery Format=HTML Only even when Delivery Format=Auto-Detect is used.
   Prefs name of mailnews.compose.Always_Send_As_HTML_If_User_Composed_HTML_Mail may be better.

This kind of change was not rejected in bug 1210244.
User's wants/needs here === send as text/html even when Tb considers convertible to text.
This is done only when user intentionally sets prefs setting.
If prefs setting is not changed, absolutely same behavior as current.
What's wrong?

I don't think "silently send as multipart/alternative by Send Options" is needed.
If forcing Both is needed, Delivery Format=Both can be used.
Please note that there is already "Text Composition mode" and "Delivery Format=Plain Text Only".
And, even though it's same as "Hidden option for users", Shift+Compose/Reply/Forward works in Toolbar button, Menu, Context menu. Further, bug 1213628 will be fixed.
If text/plain is needed, user can choose any option in above.
This bug is for "user wants text/html always, but Tb silently sends as test/plain by UNKNOWN reason", isn't it?
If so, I think following is sufficient.
  When user composed mail in HTML mode, always sent as text/html.
  When user composed mail in Text mode, or when user sets Delivery Format=Plain Text Only, sent as text/plain.
FYI.
attachment #8671244 [details] (attached to bug 1210244) is example of simple HTML with which I could observe phenomenon of "sent as text/plain by Auto-detect/Auto-downgrade when Prefers=Unknown".
It looks that it's mismatch between "Fixed Width Font for <tt> is Formatting by Tag for some users" and "it's convertible to text, because no format is made for convertible check logic of Tb".
(note: even if a user repeatedly complaints on <tt>, such user usually never complins on <pre>. It's mystery for me.)
Simple Link case(link text==link url, created by copy/paste of a link from other application) may be debatable. In HTML composition window, it's shown as proper HTML Link. So, I think user usually expects "sent as tet/html".
I don't know other cases.
"CSS Style case" is "such simple HTML" + "<style>, <link rel=tylesheet>, Style attribute of tag in such simple HTML".
If tag for formatting(<b>, <FONT>, other many kinds of HTML tag) is used, phernomenon can't be observed regardless of that CSS Style is used or not.

To aceman.

I believe that there is no need to do change actually for all requests which is relevant to Auto-Detect/Auto-Downgrade.
I believe that one of next only is sufficient even if actually mandatory,
  (a) prefs like mailnews.compose.Always_Send_As_HTML_If_User_Composed_HTML_Mail = true/false
  (b) press like mailnews.compose.DefaultForPrefersUnknown = HTML or Test
      Note: (b) can be utilized by Address Book UX upon new Contact registration.
because following official methods and workarounds are already available.
  Official-1 : Delivery Format=HTML or Both
  Official-2 : Correctly set Text Domain/HTML Domain or Prefers in Address Book as required.
               Note: (b) is for helping this action by user. User doesn't need to change every Contact. 
  Many workrouns : background color=#FEFEFE, text color=#FFFFFF :-),
                   <B>, <FONT> etc. in HTML signature, Always HTML addon, ...
I think Menu change/command change etc. is a kind of overkill for actual problem.

To Ben.
Ben, please accept mailnews.compose.Always_Send_As_HTML_If_I_Composed_In_HTML_Mode_And_I_Want_HTML_Mail :-)
IIUC, you didn't completely reject this kind of prefs setting...
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #89)
> However, as I tried to explain in my Comment 81, I think the approach/design
> of this patch does not make sense and it will create more problems and
> confusion than it solves. Even the patch code looks very much like a hack,
> and we are perpetuating the nonsense of the current misdesign by adding more
> variables to ensure that HTML compositions actually get sent as HTML
> (wow!!!).
As I understand some people want to send plain text even if composing in HTML. Yes, then we need more variables.

> Auto-Detect only exists in HTML editor, never in plaintext editor.
> Menu with choices for Delivery Format only exists in HTML editor, never in
> plaintext editor. I understand your new "delivery format": "compose mode"
> promises this:
> (In reply to :aceman from comment #79)
> > Bug 414299 comment 146 has a good idea of inferring the format from the
> > composition mode of the compose window (either HTML or plain).
> 
> When composing in HTML mode, send HTML (without Auto-Degrade, yeah).
> When composing in Plaintext mode, send Plaintext.
> 
> Now that's trivial, and useless for plaintext editor which can only send
> plaintext.
Yes, but the option/pref must be there even if the user does not see it in plain text composition.

> But even for HTML editor, it's confusing:
> 
> > My proposal is this:
> > In the currect Options->Delivery format menu add a new option like
> > "Composing mode", so:
> > "Autodetect"
> > "Composing mode"
> > -------------
> > "Plain text"
> > "HTML"
> > "Both"
> 
> In HTML editor (the only editor where this menu occurs), "composing mode" is
> *always* HTML. So what's the difference between "Composing mode(==HTML)" and
> the existing menu "HTML"? I see none. The existing "HTML" command/option
> will actually also bypass "Auto-Downgrade". So we'd have two menu options
> which do exactly the same, which violates ux-minimalism. Even "Assume HTML"
> does not make it any better.
The existing HTML option is only for current message. My new one is permanent, for that identity.
Also the HTML and my new "Assume HTML" are not the same.
The existing "HTML" option totally forces HTML for the current message, regardless of recipients (also the Both and plain text items). "Assume HTML" only overrides detection of convertibility and should trigger the Send options IF recipients are unknown or plain (once bug 1204379 is done). So if we split the unknown or plain recipients handling in some other bug, user will be able to set e.g. Both for unknown recipients, and plain for Plain recipients. I do not think unconditionally forcing HTML for all recipients in all cases (as I understand your options in the identity UI) would cover all needs.

Please try the patch first. If you can't I make you a build once building works on servers.

> > Choosing one of the first 2 options would be preserved in a pref across
> > compositions.
> > The latter 3 would not be preserved (as today), and act only on the current
> > message.
> 
> Why? That's very confusing, and not necessary.
Why not? If it is confusing, we may change the wording, like:
Autodetect (for all messages)
Assume HTML (for all messages)
-------------
Plain text (for this message)
HTML (for this message)
Both (for this message)

I already renamed "Compose mode" to "Assume HTML" when you say it is only shown in HTML mode. But internally we can handle it for both modes.

> Technical requirements for the full feature:
> 2.) prefs for each identity (!) with UI in identity settings
> - if not set, use global pref
> - UI in identity settings > composition and addressing > composition >
> below this option:
> [ ] compose messages in HtML format
> insert this option:
>     Default Message Delivery Format: [Use global setting:
> #displaytext-of-current-global-setting# v] <- dropdownlist
> - in identity settings UI, default to "Use global setting..." (which should
> NOT need/create the per-identity-pref)
> 3.) When starting new composition, get value of current sender's identity
> pref, and use that to execute/tick the respective command from delivery
> Format menu
I think this patch mostly does this (except the UI is in the compose window, not in identity settings) and it does not totally force HTML, just hints more and still obeys Send options. You can set Send options to send HTML or Both if you want to obey the HTML hint (which overrides convertibility detection if bug 1204379 is finished).
A simple pseudo code.
  http://mxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgCompose.cpp#4967
  4967   }
 +       
 +       bool always_send_html = Value of
 +         mailnews.compose.Always_Send_As_HTML_If_I_Composed_In_HTML_Mode_And_I_Want_HTML_Mail :  
 +       // If user wants text/html always when he composed mail in HTML mode, kindly stop to do auto-downgrade
 +       if(always_send_html){ *result = nsIMsgCompSendFormat::HTML; return NS_OK; }
  4968
  4969   // If everyone supports HTML, then return HTML.
  4970   if (allHtml)
aceman, how about this kind of code? What's bad?
(In reply to WADA from comment #95)
>  +       if(always_send_html){ *result = nsIMsgCompSendFormat::HTML; return
> NS_OK; }
> aceman, how about this kind of code? What's bad?

I think this is a too big gun :) I think nobody wants a profile-wide pref to always send HTML (which would also force it for News even if composed in plain text).

In the patch here I allow the setting to be per-identity (that is why so much code). And also allows to obey Send options. So if you want HTML forced in all cases, you just set it at 2 places: here in the compose window the "Assume HTML" from the patch and in Send options you set "if unknown, send HTML", if "if plain recipients, send HTML". Then we just need to decide what to do if recipients are mixed.

E.g. Magnus/Ben may set in Send options "if plain recipients, send Plain".
WADA, maybe it's time to slow down a bit in frequency and length of your comments, so that other's have time to catch up and think for themselves about solutions... You're rapidly filing more, and more extensive comments than myself, which makes me worried 8) ;) Aceman already hinted that he hasn't read everything... nor have I...
Also, it'll give you time to check out my proposal, and reflect some of my comments so that we don't have to repeat ourselves...

E.g., I already mentioned that to avoid even harder reading of code, can we please construct any prefs so that they don't involve double negation:

> If mailnews.compose.skip_auto_downgrade=false, absolutely same as current.

Do (not not) do auto-downgrade, well, that's just (DO) auto-downgrade, right?
So it would have to be:

mailnews.compose.auto_downgrade=true

Maybe following is more descriptive and future-proof:

mailnews.compose.sendformat.auto_downgrade=true

(In reply to WADA from comment #91)
> (In reply to :aceman from comment #88)
> > WIP patch
> 
> Why such big change is needed?
> I feel logic like following is sufficient(see bug 1210244 coment #60)
>  var skip_auto_downgrade = prefs setting value of
> mailnews.compose.skip_auto_downgrade ;
>  var force_auto_downgrade_if_Convertible = !skip_auto_downgrade &&
> aConvertible==nsIMsgCompConvertible::Plain ;
>  var force_auto_downgrade_if_Not_Convertible = !skip_auto_downgrade &&
> aConvertible==nsIMsgCompConvertible::No ;
>  if ( allhtml ) `{ regardless of aConverible value, send as text/html; } <=
> (A-1)/(A-2) == Auto-Detect
>  else if( allPlain ) {
>      if     (force_auto_downgrade_if_Convertible    ) { send as text/plain ;
> } <= (B-1) == Auto-downgrade
>      else if(force_auto_downgrade_if_Not_Convertible) { send as text/plain ;
> } <= (B-2) == Auto-downgrade

Hmm. No. Magnus already said we cannot continue doing silent full dataloss downgrades without asking.
force_auto_downgrade_if_Not_Convertible should never exist as an option, and I think we can easily fix that case here, or not? The maximum we could silently do is when convertible::Altering (iiuc, the sequence of convertible degree returns is ::plain, ::yes, ::altering, ::no). allPlain + > convertible:plain is clearly a case for send options. And most probably a case for dedicated allPlain send options, because combined send options for allPlain and Mixed do not make sense. Please see my proposal for further explanation and proof of concept. I did look at your proposal and it was helpful, thanks.

>      else { send as text/html ; }
>   }
>   else if( !allhtml && !allPlain ) {
>      if     (force_auto_downgrade_if_Convertible    ) { send as text/plain ;
> } <= (C-1) == Auto-downgrade
>      else if(force_auto_downgrade_if_Not_Convertible) { UseSendOptions     ;
> } <= (C-2) == Send Options

That looks like a contradiction in itself. We're NOT doing force_auto_downgrade_if_Not_Convertible, but you propose to use current combined send options (for allPlain and mixed), so the variable itself is confusing because it suggests the wrong thing. But again, imo combined send options need disentangling.

My proposal introduces dedicated sendOptions for allPlain case only.
I have no problem with introducing separate sendOptions for Mixed case also, if needed.

But looking at my proposal, with forcing "both" sometimes for very few cases of somePlain & someHTML, I think that's exactly what multipart/alternative is for (both plain+html) for so we could just use that silently (which is not worse than now, because we're already defaulting to multipart/alternative). Otoh, my proposal *avoids* sending multipart/alternative (both plain+html) for several cases where it's not needed because it's clear from global user preference for "unknown" that everyone prefers HTML.

*Superseding* that, my proposal has pref for auto-downgrade, which is completely independent of auto-detect, as it will just bypass auto-detect via recipient-prefers completely, and send Plain if convertible::plain for applicable recipients scope. In my proposal, I offer separate simple pref for finetuning the *scope* of auto-downgrade according to users wishes/needs:

force_downgrade_if_convertible_plain =
(pref_autodowngrade==true && convertible::plain && recipientScope==pref_autodowngrade_scope)

Scope 1: allPlain
Scope 2: *allPlain | mixed | someBoth (but not allBoth)* (default)
Scope 3: allPlain | mixed | someBoth | allBoth | allHTML

Scopes are incremental, because accepting downgrading for higher scope (involving more recipients of type prefers-HTML, or Both) certainly implies accepting downgrading for lower scope (e.g. allPlain).

Basically, scopes are just same as WADA's 3 cases in options matrix (but my matrix is much smaller for same/similar degree of control, because many possible combinations in WADA's matrix are not reasonable):

Scope 1: allPlain
Scope 2: ... & mixed (incl. someBoth) (default)
Scope 3: ... & allHTML (incl. allBoth)

Scope 1: Most conservative. Only try downgrading when all recipients prefer plain (my favourite).
Scope 2: More aggressive, same as current behaviour. Downgrade if recipients are Scope 1, AND
         also if mixed cases of prefers-plain where some (but never all) recipients prefer HTML, or Both.
Scope 3: Most aggressive. Downgrade if recipients are Scope 1 or Scope 2, and
         even if *all* recipients prefer HTML, or Both.

Note on Scopes: as we provide user-defined resolving of "unknown" to at least either "plain" or "html" via pref, prefers/scope of "really unknown" will never occur. But in my proposal, prefers/scope of "both" may also occur for "unknown", because multipart/alternative covers exactly case where prefers-html-or-text is unknown. Also, default for unknown==prefers-html in my proposal. So all-unknown translates to allHTML. So with current defaults of my proposal, auto_downgrade will not occur for all-unknown (but can be made to occur by changing defaults). Hmmm. That's a problem if devs insist on defaulting to all-unknown-->auto-downgrade. So I'll have to think about that again but I'm tired of thinking. But I can just change one of several default settings and it will work as currently, e.g. make *Scope 3* the default instead of Scope 2. So Auto-Downgrade would then apply regardless of any recipient preference by default. Which might be OK since user can always change it, and we'll tame the dataloss of Auto-Downgrade in other bugs. Give me some time to get used to the nasty thought of giving auto-downgrade more scope by default... ggggrrrr

>      else { send as text/html ; Or UseSendOptions as currently done in (C-2)
> ; }
>      // send as text/html is beter because current Send Options panel is for
> (C-2) only
>   }
> 
> Above is written as current statements change for explanation, and in case
> of that send as text/plain is needed when allPlain.
> Following is perhaps better, because it's easy to understand.
>   if (skip_auto_downgrade) { send as text/html; return; }
>   // Do same code as current at here

I'm losing you here. I don't think it works.

> By only simple logic change with a new prefs settig(minimum change),
> following is achieved.
> If mailnews.compose.skip_auto_downgrade=false, absolutely same as current.

So WADA suggests (my translation by resolving double negation):
> if auto_downgrade==true, exactly same behaviour as current.
Well, I think it's fine to improve current edge cases and even the entire send options as long as we keep the current behaviour for

current scopes which trigger downgrade:

convertible::plain && scope: all-plain
convertible::plain && scope: all-unknown
convertible::plain && scope: mixed involving somePlain   (somePlain && (someHTML &&/|| someUnknown))
convertible::plain && scope: mixed involving someUnknown (some Unknown && (someHTML &&/|| somePlain)

(but the following scope currently does NOT trigger downgrade:)
convertible::plain && scope: all-html

> If mailnews.compose.skip_auto_downgrade=true, always sent as text/html,
> regardless of aConverible value, regardless of Prefers setting, regrdless of
> Send Options setting.
> => Identical to "kill auto-downgrade, ignore Send Options".
>    Identical to forcing Delivery Format=HTML Only even when Delivery
> Format=Auto-Detect is used.

That's VERY confusing, and a contradiction in itself:
Auto-Detect==HTML always!?

If you want that case, it's better realized by just doing my proposal of allowing a global default for delivery format menu, where user can pre-set HTML for new compositions. If we think plaintext should not (yet) be possible as an optional default because it's not (yet) safe, that's also OK if we reserve the slot in terms of pref values for future use.
Your way of using Auto-Detect but defaulting to HTML is very confusing for code and UI status, actually not possible because it violates ux-userfeedback: "interfaces should provide feedback about their current status. Users should never wonder what state the system is in." We can't indicate "Auto-Detect" but silently do "always HTML". 

>    Prefs name of
> mailnews.compose.Always_Send_As_HTML_If_User_Composed_HTML_Mail may be
> better.

I think that's just a minimal version of the default delivery format pref which I suggested, so me might as well do the real thing and allow any of auto-detect | (perhaps:) plain | html | both.

> This kind of change was not rejected in bug 1210244.
> User's wants/needs here === send as text/html even when Tb considers
> convertible to text.
> This is done only when user intentionally sets prefs setting.
> If prefs setting is not changed, absolutely same behavior as current.
> What's wrong?

It's twisted. Instead of just switching off Auto-Downgrade (which we easily can if we provide checkbox and pref for isconvertible==NO, but no double negatives), you're creating a special case for what should never be a special case, but a possible default delivery format setting out of several legitimate formats.

> I don't think "silently send as multipart/alternative by Send Options" is
> needed.
> If forcing Both is needed, Delivery Format=Both can be used.

Please WADA there's nothing wrong with "Both" if user needs that.
Delivery format both (from menu) is currently NOT sufficient because I can't make that a message-centric default. Again, offer global pref for default delivery format for new compositions as in my proposal, and all problems solved. Ideally (and required in the long run), make that a per-identity pref, which is not very hard if we default to using the global pref, so we won't create per-identity prefs unless really needed by user.
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #97)
> WADA, maybe it's time to slow down a bit in frequency and length of your
> comments, so that other's have time to catch up and think for themselves
> about solutions... You're rapidly filing more, and more extensive comments
> than myself, which makes me worried 8) ;) Aceman already hinted that he
> hasn't read everything... nor have I...
> Also, it'll give you time to check out my proposal, and reflect some of my
> comments so that we don't have to repeat ourselves...

And now you write even longer comment :)

It looks like I need to make a build with all the partial patches applied so that you all see the whole picture I aim for. I'll use the bug 1210244 for the separation of unknown and plain recipients in the Send options. Quickly tell me if there is a better bug for that. Or do we make a new one? :)
(In reply to :aceman from comment #96)
> (In reply to WADA from comment #95)
> >  +       if(always_send_html){ *result = nsIMsgCompSendFormat::HTML; return
> > NS_OK; }
> > aceman, how about this kind of code? What's bad?
> 
> I think this is a too big gun :) I think nobody wants a profile-wide pref to
> always send HTML (which would also force it for News even if composed in
> plain text).

+1

> In the patch here I allow the setting to be per-identity (that is why so
> much code). And also allows to obey Send options. So if you want HTML forced
> in all cases, you just set it at 2 places: here in the compose window the
> "Assume HTML" from the patch and in Send options you set "if unknown, send
> HTML", if "if plain recipients, send HTML". Then we just need to decide what
> to do if recipients are mixed.

Sorry Aceman, either I'm sitting on my brains or really as explained before, that approach is VERY wrong and highly confusing. Does this patch already have UI? Can you attach a screenshot?

> E.g. Magnus/Ben may set in Send options "if plain recipients, send Plain".

Really? So like my proposal, you have limited classic send options to allPlain case?
Anyway, let me ask the other way round, to be more productive... ;)

Is it too hard or impossible to implement my proposal of Comment 89?

(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #89)
> Please have a look at my proposal of attachment 8672330 [details], page 1,
> Send options UI. This bug is about implementing the section with the
> caption:
> *Default Message Delivery Format*
> 
> Technical requirements for the full feature:
> 1.) global pref with UI in send options (as depicted in proposal, use
> dropdown instead of bullets)
> - defaults to Auto-Detect

SEND OPTIONS DIALOGUE

+--- Default Message Delivery Format ------------------------------------------------------------+
| When composing HTML messages, use the following delivery format by default:  [Restore Default] |
| [*Auto-Detect            v]                                                                    |
| +------------------------------------------+                                                   |
| |Auto-Detect                               |                                                   |
| |Plain Text Only (ask before removing HTML)|                                                   |
| |Rich Text (HTML) only                     |                                                   |
| |Plain and Rich (HTML) Text                |                                                   |
| +------------------------------------------+                                                   |
| Note: You can specify a different default message delivery format for each identity            |
| in the account settings.                                                                       |
+------------------------------------------------------------------------------------------------+

> 2.) prefs for each identity (!) with UI in identity settings
> - if not set, use global pref
> - UI in identity settings > composition and addressing > composition >
> below this option:
> [ ] compose messages in HtML format
> insert this option:
>     Default Message Delivery Format: [Use global setting:
> #displaytext-of-current-global-setting# v] <- dropdownlist
> - in identity settings UI, default to "Use global setting..." (which should
> NOT need/create the per-identity-pref)

We could perhaps start with global pref and do per-identity later. But you said you know how to do per-identity, so we could try it now :)

> 3.) When starting new composition, get value of current sender's identity
> pref, and use that to execute/tick the respective command from delivery
> Format menu
> 
> Oh yes, this feels good...

WADA, Aceman, any problems with that approach?
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #99)
> > In the patch here I allow the setting to be per-identity (that is why so
> > much code). And also allows to obey Send options. So if you want HTML forced
> > in all cases, you just set it at 2 places: here in the compose window the
> > "Assume HTML" from the patch and in Send options you set "if unknown, send
> > HTML", if "if plain recipients, send HTML". Then we just need to decide what
> > to do if recipients are mixed.
> 
> Sorry Aceman, either I'm sitting on my brains or really as explained before,
> that approach is VERY wrong and highly confusing. Does this patch already
> have UI? Can you attach a screenshot?
> 
> > E.g. Magnus/Ben may set in Send options "if plain recipients, send Plain".
> 
> Really? So like my proposal, you have limited classic send options to
> allPlain case?
No.

> Anyway, let me ask the other way round, to be more productive... ;)
> 
> Is it too hard or impossible to implement my proposal of Comment 89?
It is not, but it is much more UI and options. Which I think is not necessary. Please let me build my simpler proposal and then you all test it if it covers everything.

> (In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment
> SEND OPTIONS DIALOGUE
> 
> +--- Default Message Delivery Format
> ------------------------------------------------------------+
> | When composing HTML messages, use the following delivery format by
> default:  [Restore Default] |
> | [*Auto-Detect            v]                                               
> | Note: You can specify a different default message delivery format for each
> identity            |
> | in the account settings.                                                  
> |
> +----------------------------------------------------------------------------
> --------------------+
> 
> > 2.) prefs for each identity (!) with UI in identity settings
> > - if not set, use global pref
> > - UI in identity settings > composition and addressing > composition >
> > below this option:
> > [ ] compose messages in HtML format
> > insert this option:
> >     Default Message Delivery Format: [Use global setting:
> > #displaytext-of-current-global-setting# v] <- dropdownlist
> > - in identity settings UI, default to "Use global setting..." (which should
> > NOT need/create the per-identity-pref)
>
> > 3.) When starting new composition, get value of current sender's identity
> > pref, and use that to execute/tick the respective command from delivery
> > Format menu
> > 
> > Oh yes, this feels good...
> 
> WADA, Aceman, any problems with that approach?
My patch here does most of what you say (per-identity options), just with less options. Instead of those 4/5 options you list per-identity I have 1 (in the options->delivery format). You choose between:
1. auto-detect = if plain=>send plain, if composing in HTML, detect convertibility (with its quirks) and then decide if consulting Send options is needed.
2. assume HTML = if plain=>send plain, if composing in HTML, assume it is HTML (ignore convertibility) and then decide if consulting Send options is needed.
[midair collision, but I'll post this anyway to get more clarity...]

I think I'm slowly starting to understand Aceman's patch plan here.
Is it the following?

Composition Menu > Options > Delivery Format >

( ) Auto-Detect == Auto-Detect with Auto-Degrade (before detection)
(o) Assume HTML == Auto-Detect without Auto-Downgrade (via forcing isconvertible::no)
------
html
plain
both

Whenever changed, user's radio choice between top two options is remembered in per-identity pref, without any other UI. Correct?
If yes: Yeah, so it's not really about "Assuming HTML", but it's just a simple switch to bypass Auto-Downgrade before Auto-Detect, while still using the full current flawed routines of Auto-Detect. Interesting. Will think about it.
In that case, "Assume HTML" is just bad label for functionality which might work. "Assume HTML" is meaningless for user not familiar with the hidden appetite of Auto-Downgrade. Can easily be misunderstood as "Enforce delivery format HTML".

It looks like true-false option for Auto-Downgrade, so checkbox might be more appropriate UI (you might want to swap first two options, but I went for this):

 o  Auto-Detect                             
[√] Plain text if no formatting       if true: run isConvertible normally, full program downgrade/detect
--------------------                  if false: return isconvertible::no --> detect/Send options if applic
    Plain text
    HTML
    Both

Yeah, that might work as an interim fix. I'm not sure how sustainable it would be in the long run.
But it's definitely good to have simple and fast-landing option for disabling mystery of auto-downgrade.
It doesn't solve many other big problems but perhaps your piecemeal approach can work...

For another bug: Something we might want is an indicator in status bar or elsewhere, which send format will result from auto-detect. Perhaps if user clicks on status bar indicator, allow changing the send format.
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #101)
> [midair collision, but I'll post this anyway to get more clarity...]
> 
> I think I'm slowly starting to understand Aceman's patch plan here.
> Is it the following?
> 
> Composition Menu > Options > Delivery Format >
> 
> ( ) Auto-Detect == Auto-Detect with Auto-Degrade (before detection)
> (o) Assume HTML == Auto-Detect without Auto-Downgrade (via forcing
> isconvertible::no)
> ------
> html
> plain
> both
> 
> Whenever changed, user's radio choice between top two options is remembered
> in per-identity pref, without any other UI. Correct?

Exactly.

> If yes: Yeah, so it's not really about "Assuming HTML", but it's just a
> simple switch to bypass Auto-Downgrade before Auto-Detect, while still using
> the full current flawed routines of Auto-Detect. Interesting. Will think
> about it.
> In that case, "Assume HTML" is just bad label for functionality which might
> work. "Assume HTML" is meaningless for user not familiar with the hidden
> appetite of Auto-Downgrade. Can easily be misunderstood as "Enforce delivery
> format HTML".

Sure, the wording must be fine-tuned. Yes, it does not mean enforce HTML, just say that the mail is HTML (without checking convertiblity). And then let Send options and recipient prefs decide what to do with that.
This option is for those who say convertibility detection will always be flawed and they want to preserve evy last bit of formatting. In that case, if composing in HTML, let TB assume it needs to be sent as HTML without checking.

> It looks like true-false option for Auto-Downgrade, so checkbox might be
> more appropriate UI (you might want to swap first two options, but I went
> for this):
> 
>  o  Auto-Detect                             
> [√] Plain text if no formatting       if true: run isConvertible normally,
> full program downgrade/detect
> --------------------                  if false: return isconvertible::no -->
> detect/Send options if applic
>     Plain text
>     HTML
>     Both
> 
> Yeah, that might work as an interim fix. I'm not sure how sustainable it
> would be in the long run.
> But it's definitely good to have simple and fast-landing option for
> disabling mystery of auto-downgrade.
> It doesn't solve many other big problems but perhaps your piecemeal approach
> can work...

If you add checkbox, you get 3 options now. If your "plain text if no formatting" (who decides that) is checked, what is the difference to Autodetect?
So essentially, approach of Aceman's patch is same as my patch of Attachment 8657637 [details] [diff] in bug 1202276, but mine was hardcoded and Aceman's is optional, plus remembered per identity.
Does that mean I have to set it for every identity at least once?? Annoying, because it will catch me by surprise if I forget to change it for any of my, say, 10+ identities...
I still suspect, even in its reduced version, this should go into global send options, too, to make things easier for users with many identities, administrators etc...
(In reply to :aceman from comment #102)

Thanks for fast reply and all your patient work here with people like me and WADA ;)

> (In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment
> #101)
> >  o  Auto-Detect                             
> > [√] Plain text if no formatting       if true: run isConvertible normally,
> > full program downgrade/detect
> > --------------------                  if false: return isconvertible::no -->
> > detect/Send options if applic
> >     Plain text
> >     HTML
> >     Both
> > 
> If you add checkbox, you get 3 options now. If your "plain text if no
> formatting" (who decides that) is checked, what is the difference to
> Autodetect?

No, I think you haven't understood me yet.
This should work exactly the same as your minimalist proposal, only using different UI.

It's not 3 options, it's 2:

1) (o) Auto-Detect                 (Auto-Detect with    )
&& [x] Plain text if no formatting (Auto-Downgrade==true)

2) (o) Auto-Detect                 (Auto-Detect with     )
&& [ ] Plain text if no formatting (Auto-Downgrade==false)

Auto-Detect is radio button alternative to the 3 options below separator.
Auto-Downgrade ("Plain text if no formatting") is independent checkbox in the UI.
In code, you set/remove and check the status of Auto-Downgrade-Checkbox in much the same way you checked the radio button for "Assume HTML", only that it will not affect the Auto-Detect radio button which is still active, because we'll still use Auto-Detect, only without Auto-Downgrade.

For better code reading, I would not make this another send format, but just treat it for what it is:
An extra pref which switches an important detail of Auto-Detect conglomerate on/off.
The send format which we are using is still auto-detect (1st option with radio button).

> If your "plain text if no formatting" (who decides that) is checked, what is the difference to
> Autodetect?

If "Plain text if no formatting" is checked, you'll get Auto-Detect PLUS Auto-Degrade.
If "Plain text if no formatting" is unchecked, just Auto-Detect (NO==Auto-Degrade).
You just have to convince your head, after years of indoctrination by application behaviour and Ben,  that Auto-Detect does NOT necessarily include Auto-Degrade!!! We are here to disentangle them once and for all!!!

We are fixing "if no formatting" in other bugs, aren't we?
Yes, wording is not ideal, but that's the idea of Auto-Downgrade, as documented in code comment:
> If we can guarantee that converting to plaintext is not lossy...
Iow, the assumption is that the little HTML formatting which might be there can be losslessly converted into plaintext. Iow, there's virtually no HTML formatting (which might actually be true after we tame the convertible algorithm).

Better wording welcome.
"Plaintext" is definitely needed in wording for "Auto-Degrade" checkbox, because it's a delivery format list and what you'll get from that option, if checked and found true ("no formatting"), is the delivery format "plaintext". That's exactly the transparency and de-mystification ten thousands of users have been waiting for (12000+ downloads of "Always HTML" addon, which is also poor wording, because it's never "always HTML" for delivery format, but only "always assume that HTML composition with little formatting is actually HTML composition".) Auto-Degrade, the bloody dinosaur, now exposed and about to be tamed, once and forever. Rejoice!
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #103)
> So essentially, approach of Aceman's patch is same as my patch of Attachment
> 8657637 [©] [details] [diff] in bug 1202276, but mine was hardcoded and
> Aceman's is optional, plus remembered per identity.
> Does that mean I have to set it for every identity at least once?? Annoying,
> because it will catch me by surprise if I forget to change it for any of my,
> say, 10+ identities...
> I still suspect, even in its reduced version, this should go into global
> send options, too, to make things easier for users with many identities,
> administrators etc...

Aceman, is it really needed/useful to have separate setting
Auto-Downgrade==true|false for each and every identity?

I'll think about this some more, but I believe we can very well get away with just making this a global send option. Much easier. Let's not spread and root the fragile madness even further. You either like Auto-*Downgrade*, or you hate it. I don't think it depends on the identity. It's never predictable enough, and just using a single <b> will defeat Auto-Downgrade (very small lossless conversion of formatting window). So it's never stable. So it's pointless to enable it "per-identity". Different story for Auto-*Detect* vs. other, fixed delivery formats. The big send format decisions make sense per identity, so that you can say, identity 1 should use auto-detect, identity 2 should prefer plain, identity 3 should always send both.
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #105)
> Aceman, is it really needed/useful to have separate setting
> Auto-Downgrade==true|false for each and every identity?

I don't know, I just assume people could leave it at Autodetect for identities used for News, and Assume HTML on others.

I think that is on Magnus now to decide how granular it needs to be.

(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #104)
> (In reply to :aceman from comment #102)
> Thanks for fast reply and all your patient work here with people like me and
> WADA ;)


> 1) (o) Auto-Detect                 (Auto-Detect with    )
> && [x] Plain text if no formatting (Auto-Downgrade==true)
> 2) (o) Auto-Detect                 (Auto-Detect with     )
> && [ ] Plain text if no formatting (Auto-Downgrade==false)
> We are fixing "if no formatting" in other bugs, aren't we?

> > If we can guarantee that converting to plaintext is not lossy...
> Iow, the assumption is that the little HTML formatting which might be there
> can be losslessly converted into plaintext. Iow, there's virtually no HTML
> formatting (which might actually be true after we tame the convertible
> algorithm).
Yes, I am improving the detection of "what is formatting", but in your own words that will never be 100% correct. That is why the whole patch is here. So then I am afraid the wording "Plain text if no formatting" will scare away the user, because he will think there is still some secret code in TB that decides what is formatting and what is not. And that is exactly NOT the case here, the option switches off this code. So I do not like this wording better.

> That's exactly the transparency and de-mystification ten
> thousands of users have been waiting for (12000+ downloads of "Always HTML"
> addon, which is also poor wording, because it's never "always HTML" for
> delivery format, but only "always assume that HTML composition with little
> formatting is actually HTML composition".)
Seems like exactly what I do here ;)
(In reply to :aceman from comment #106)
> Yes, I am improving the detection of "what is formatting", but in your own
> words that will never be 100% correct. That is why the whole patch is here.
> So then I am afraid the wording "Plain text if no formatting" will scare
> away the user, because he will think there is still some secret code in TB
> that decides what is formatting and what is not. And that is exactly NOT the
> case here, the option switches off this code. So I do not like this wording
> better.

Neeeee, there's a misunderstanding here, but maybe my fault...
Of course, the checkbox I suggested works the other way round compared to your "Assume HTML" radio box:
[x] Plain text if no formatting == Auto-Downgrade==true (Assume HTML==false)
[ ] Plain text if no formatting == Auto-Downgrade==false (Assume HTML==true)
So if the "if no formatting" condition scares away the user so that he unchecks auto-degrade, that's very correct and safe behaviour for the user... So it's very correct to be a bit scary and clear about what this option does, namely to send plaintext if there's very little formatting!

Alternatively, how's this?
[x] Plain text if losslessly convertible    (which might be even more scary... fair enough!)

I maintain that "Assume HTML" would be grossly misleading because it wrongly suggests that the message will always be sent as HTML, which is not necessarily the case e.g. when all recipients are prefers-plain, or some are prefers-plain and send-options have been set to "send plaintext only"...
It's also quite of context because user can't understand the meaning of "assume" in a list of otherwise very clear and unambiguous delivery formats.
So I think
"[x] plaintext if no formatting" is pretty clear:
Users who like plaintext, will not bother and keep that option active
Users who don't like plaintext, will be alarmed and switch it off
So both types of users get exactly what they want, and it's pretty exactly what's written on the box...
No problem for me. In that case we need to remove the "autodetect" item.
(In reply to :aceman from comment #96, with skipping many comments)
> (In reply to WADA from comment #95)
> >  +       if(always_send_html){ *result = nsIMsgCompSendFormat::HTML; return NS_OK; }
> > aceman, how about this kind of code? What's bad?
> I think this is a too big gun :) I think nobody wants a profile-wide pref to
> always send HTML (which would also force it for News even if composed in plain text).

I see.

If "profile-wide pref for message send" is dangerous, how about next? 
 - bool always_send_html = Value of
 + bool always_return_Convertible_for_me = Value of
        mailnews.compose.Always_Send_As_HTML_If_I_Composed_In_HTML_Mode_And_I_Want_HTML_Mail ;  
   At appropriate somewhere which is relevant to HTML Email(news is irrelevant),
   // If user wants text/html always when he composed mail in HTML mode for Non-Text-Domain, do following.
   if(always_return_Convertible_for_me){ aCovertible = Not-Converible ; }
This is not "profile-wide pref for mail send logic".
This is "profile-wide pref of a people for his HTML EMAIL composition only".
This is official version of msgcompose.background_color=#FEFEFE or msgcompose.text_color = #FFFFFF :-)

aceman, please avoid overkill in solution of this kind of problem.
Please note following.
msgcompose.background_color=#FEFEFE like one was/is well known workaroud and majority of Tb users doen't complain after he knew simple/easy/effective workaaround for him.
Simple is best. Maximum fruit by minimum effort :-) // one-liner patch failed to get fruit...
I also think "auto-downgrade when allPlain && Not-Convertible" is a "data loss", as you, Magnus, and some other peoples say.
However, as you knowm, many Tb users currently keep Prefers=Unknown in his Address Book.
So, "auto-downgrade when allPlain && Not-Convertible" occurs only when a user appropriately sets Text Domain setting according to his requirement. i.e. allPlain case is currently edge case, exceptional case :-)
When user will be able to set default for Unknown, many recent users usually choose HTML, so allPlain will be still edge case.  

> Then we just need to decide what to do if recipients are mixed.

If user will be able to set default for Unknown, mixed case is "HTML Domain recipient + Text Domain recipient".
So, "send as text/plain for Text Domain recipient" is a reasomable action.
And, because of mixed case, one of next occurs in current Auto-Detect.
  if     (mixed && Convertible)     send as text/plain // auto-downgrade
  else if(mixed && Not-Convertible) use Send Options
So, mixed case can be considered as a variant of allPlain case, if user can choose default for Unknown.
I think there is no need to worry about mixed case if Unknown will be changed to HTML or Text by user.
(In reply to :aceman from comment #109)
> No problem for me. In that case we need to remove the "autodetect" item.

You want to remove the "Auto-Detect" from menu? why?
Please see comment 104 about the possible UI states. Auto-Detect is always on (radio button is checked) by default. Below Auto-Detect, there's an independent checkbox (not part of the radio group), where you can toggle Auto-Downgrade on or off as you please.

When one of the three explicit send formats is chosen from radio group, you only have to disable the "Auto-Downgradee" aka "Plaintext if no formatting" option because it will not apply when an explicit format is requested.
(In reply to WADA from comment #110)
> However, as you knowm, many Tb users currently keep Prefers=Unknown in his
> Address Book.

Well, that is only because that is the default.

I pretty much NEVER change this for any Address Book entries because most people don't have a clue. I have set it for a few technical lists I'm on, but even those I sometimes forget to do.

So, it isn't that TB users are keeping this setting intentionally - most don't even notice it or know what it is for.
^^ ni for comment 111
Flags: needinfo?(acelists)
Workaround:
Ben Bucksch wrote in comment #64:
> Arivald created an extension to work around this (until this bug is fixed properly).
> https://addons.mozilla.org/thunderbird/addon/always-html/

Ben Bucksch wrote in 2012 in comment #65:
> The solution here is to add a new option in prefs that says "Always
> HTML+Plaintext, even if no formatting is used", and leave the other options
> working as-is, just rename it to "Send HTML+Plaintext, if there is
> formatting".

I still stand by this. If somebody would make a patch to do this (and exactly this). Defaults must stay as-is. I'd support that.
Whiteboard: [GS][patchlove][has draft patch] → [GS][patchlove][has draft patch] Workaround: https://addons.mozilla.org/de/thunderbird/addon/always-html/
(In reply to Ben Bucksch (:BenB) from comment #114)
> Ben Bucksch wrote in 2012 in comment #65:
> > The solution here is to add a new option in prefs that says "Always
> > HTML+Plaintext, even if no formatting is used", and leave the other options
> > working as-is, just rename it to "Send HTML+Plaintext, if there is
> > formatting".
> 
> I still stand by this. If somebody would make a patch to do this (and
> exactly this). Defaults must stay as-is. I'd support that.

OK, is that not what my patch does? Yes, I have it more flexible and still allow downgrade for plain text recipients. But if you want a full-scale toggle to always send HTML+plain (Both) to everybody, then surely I can do that.
Flags: needinfo?(acelists)
Attached patch patch v2 (obsolete) — Splinter Review
OK, it looks like we can move forward now after explanation from Ben and Magnus. This makes the single option. I just moved it from the compose window do Send Options and it is global.
Attachment #8672345 - Attachment is obsolete: true
Attachment #8672345 - Flags: feedback?(m-wada)
Attachment #8673195 - Flags: ui-review?(richard.marti)
Attachment #8673195 - Flags: review?(mkmelin+mozilla)
Attachment #8673195 - Flags: review?(ben.bucksch)
Attachment #8673195 - Flags: feedback?(m-wada)
Attachment #8673195 - Flags: feedback?(bugzilla2007)
Comment on attachment 8673195 [details] [diff] [review]
patch v2

Review of attachment 8673195 [details] [diff] [review]:
-----------------------------------------------------------------

Is "assume" the correct term here? Wouldn't something like "Always handle message be in HTML format when composing in HTML (Rich Text)" be better?

::: mail/locales/en-US/chrome/messenger/preferences/sendoptions.dtd
@@ +2,5 @@
>     - License, v. 2.0. If a copy of the MPL was not distributed with this
>     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
>  
>  <!ENTITY dialog.title                 "Send Options">
> +<!ENTITY defaultHTML.label            "Always assume message is in HTML format when composing in HTML (Rich Rext)">

Rich Text not Rich Rext
Attachment #8673195 - Flags: ui-review?(richard.marti) → ui-review+
Assignee: nobody → acelists
Comment on attachment 8673195 [details] [diff] [review]
patch v2

Review of attachment 8673195 [details] [diff] [review]:
-----------------------------------------------------------------

I think this is pretty close, but currently it sends both if the pref is checked. Maybe that's ok - seems gmail sends both.

One slight problem with this patch is that if you say, compose a news post you'll get the dialog saying you used formatting/bullets etc, even though you clearly didn't...

::: mail/locales/en-US/chrome/messenger/preferences/sendoptions.dtd
@@ +2,5 @@
>     - License, v. 2.0. If a copy of the MPL was not distributed with this
>     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
>  
>  <!ENTITY dialog.title                 "Send Options">
> +<!ENTITY defaultHTML.label            "Always assume message is in HTML format when composing in HTML (Rich Rext)">

It would probably be easier to formulate it from the plain text perspective. 

[x] Send messages as plain text when possible.
Attachment #8673195 - Flags: review?(mkmelin+mozilla) → feedback+
(In reply to Magnus Melin from comment #118)
> I think this is pretty close, but currently it sends both if the pref is
> checked. Maybe that's ok - seems gmail sends both.
For me both is ok:) If all are HTML, send HTML, if all are plain, send plain. If some mix, consult Send Options (I think both is the default). That is the current algorithm.

> One slight problem with this patch is that if you say, compose a news post
> you'll get the dialog saying you used formatting/bullets etc, even though
> you clearly didn't...

Well, that was the point of the pref that now everything is HTML and unconvertible (when composed in HTML). I proposed it per identity (so that some could be tied to News accounts and have the pref off).
Maybe we could do some guessing based on account or recipient type (e.g. newsgroup)?

> It would probably be easier to formulate it from the plain text perspective. 
> 
> [x] Send messages as plain text when possible.

OK, then I need to invert the pref meaning.
(In reply to :aceman from comment #119)
> > One slight problem with this patch is that if you say, compose a news post
> > you'll get the dialog saying you used formatting/bullets etc, even though
> > you clearly didn't...
> 
> Well, that was the point of the pref that now everything is HTML and
> unconvertible (when composed in HTML). I proposed it per identity (so that
> some could be tied to News accounts and have the pref off).
> Maybe we could do some guessing based on account or recipient type (e.g.
> newsgroup)?

It looks to me we ignored convertibility when posting to newsgroups anyway (if composing in HTML):
3990 function DetermineHTMLAction(convertible)
3991 {
3992   if (!gMsgCompose.composeHTML)
3993     return nsIMsgCompSendFormat.PlainText;
3994 
3995   if (gSendFormat == nsIMsgCompSendFormat.AskUser)
3996     return gMsgCompose.determineHTMLAction(convertible);
3997 

4889   if (!newsgroups.IsEmpty())
4890   {
4891     *result = nsIMsgCompSendFormat::AskUser;
4892     return NS_OK;
4893   }

I do not see how my patch affects this.
Maybe we could actually start observing convertibility for newsgroups and always send as plain if convertible? Of course, we can do it in a new bug. Should I file it?

But at least I now see we have special path for newsgroups so my pref per identity/account was really not needed.
Flags: needinfo?(mkmelin+mozilla)
(In reply to :aceman from comment #120)
> I do not see how my patch affects this.
> Maybe we could actually start observing convertibility for newsgroups and
> always send as plain if convertible? Of course, we can do it in a new bug.
> Should I file it?
Seem to be bug 396395.

> But at least I now see we have special path for newsgroups so my pref per
> identity/account was really not needed.
Well, if we need special handling for some accounts (e.g. news) as bug 396395 requests, we may need the pref per identity again :)
Hi Aceman,

very good work hey. digging deeper into the internals of the auto-downgrade, auto-detect delivery format internal conglomerate :)
but you are too fast for me, so I'm scared you'll fix bug before I could explain problem properly and how to fix it correctly ;)

Per Magnus suggestion:
> [x] Send messages as plain text when possible.
Yes, this inverted form is much better and more correct. Meaning in code:
- Checked: Auto-Downgrade to plain if convertible::plain
- Unchecked: Don't auto-downgrade to plain if convertible::plain
Auto-downgrade to plain == nsIMsgCompSendFormat::PlainText;

Auto-Downgrading only makes sense with Auto-Detect. (You can't set explicit delivery format "HTML-only" and then expect Auto-Downgrade to switch that to plaintext).
But Auto-Detect, by definition, is recipient-centric delivery-format decision (based on recipient preferences), so Auto-Detect can never be message-centric "Always HTML".
("Always HTML" is same as "HTML-Only" as default setting. So that's case of allowing user to set one out of  {Auto-Detect | plain | html | both} as default delivery format for new comopositions.)

SO, I think this is better, correcter, and easier patch (current TB code quoted):

> 4968 
> 4969   // If everyone supports HTML, then return HTML.
> 4970   if (allHtml)
> 4971   {
> 4972     *result = nsIMsgCompSendFormat::HTML;
> 4973     return NS_OK;
> 4974   }
> 4975 
> 4976   // If we can guarantee that converting to plaintext is not lossy, send the
> 4977   // email as plaintext. Also send it if everyone prefers plaintext.
> 4978   if (aConvertible == nsIMsgCompConvertible::Plain || allPlain)

// If user accepts auto-downgrading for cases of so-called "loss-less conversion" (pref auto_downgrade_if_convertible_plain==true),
// or if all message recipients prefer plain text
autoDowngrade = Services.prefs.getBoolPref("mail.compose.sendformat.auto_downgrade_if_convertible_plain");
convertiblePlain = (aConvertible == nsIMsgCompConvertible::Plain);
if (allPlain || autoDowngrade && convertiblePlain)

> 4979   {
> 4980     *result = nsIMsgCompSendFormat::PlainText;
> 4981     return NS_OK;
> 4982   }

Message-centric Auto-downgrade supersedes recipient-centric auto-detect for almost all recipient scopes except allHTML, which is inconsistent and confusing again.
recipient scopes of some-prefer-HTML and all-prefer-HTML are not different enough to justify this inconsistency.
Now that we've made auto-downgrade optional, it's safe and sane to make it 100% message-centric without exception, and supersede allHTML scope as well.
If we agree on that, you need to move the allHTML case from above to below the auto-downgrade PLAIN code section, HERE:

4969   // If everyone supports HTML, then return HTML.
4970   if (allHtml)
4971   {
4972     *result = nsIMsgCompSendFormat::HTML;
4973     return NS_OK;
4974   }

(In reply to Aceman from Comment #119)
(In reply to Magnus Melin from comment #118)
>> It would probably be easier to formulate it from the plain text perspective. 
>>
>> [x] Send messages as plain text when possible.
> OK, then I need to invert the pref meaning.
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #122)
> very good work hey. digging deeper into the internals of the auto-downgrade,
> auto-detect delivery format internal conglomerate :)
> but you are too fast for me, so I'm scared you'll fix bug before I could
> explain problem properly and how to fix it correctly ;)

Well, if the problem is not explained in the 500+ comments in all the linked bugs, then there may be a slight problem. Maybe we should start from a different angle ;)

> Per Magnus suggestion:
> > [x] Send messages as plain text when possible.
> Yes, this inverted form is much better and more correct. Meaning in code:
> - Checked: Auto-Downgrade to plain if convertible::plain
> - Unchecked: Don't auto-downgrade to plain if convertible::plain
> Auto-downgrade to plain == nsIMsgCompSendFormat::PlainText;

This is implemented in the patch, just placed in Send options.

> > 4976   // If we can guarantee that converting to plaintext is not lossy, send the
> > 4977   // email as plaintext. Also send it if everyone prefers plaintext.
> > 4978   if (aConvertible == nsIMsgCompConvertible::Plain || allPlain)
Yes, this is the current code.

> // If user accepts auto-downgrading for cases of so-called "loss-less
> conversion" (pref auto_downgrade_if_convertible_plain==true),
> // or if all message recipients prefer plain text
> autoDowngrade =
> Services.prefs.getBoolPref("mail.compose.sendformat.
> auto_downgrade_if_convertible_plain");
> convertiblePlain = (aConvertible == nsIMsgCompConvertible::Plain);
> if (allPlain || autoDowngrade && convertiblePlain)
As I understand it, this is exactly what the patch does. Just not as a third variable, but makes convertiblePlain set to false always when composed in HTML. So that leaves you with autodowngrade only "if (allPlain) ..."

> Message-centric Auto-downgrade supersedes recipient-centric auto-detect for
> almost all recipient scopes except allHTML, which is inconsistent and
> confusing again.
> recipient scopes of some-prefer-HTML and all-prefer-HTML are not different
> enough to justify this inconsistency.
Why? In my opinion even a single Plain recipient in the mix changes the equation and must be taken care of.

> Now that we've made auto-downgrade optional, it's safe and sane to make it
> 100% message-centric without exception, and supersede allHTML scope as well.
> If we agree on that, you need to move the allHTML case from above to below
> the auto-downgrade PLAIN code section, HERE:
> 
> 4969   // If everyone supports HTML, then return HTML.
> 4970   if (allHtml)
> 4971   {
> 4972     *result = nsIMsgCompSendFormat::HTML;
> 4973     return NS_OK;
> 4974   }

I do not understand this movement. What would it change? When your condition at 4978 is now "if (allPlain)" and at 4970 it is "if (allHtml)" then I think these blocks exclude each other and their order is not important.
(In reply to :aceman from comment #123)
> (In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment
> #122)
> > very good work hey. digging deeper into the internals of the auto-downgrade,
> > auto-detect delivery format internal conglomerate :)
> > but you are too fast for me, so I'm scared you'll fix bug before I could
> > explain problem properly and how to fix it correctly ;)

> This is implemented in the patch, just placed in Send options.

Yeah, I can file another but to expose it in delivery format menu, and it won't be hard to change.
But just having the option anywhere is most important.

> > > 4976   // If we can guarantee that converting to plaintext is not lossy, send the
> > > 4977   // email as plaintext. Also send it if everyone prefers plaintext.
> > > 4978   if (aConvertible == nsIMsgCompConvertible::Plain || allPlain)
> Yes, this is the current code.
> 
> > // If user accepts auto-downgrading for cases of so-called "loss-less
> > conversion" (pref auto_downgrade_if_convertible_plain==true),
> > // or if all message recipients prefer plain text
> > autoDowngrade =
> > Services.prefs.getBoolPref("mail.compose.sendformat.
> > auto_downgrade_if_convertible_plain");
> > convertiblePlain = (aConvertible == nsIMsgCompConvertible::Plain);
> > if (allPlain || autoDowngrade && convertiblePlain)
> As I understand it, this is exactly what the patch does. Just not as a third
> variable, but makes convertiblePlain set to false always when composed in
> HTML. So that leaves you with autodowngrade only "if (allPlain) ..."

Sorry, from my reading it's not the same.
convertible is not boolean.
convertible has graded return values, degree of convertibility:
plain | yes | altering | no

Your code succeeds to skip downgrading, but only by force-changing the return value of isconvertible.
That's wrong and comes with unwanted side effects:

As we fall through all the delivery format detection conditions, we finally end up in "use send options". In send options, user can set ASK. Your fake, wrong return value of isconvertible::no will be will be used by ASK dialogue to change the dialogue message and the default replies according to degree of convertibility. So ASK dialogue will provide wrong messages from your fake value.

Anyway, is there anything wrong about my approach?
I think it's cleaner, easier to read, and more correct to directly opt out of / switch off auto-downgrade in the outer format determination code, than to plant wrong fake return value into isconvertible and risk any side effects.

> > Message-centric Auto-downgrade supersedes recipient-centric auto-detect for
> > almost all recipient scopes except allHTML, which is inconsistent and
> > confusing again.
> > recipient scopes of some-prefer-HTML and all-prefer-HTML are not different
> > enough to justify this inconsistency.
> Why? In my opinion even a single Plain recipient in the mix changes the
> equation and must be taken care of.

No, you misunderstand. Maybe because you are using Auto-downgrade and auto-detect the wrong way round, unlike the rest of us, as you said in one comment. This case here is just what you yourself have already okayed:
- We currently try auto-downgrade for all recipient scopes (including someHTML), except allHTML
- You agreed it's also ok to auto-downgrade for scope of allHTML if message is just plaintext.
Reason: User who likes auto-downgrade already ignores almost all recipient preferences because he just  wants to send plaintext when message looks like plaintext. Such user will want/accept downgrading even when all recipients are html. See below.

> > Now that we've made auto-downgrade optional, it's safe and sane to make it
> > 100% message-centric without exception, and supersede allHTML scope as well.
> > If we agree on that, you need to move the allHTML case from above to below
> > the auto-downgrade PLAIN code section, HERE:
> > 
> > 4969   // If everyone supports HTML, then return HTML.
> > 4970   if (allHtml)
> > 4971   {
> > 4972     *result = nsIMsgCompSendFormat::HTML;
> > 4973     return NS_OK;
> > 4974   }
> 
> I do not understand this movement. What would it change? When your condition
> at 4978 is now "if (allPlain)" and at 4970 it is "if (allHtml)" then I think
> these blocks exclude each other and their order is not important.

No, you already agreed that it's ok with you to apply optional autodowngrade even if allHTML is true (all prefer html). It's more consistent because autodowngrade is message-centric so to pick only one special case for not-autodowngrading in a recipient-centric way is not consistent/predictable.
with my approach, there's no need to write in UI: send as plain only if losslessly convertible and not (all recipients prefer html). To include allHTML scope in optional auto-downgrading, we need to move allHTML case down. between allplain and allhtml, otherwise order is NOT important except for the downgrading logic because it overrides all.

My mistake is that I forgot to change the comment:
> > 4969   // If everyone supports HTML, then return HTML.
This must become:
// If everyone supports HTML (and we haven't auto-downgraded before for simple messages), then return HTML.
Comment on attachment 8673195 [details] [diff] [review]
patch v2

Review of attachment 8673195 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, this is taking nice shape if the cooperative input is considered.
I'd like to finetune a little what we do here, which also makes the patch easier and keeps the original foundations laid by Ben more intact while avoiding unwanted side effects from fake return value of nsIMsgCompConvertible.No.
f+ with that fixed.
Sorry for any intersections with my previous comment. Asynchronous work style here.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +4011,5 @@
>  {
>      if (!gMsgCompose.composeHTML)
>          return nsIMsgCompConvertible.Plain;
>  
> +    if (Services.prefs.getBoolPref("mail.default_html_format")) {

I'm proposing better pref name below.

@@ +4013,5 @@
>          return nsIMsgCompConvertible.Plain;
>  
> +    if (Services.prefs.getBoolPref("mail.default_html_format")) {
> +      // We must be in HTML compose mode if we got here.
> +      return nsIMsgCompConvertible.No;

This is the wrong way of preventing auto-downgrade, because it changes the graded return value of convertible (plain | yes | altering | no) to the highest value (no), which will have side effects if user has ASK in send options. ASK dialogue is different depending on convertible degree, so users might see wrong type of ASK dialogue. See my previous comment for a better code in the main format determination logics, not in the convertible helper function.

::: mail/components/preferences/sendoptions.xul
@@ +27,5 @@
>  
>      <stringbundle id="bundlePreferences" src="chrome://messenger/locale/preferences/preferences.properties"/>
>  
>      <preferences id="SendOptionsPreferences">
> +      <preference id="mail.default_html_format"      name="mail.default_html_format" type="bool"/>

need better and inverted pref name, see below

@@ +33,5 @@
>        <preference id="mailnews.html_domains"         name="mailnews.html_domains" type="string"/>
>        <preference id="mailnews.plaintext_domains"    name="mailnews.plaintext_domains"    type="string"/>
>      </preferences>
>  
> +    <checkbox id="defaultHTML"

After inverting the pref (plus its effect) as Magnus suggests (see below), the ID should be:

<checkbox id="autoDowngradeIfConvertiblePlain" (my favorite)
or
<checkbox id="autoDowngradeToPlain"

::: mail/locales/en-US/chrome/messenger/preferences/sendoptions.dtd
@@ +2,5 @@
>     - License, v. 2.0. If a copy of the MPL was not distributed with this
>     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
>  
>  <!ENTITY dialog.title                 "Send Options">
> +<!ENTITY defaultHTML.label            "Always assume message is in HTML format when composing in HTML (Rich Rext)">

I love the correct clarity after inverting this, but I think it does not sufficiently describe what we're doing, and thus gives already-burnt-by-auto-detect users more fear than necessary, because definition of "whenever possible" is open to many different views. How about this?

[x] For simple message with virtually no HTML, always send message as plain text
or:
[x] Send messages as plain text when they look like unformatted plaintext
or:
[x] Send messages as plain text when lossless conversion seems possible.

::: mailnews/mailnews.js
@@ +172,5 @@
>  
>  pref("mail.default_html_action", 0);          // 0=ask, 1=plain, 2=html, 3=both
> +// true means message will be always considered HTML when composed in HTML mode
> +// false means auto-detect convertibility
> +pref("mail.default_html_format", false);

As Magnus said, we're not really trying to force HTML, we're trying to skip Auto-Downgrade to plaintext because it prevents/skips Auto-Detect of correct delivery format, but otherwise leave Auto-Detect intact and use it fully according to design and users preferences (both of which I'd also like to finetune, especially in favor of handling plaintext recipients, which is the main usecase of auto-detect). In most, but not all cases, this will trigger HTML. "Always HTML" is already implemented in delivery format menu action "HTML only"; what's missing is pref to freely change default value of delivery format menu action.

Magnus suggested...

> [x] Send messages as plain text when possible.
> ...to which Aceman replied:
> OK, then I need to invert the pref meaning.

Yes, please invert the pref!
I suggest this pref name:

a) mail.compose.sendformat.auto_downgrade_if_convertible_plain=true
or
b) mail.compose.sendformat.auto_downgrade
or
c) mail.compose.sendformat.plain_if_convertible_plain

.sendformat. makes a better systematic categorization and will allow us to bundle any future preferences around sendformat in the same place in prefs configurator.
I like a) best, then c). but maybe b) is also ok.

auto_downgrade_if_convertible_plain, while a bit long (who cares, nobody needs to write it), is certainly the most accurate description what this pref is about, which will assist future generations to understand better what we're doing here.
Attachment #8673195 - Flags: feedback?(bugzilla2007) → feedback+
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #124)
> Sorry, from my reading it's not the same.
> convertible is not boolean.
> convertible has graded return values, degree of convertibility:
> plain | yes | altering | no
But we handle all the non-plain values the same in regard to auto-downgrading.

> Your code succeeds to skip downgrading, but only by force-changing the
> return value of isconvertible.
> That's wrong and comes with unwanted side effects:
> 
> As we fall through all the delivery format detection conditions, we finally
> end up in "use send options". In send options, user can set ASK. Your fake,
> wrong return value of isconvertible::no will be will be used by ASK dialogue
> to change the dialogue message and the default replies according to degree
> of convertibility. So ASK dialogue will provide wrong messages from your
> fake value.

Yes, I already thought about improving that dialog and e.g. mentioning what types of recipients there actually are so that the user can make better decisions. Yes, the dialog may use the 4 levels of convertibility. If you want it to provide indication that the question is up just because the user had the pref on (and not based on inconvertibility), I am fine with that. It will just be more code and a new variable. I tried to keep it short using existing variables ;)

> > > Message-centric Auto-downgrade supersedes recipient-centric auto-detect for
> > > almost all recipient scopes except allHTML, which is inconsistent and
> > > confusing again.
> > > recipient scopes of some-prefer-HTML and all-prefer-HTML are not different
> > > enough to justify this inconsistency.
> > Why? In my opinion even a single Plain recipient in the mix changes the
> > equation and must be taken care of.
> 
> No, you misunderstand. Maybe because you are using Auto-downgrade and
> auto-detect the wrong way round, unlike the rest of us, as you said in one
> comment. This case here is just what you yourself have already okayed:
> - We currently try auto-downgrade for all recipient scopes (including
> someHTML), except allHTML
> - You agreed it's also ok to auto-downgrade for scope of allHTML if message
> is just plaintext.
> Reason: User who likes auto-downgrade already ignores almost all recipient
> preferences because he just  wants to send plaintext when message looks like
> plaintext. Such user will want/accept downgrading even when all recipients
> are html. See below.
I do not see how that answers my question. But if you want to downgrade for allHtml then doing the same for someHtml is logical.

> > I do not understand this movement. What would it change? When your condition
> > at 4978 is now "if (allPlain)" and at 4970 it is "if (allHtml)" then I think
> > these blocks exclude each other and their order is not important.
> 
> No, you already agreed that it's ok with you to apply optional autodowngrade
> even if allHTML is true (all prefer html).
I'm not aware of that agreement, but I do not care :) I give you free way :)

> My mistake is that I forgot to change the comment:
> > > 4969   // If everyone supports HTML, then return HTML.
> This must become:
> // If everyone supports HTML (and we haven't auto-downgraded before for
> simple messages), then return HTML.

OK.
(In reply to :aceman from comment #120)
> I do not see how my patch affects this.

We don't get to the ask case if the message is plaintext. Now your patch changed that the format to pretend we're html, so you always get the prompt.

> Maybe we could actually start observing convertibility for newsgroups and
> always send as plain if convertible? Of course, we can do it in a new bug.
> Should I file it?

Sure.
Flags: needinfo?(mkmelin+mozilla)
Comment on attachment 8673195 [details] [diff] [review]
patch v2

-, because I think too early to release. 

If you are eager for multipart/alternative, how about combination of next?

(1) Slightly modified your patch proposed to this bug.
    If mail.default_html_format==true
    Do all things in your current patch
    + if(mail.default_html_format==true) { send multiprt/alternative; return; }
      // current send logic
      else if(allHtml) ...
    If mail.default_html_format==false, nothing is changed from current.
    All Thomas D.'s wants are fulfilled, because multipart/alternative is always sent
    and all his styling/formatting is kept in text/html part always.

(2) CSS Style support in Convertible check, Convert Prefers=Unknown to HTML or Text.
    You already has these two kinds of solution, and implementing is in progress.
    All current bug report is following case.
      if(Convertible due to simple HTML(Body Text, P, PRE) && <tt>/<a> with link text=link url is containedd or not
         && CSS Style is used or not && Prefers=Unknown recipoent only)
    So, if style attribute is supported in Convertible check, Convertible is changed to Not-Convertible in above case. 
    If Prefers=Unknown is converted to HTML by pref=HTML, Prefers=Unknown only is changed to allHtml.
    If Prefers=Unknown is converted to Text by pref=Text, Prefers=Unknown only is changed to allText.
    Then, currently reported all cases relevant to auto-downgrade are resolved.

(3) Outstanding issue after (2).
    "if( at least one Text Domain recipient is contained in recipients && Not-Convertible) send text/plain"
    is "data loss" for you and Magnus. Thomas D. is irrelivent, because satisfied by (1).
    New pref : DataLoss_for_a_M = true/false
    - if (aConvertible == nsIMsgCompConvertible::Plain || allPlain)
    + if (
           if(aConvertible != nsIMsgCompConvertible::Plain && DataLoss_for_a_M==true)
           { ; } // Do nothing, use Send Options
           else { send text/plain as currently done; return; }
         )
    By this, if DataLoss_for_a_M = true, Send Options panel is used for "Not-Convertible&&allPlain" case.
    In Send Options panel, any of Ask, Both, HTML only, Text can be set by user.
    Because Send Options panel is used in "At least one Text Domain recipient is contained in recipiets" case,
    current Send options panel can be used.

Therefore,
- Majority of Tb users can get fruit by (2) witout doing known workarounds,
  with mail.default_html_format==false && DataLoss_for_a_M = false, without affected by any of (1) and (2).
  Because of no problem in Auto-Detect for majority of Tb users after (2), there is no need of menu change.
  No objection from Ben. No objection from Magnus on multipart/alternative.
  No objection from me, because I can get fruit by (2) and I'm not affected by changes.
- Thomas D. is satisfied by (1), mail.default_html_format==true
- Mgnus is satisfied by (2), DataLoss_for_a_M = true

I'm not opposit to your patch.
But I don't think all-in-one solution is mandatory.
I think this kind of solution is better implemented over minimum solutions like following.
(a) CSS Style attribute support in Convertible check logic,
(b) Convert Prefers=Unknown to HTML or Text by user's pref setting for default of Unknown.
(a) was hidden for long time.
(b) is a representation of my tweet in a bug in 2013.
"Default for Prefers=Unknown" is a part of a bug opened in 2000.
Please note that this bug was opened in 2002.
I believe that "no patch yet" doesn't mean "any developer ignored this issue".
I believe that "impact of this bug" is never big.
Attachment #8673195 - Flags: feedback?(m-wada) → feedback-
(In reply to Magnus Melin from comment #118)
> One slight problem with this patch is that if you say,
> compose a news post you'll get the dialog saying you used formatting/bullets etc,
> even though you clearly didn't...

Magnus, read bug 210244 comment #4 and bug 210244 Comment #5 for News posting case, please.

Note: 4 and 5, not 119. It's by bug opener. Even if long, I thibk peoples usully read and understand at least top part of a bug.
Your compaint in bug 210244 comment #126 was "I can't believe this bug has 125(!)", although "lengthy comments" is far less in this bug than that bug ;-).
(In reply to WADA from comment #128)
> Comment on attachment 8673195 [details] [diff] [review]
> patch v2
> 
> -, because I think too early to release.

?

> If you are eager for multipart/alternative, how about combination of next?
> 
> (1) Slightly modified your patch proposed to this bug.
>     If mail.default_html_format==true
>     Do all things in your current patch
>     + if(mail.default_html_format==true) { send multiprt/alternative;
> return; }
>       // current send logic
>       else if(allHtml) ...
>     If mail.default_html_format==false, nothing is changed from current.

See my comment 122 why I think this pref name is conceptually wrong and unwanted, and concept already rejected by Magnus comment 127. Following is what we really want here, and really simple without ever touching isConvertible algorithm logic:

// If user accepts auto-downgrading for cases of so-called "loss-less conversion" (pref auto_downgrade_if_convertible_plain==true),
// or if all message recipients prefer plain text
autoDowngrade = Services.prefs.getBoolPref("mail.compose.sendformat.auto_downgrade_if_convertible_plain");
convertiblePlain = (aConvertible == nsIMsgCompConvertible::Plain);

if (allPlain || autoDowngrade && convertiblePlain)
{ send plain; return;}

>     All Thomas D.'s wants are fulfilled, because multipart/alternative is
> always sent

WADA, please don't misrepresent my requests. I never requested that auto-detect should always just send multipart/alternative. It's misunderstanding by aceman and you. My request is following:

* make auto-Downgrade a fully message-centric pref, a simple switch which bypasses all auto-detection of recipient preferences and just always sends plain when the *message* is plaintext (even for allHTML recipients), and user wants it (pref.auto-downgrade==true).

> if (allPlain || autoDowngrade && convertiblePlain) {send plain; return;}

* if pref.auto-downgrade==false (user doesn't want), use auto-detect with send options:

if (allHTML)    {send HTML; return;}
if (allPlain || somePlain)  {use SendOptions (default: both); return;}
if (someBoth)   {send both; return;}

Recipient scope of SomeBoth==some(unknown==both) assuming *triple* value for user pref which disambiguates "unknown" globally into unknown==(plain || html || both). Also wanted by aceman.

WADA, FYI:
Note that "both" is current default send format for all cases of "unknown" && !=convertible::plain.
Nobody has requested to remove that option, so we must keep it when disambiguating unknown.
We surely want default of unknown==html OR unknown==both (in a 2015 HTML world, the war is over).
We want send options only for cases of somePlain and allPlain (to reduce send options matrix and exclude clear cases where we can actually auto-detect safely, e.g. allHTML->send html; unknown scope now covered by its own pref).

So with WADA's proposal, only way for user to get "unknown==both" would be

"unknown==plain" -> send options -> unknown=="both"   [sic!!!]

That's extremely confusing UX because you are forcing users to set "unknown==plain" when they really want "unknown==both". We should just allow them to set "unknown==both" directly, with the extra benefit of having dedicated send options which ONLY cover cases of somePlain and allPlain (and that's where send options are *really* needed; for all other scopes, send options are not needed after having explicit send option for scope of "unknown").

> (2) CSS Style support in Convertible check, Convert Prefers=Unknown to HTML
> or Text.

>     If Prefers=Unknown is converted to HTML by pref=HTML, Prefers=Unknown
> only is changed to allHtml.

Which would mean, if current send logic is kept, that auto-downgrade will never happen, and user cannot use auto-downgrade on unknown recipients, and we can't default to auto-downgrade, because it would require setting unknown==plain to get unknown==both which is worse than before and definitely unwanted because confusing (see above). Iow, Magnus, Ben, Joshua will not accept that solution because it de-facto removes current default of {use Auto-Downgrade for "unknown"}.

> (3) Outstanding issue after (2).
>     "if( at least one Text Domain recipient is contained in recipients &&
> Not-Convertible) send text/plain"
>     is "data loss" for you and Magnus. Thomas D. is irrelivent, because
> satisfied by (1).

No, I'm not satisfied by (1), misrepresentation of my request.
WADA, please stop discussing that case and confusing others.
Magnus has already made it clear that we can never accept forced and silent, full dataloss of formatting for cases of !=convertible::plain.

allPlain scope, per current wording and internal logic, should simply be covered by send options.
I'm fine with conflating allPlain and somePlain scopes if it helps to move on, so we'd have:
if (allPlain || somePlain) {use send options [default:both]; return}

If we keep current send options algorithm, this would actually *still* allow users to opt-in to full-silent-dataloss-downgrade by choosing: "send plain only" in send options (like Magnus, I think it should never be allowed without warning; but we can discuss that later).

>     New pref : DataLoss_for_a_M = true/false
>     - if (aConvertible == nsIMsgCompConvertible::Plain || allPlain)
>     + if (
>            if(aConvertible != nsIMsgCompConvertible::Plain &&
> DataLoss_for_a_M==true)
>            { ; } // Do nothing, use Send Options
>            else { send text/plain as currently done; return; }
>          )
>     By this, if DataLoss_for_a_M = true, Send Options panel is used for
> "Not-Convertible&&allPlain" case.

That's much more complicated than necessary.

>     In Send Options panel, any of Ask, Both, HTML only, Text can be set by
> user.
>     Because Send Options panel is used in "At least one Text Domain
> recipient is contained in recipiets" case,
>     current Send options panel can be used.
> 
> Therefore,
> - Majority of Tb users can get fruit by (2) witout doing known workarounds,
>   with mail.default_html_format==false && DataLoss_for_a_M = false, without
> affected by any of (1) and (2).

That's two prefs where we can easily get away with one without hurting anyone.
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #130)

> * if pref.auto-downgrade==false (user doesn't want), use auto-detect with
> send options:

More precisely:
* if (pref.auto-downgrade==false || convertible!=Plain), use auto-detect with send options.

All recipient scopes and their conflation for simple auto-detect algorithm shown in PDF attachment 8675716 [details]; see Bug 1210244 Comment 186, Bug 1210244 Comment 187. The PDF also shows graphically how my proposal of fully message-centric auto-downgrade pref simply bypasses *all* of auto-detect and sends plain when the *message* is plain, regardless of recipient preferences. This is possible by exposing the auto-downgrade pref (in send options; ideally also in delivery format menu), so user can transparently choose if he likes message-centric auto-downgrading or not.

I ultimately want to offer following free choices for user:

1) Auto-Detect* with Auto-Downgrade (mix of message-centric AND recipient-centric; TB default)
2) Auto-Detect* without Auto-Downgrade (strictly recipient-centric)

3) Choose default delivery format** for all new compositions (strictly message-centric):
Auto-Detect* (TB default) || HTML || Plain || Both (preset radio options in delivery format menu)

(*): Auto-Detect uses the following inputs for deciding best format:
- Explicit user pref: Auto-Downgrade==true|false (where false bypasses entire auto-detection)
- Explicit user pref: scope unknown==prefers plain|html|both (used for auto-detection according to simple scope matrix of Bug 1210244 Comment 186)
- Explicit user pref: scope allPlain||somePlain -> use current send options (default:both)

(**): message-centric default delivery format: ideally per-identity, but we can start with global default pref
All right. Let's move on with this.
Simple Solution Series. Piecemeal approach. Part 1.

Magnus, this patch addresses all of your inputs.
Minimal fix for two issues.

1) Make Auto-Downgrade behaviour optional via pref / options UI.

Simple checkbox in send options dialogue, ON by default (mailnews.sendformat.auto_downgrade=true): If auto_downgrade==true AND message is convertible::plain --> send plain text (now fully message-centric; inverted pref design per Magnus Comment 118). This addresses one of the main painpoints as it makes the current auto-downgrade-mystery/surprise transparent and user-controllable from Send Options dialogue.

2) Use send options for allPlain && convertible!=plain

When all recipients are prefers-plain (allPlain scope), with this patch we still default to sending plain if Auto-downgrade==true AND the message is convertible::plain. Otherwise, use send options! As Magnus requested in another comment, we must eliminate silent full dataloss of formatting for messages which are NOT convertible::plain. That's exactly what the current UI already claims to do: "When sending messages in HTML format and one or more recipients are not listed as being able to receive HTML [that's definitely true for allPlain!]: [Send options dropdown: Ask | plain | html | *both*].

3) FTR: Issues NOT YET addressed in this patch (Full plan):

a) Global pref to explicitly map prefers-unknown to prefers-something (plain | html | both); currently, recipient scopes of someUnknown/allUnknown are bundled into send options which are inherently designed for somePlain/allPlain scopes.
b) Pref for message-centric default delivery format (Auto-detect | plain(?) | html | both); e.g. allow message-centric users to always start their compositions with delivery format "HTML" or "BOTH" (without using Auto-Detect). Ideally global default, per-identity exceptions in account settings allowed.
c) Send options dialogue structural/textual cleanup after a) and b).
d) Expose Auto-Downgrade switch in Delivery Format menu (primary UI).
e) Consider force-warning against dataloss at least for convertible::No (after this patch, now less important as we'll never default to that, so at least user needs to opt in).
Attachment #8681918 - Flags: review?(mkmelin+mozilla)
Pls note that this bug has been slightly morphed (shift of focus), which is correct according to inherent intention: Delivery Format "Auto-Detect" is a recipient-centric algorithm to detect the best message delivery format. Mystery/surprise for users comes especially with simple messages, when for recipients of type "prefers-unknown" ("not explicitly listed as being able to receive HTML"), user's send options are not honoured (default:both), but we suddenly send plaintext (message-centric Auto-Downgrade bypasses recipient- Auto-Detect), without any hint nor controllable option in the UI. So exposing Auto-Downgrade in Send Options Dialogue is a minimal solution for most of the user problems in this bug.

Notwithstanding, the original summary of this bug is also a legitimate and useful request (comment 132, 3b): allowing the user to actually avoid recipient-centric Auto-Detect altogether by setting a fully message-centric, initial default message delivery format (e.g. "HTML", or "Both").
Summary: Provide prefs. setting to turn off Options -> Format -> Auto-Detect → Provide prefs. setting to turn off Options > Delivery Format > Auto-Detect (especially to avoid Auto-Downgrade mystery/surprise)
Comment on attachment 8681918 [details] [diff] [review]
WIP patch v.3: Minimal fix: Make auto-downgrade optional; use send options for {allPlain} scope (when all recipients prefer plain text)

OK, WIP = Proof of concept = Pseudo Code...
Patch won't apply because I didn't observe C++ syntax in nsMsgCompose.cpp.
But I guess that's just nitfixes to get the syntax right.
More nits: some wrong indentations.

Magnus, please review this anyway because we need your feedback on the proposed patch. Tia.
Attachment #8681918 - Flags: review?(mkmelin+mozilla) → feedback?(mkmelin+mozilla)
OK, so I tried to fix the syntax in nsMsgCompose.cpp to conform with c++.
I'm not familiar with c++ at all, so please correct me if it's still wrong.

Patch logic is still the same as described with more detail in Comment 132 (more background in Comment 131):

1) Make Auto-Downgrade behaviour optional via pref / checkbox in options UI.

Remove the silent mystery/surprise factor where we currently send simple HTML messages as plain text without ever telling the user, nor offering control.

2) Use send options for allPlain && (convertible!=plain || AutoDowngrade=false).

Per Magnus request to to eliminate the current silent, forced, full dataloss of formatting when all recipients prefer plain text but the message has consequential HTML formatting (including inline images etc.; existing bugs).

Friendly takeover from Aceman, whose patch (based on analysis by WADA and myself) was a great starting point :)
Assignee: acelists → bugzilla2007
Attachment #8673195 - Attachment is obsolete: true
Attachment #8681918 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8673195 - Flags: review?(ben.bucksch)
Attachment #8681918 - Flags: feedback?(mkmelin+mozilla)
Comment on attachment 8682030 [details] [diff] [review]
WIP patch v.4: Minimal fix: Make auto-downgrade optional; use send options for {allPlain} scope (when all recipients prefer plain text)

Feedback / review please per Comment 135. Thank you!
Attachment #8682030 - Flags: feedback?(mkmelin+mozilla)
Attachment #8682030 - Flags: feedback?(acelists)
Blocks: 1202276
Feedback / review please per Comment 135. Thank you!

V. 4.1 has a nitfix for an undefined entity in sendoptions.dtd, closing angle bracket was missing in this line:

<!ENTITY autoDowngrade.tooltiptext    "If the message has practically no HTML formatting, automatically convert to plain text before sending">

Please alert me to any other issues, I don't compile so I can't test the patch.
Attachment #8682030 - Attachment is obsolete: true
Attachment #8682030 - Flags: feedback?(mkmelin+mozilla)
Attachment #8682030 - Flags: feedback?(acelists)
Attachment #8683673 - Flags: feedback?(mkmelin+mozilla)
Attachment #8683673 - Flags: feedback?(acelists)
Any chance to make some progress here?
Thomas, if you don't compile, you can still push this to the try server and later retrieve the executable. I did this for you:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=94eb16159396
Download here:
https://ftp-ssl.mozilla.org/pub/thunderbird/try-builds/mozilla@jorgk.com-94eb16159396/try-comm-central-win32/
Comment on attachment 8683673 [details] [diff] [review]
WIP patch v.4.1: Minimal fix: Make auto-downgrade optional; use send options for {allPlain} recipient scope if (convertible!=plain || AutoDowngrade=false)

Review of attachment 8683673 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, this seems fine to me as an alternative to what I have implemented. It tries to do the same at different place in code. And it does not lie about the detected format of the message in the compose window so that the backend code can potentially make better decisions.
Thanks for driving this forward.

::: mail/locales/en-US/chrome/messenger/preferences/sendoptions.dtd
@@ +5,5 @@
>  <!ENTITY dialog.title                 "Send Options">
>  <!ENTITY sendMail.title               "Text Format">
> +<!ENTITY autoDowngrade.label          "Send the message as plain text if possible">
> +<!ENTITY autoDowngrade.accesskey      "t">
> +<!ENTITY autoDowngrade.tooltiptext    "If the message has practically no HTML formatting, automatically convert to plain text before sending">

Do we actually use tooltips in prefs dialog to convey further information? This needs UI review to check consistency (other than from the patch author:)).

::: mailnews/compose/src/nsMsgCompose.cpp
@@ +4974,5 @@
> +
> +  // Message-centric Auto-Downgrade:
> +  // If the message has practically no HTML formatting,
> +  // AND if user accepts auto-downgrading (send options pref),
> +  // bypass auto-detection of recipients' preferences and just

So if this bypasses all recipient detection, can it be placed at the top of the function to avoid looping over the recipients?

@@ +4982,5 @@
> +  bool convertiblePlain;
> +  rv = prefService->GetBoolPref("mailnews.sendformat.auto_downgrade", &autoDowngrade);
> +  NS_ENSURE_SUCCESS(rv, rv);
> +  convertiblePlain = (aConvertible == nsIMsgCompConvertible::Plain);
> +  if (autoDowngrade && convertiblePlain)

The convertiblePlain variable is not used anywhere else so you can probaly fold this into:
'if (autoDowngrade && (aConvertible == nsIMsgCompConvertible::Plain))'

@@ -4992,5 @@
>    NS_ENSURE_SUCCESS(rv, rv);
>  
> -  // If the action is a known action, return that value. Otherwise, ask the
> -  // user. Note that the preference defaults to 0, which is not a valid value
> -  // for the enum.

I think the comment about 0 being default was useful. Why do you need to remove it?

::: mailnews/mailnews.js
@@ +172,5 @@
> +// true:  If the message has practically no HTML formatting, bypass recipient-centric
> +//        auto-detection of delivery format; auto-downgrade and silently send as plain text.
> +// false: Don't auto-downgrade; use recipient-centric auto-detection of best delivery format,
> +//        including send options.
> +pref("mailnews.sendformat.auto_downgrade", true); 

Trailing space.
Attachment #8683673 - Flags: feedback?(acelists) → feedback+
(In reply to :aceman from comment #139)
> Comment on attachment 8683673 [details] [diff] [review]
> WIP patch v.4.1: Minimal fix: Make auto-downgrade optional; use send options
> for {allPlain} recipient scope if (convertible!=plain || AutoDowngrade=false)
> 
> Review of attachment 8683673 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Yes, this seems fine to me as an alternative to what I have implemented. It
> tries to do the same at different place in code. And it does not lie about
> the detected format of the message in the compose window so that the backend
> code can potentially make better decisions.

Exactly.

> Thanks for driving this forward.

My pleasure (for real)!

> ::: mail/locales/en-US/chrome/messenger/preferences/sendoptions.dtd
> @@ +5,5 @@
> >  <!ENTITY dialog.title                 "Send Options">
> >  <!ENTITY sendMail.title               "Text Format">
> > +<!ENTITY autoDowngrade.label          "Send the message as plain text if possible">
> > +<!ENTITY autoDowngrade.accesskey      "t">
> > +<!ENTITY autoDowngrade.tooltiptext    "If the message has practically no HTML formatting, automatically convert to plain text before sending">
> 
> Do we actually use tooltips in prefs dialog to convey further information?
> This needs UI review to check consistency (other than from the patch
> author:)).

As seen on bugs, opinions about the meaning of "Send the message as plain text if possible" can deviate strongly. So the label is not sufficient to understand this option. So if we don't have the tooltip, we'd have to rephrase the label along the tooltip text to make it clear.
I like the short label proposed by Magnus, and I think offering more detailed information in tooltip will present that information exactly when you need it. Otherwise we could add an info icon (flat button) to indicate there's more, and hook the tooltip to that.

> ::: mailnews/compose/src/nsMsgCompose.cpp
> @@ +4974,5 @@
> > +
> > +  // Message-centric Auto-Downgrade:
> > +  // If the message has practically no HTML formatting,
> > +  // AND if user accepts auto-downgrading (send options pref),
> > +  // bypass auto-detection of recipients' preferences and just
> 
> So if this bypasses all recipient detection, can it be placed at the top of
> the function to avoid looping over the recipients?

Wow, yes, that's a brilliant idea. Realized in attachment 8689411 [details] [diff] [review].
I pushed this even on top of newsgroup section, so we're helping newsgroups too by avoiding to ask for no reason, if auto-Downgrade==true (user pref, this bug) and isConvertible::Plain.

> @@ +4982,5 @@
> > +  bool convertiblePlain;
> > +  rv = prefService->GetBoolPref("mailnews.sendformat.auto_downgrade", &autoDowngrade);
> > +  NS_ENSURE_SUCCESS(rv, rv);
> > +  convertiblePlain = (aConvertible == nsIMsgCompConvertible::Plain);
> > +  if (autoDowngrade && convertiblePlain)
> 
> The convertiblePlain variable is not used anywhere else so you can probaly
> fold this into:
> 'if (autoDowngrade && (aConvertible == nsIMsgCompConvertible::Plain))'

ok

> @@ -4992,5 @@
> >    NS_ENSURE_SUCCESS(rv, rv);
> >  
> > -  // If the action is a known action, return that value. Otherwise, ask the
> > -  // user. Note that the preference defaults to 0, which is not a valid value
> > -  // for the enum.
> 
> I think the comment about 0 being default was useful. Why do you need to
> remove it?

Because the preference does NOT default to 0, neither before nor after my patch.
Since long, we've defaulted to 3 (Send both).

> ::: mailnews/mailnews.js
> @@ +172,5 @@
> > +// true:  If the message has practically no HTML formatting, bypass recipient-centric
> > +//        auto-detection of delivery format; auto-downgrade and silently send as plain text.
> > +// false: Don't auto-downgrade; use recipient-centric auto-detection of best delivery format,
> > +//        including send options.
> > +pref("mailnews.sendformat.auto_downgrade", true); 
> 
> Trailing space.

Removed in attachment 8689411 [details] [diff] [review].
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #140)
> (In reply to :aceman from comment #139)
> > Comment on attachment 8683673 [details] [diff] [review]
> > WIP patch v.4.1: Minimal fix: Make auto-downgrade optional; use send options
> > for {allPlain} recipient scope if (convertible!=plain || AutoDowngrade=false)
> > 
> > Review of attachment 8683673 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > ::: mail/locales/en-US/chrome/messenger/preferences/sendoptions.dtd
> > @@ +5,5 @@
> > >  <!ENTITY dialog.title                 "Send Options">
> > >  <!ENTITY sendMail.title               "Text Format">
> > > +<!ENTITY autoDowngrade.label          "Send the message as plain text if possible">
> > > +<!ENTITY autoDowngrade.accesskey      "t">
> > > +<!ENTITY autoDowngrade.tooltiptext    "If the message has practically no HTML formatting, automatically convert to plain text before sending">
> > 
> > Do we actually use tooltips in prefs dialog to convey further information?
> > This needs UI review to check consistency (other than from the patch
> > author:)).
> 
> As seen on bugs, opinions about the meaning of "Send the message as plain
> text if possible" can deviate strongly. So the label is not sufficient to
> understand this option. So if we don't have the tooltip, we'd have to
> rephrase the label along the tooltip text to make it clear.
> I like the short label proposed by Magnus, and I think offering more
> detailed information in tooltip will present that information exactly when
> you need it. Otherwise we could add an info icon (flat button) to indicate
> there's more, and hook the tooltip to that.

The other options have a explanation text above. This can also be done here.
(In reply to Richard Marti (:Paenglab) from comment #141)
> (In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment
> > I like the short label proposed by Magnus, and I think offering more
> > detailed information in tooltip will present that information exactly when
> > you need it. Otherwise we could add an info icon (flat button) to indicate
> > there's more, and hook the tooltip to that.
> 
> The other options have a explanation text above. This can also be done here.

Well, having the checkbox label AND explanation text would be redundant, because the label IS the explanation. Long explanations are permanent clutter in the UI, and space is already a scarce resource in this dialogue (also because of all the explanations). Keeping the checkbox label short and crisp is also beneficial for easy reference in documentation and support.

By inherent design, tooltips provide more detailed information when hovering over controls.
E.g., [Reply All] button is complemented by a more informative tooltip "Reply to sender and all recipients". Anything wrong or disadvantegous with having the tooltip?

Current patch:
[x] Send the message as plain text if possible

I think I'd still prefer Magnus' short variant plus my tooltip, but if others disagree, here's a compromise proposal:

  [x] Send the message as plain text if it has practically no HTML formatting

We might be lucky that this will still fit into a single line.
For comparison, current patch has:
> [x] Send the message as plain text if possible
> tooltip="If the message has practically no HTML formatting, automatically convert to plain text before sending"
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #142)
> (In reply to Richard Marti (:Paenglab) from comment #141)
> > (In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment
> > > I like the short label proposed by Magnus, and I think offering more
> > > detailed information in tooltip will present that information exactly when
> > > you need it. Otherwise we could add an info icon (flat button) to indicate
> > > there's more, and hook the tooltip to that.
> > 
> > The other options have a explanation text above. This can also be done here.
> 
> Well, having the checkbox label AND explanation text would be redundant,
> because the label IS the explanation. Long explanations are permanent
> clutter in the UI, and space is already a scarce resource in this dialogue
> (also because of all the explanations). Keeping the checkbox label short and
> crisp is also beneficial for easy reference in documentation and support.
> 
> By inherent design, tooltips provide more detailed information when hovering
> over controls.
> E.g., [Reply All] button is complemented by a more informative tooltip
> "Reply to sender and all recipients". Anything wrong or disadvantegous with
> having the tooltip?

It would be not consistent (and surprising) as tooltips aren't used in options window.
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #140)
> Wow, yes, that's a brilliant idea. Realized in attachment 8689411 [details] [diff] [review]
> [diff] [review].
> I pushed this even on top of newsgroup section, so we're helping newsgroups
> too by avoiding to ask for no reason, if auto-Downgrade==true (user pref,
> this bug) and isConvertible::Plain.
I'm not sure fixing problems in one patch in a different patch in another bug will be accepted by the reviewer. But I understand the patches change the same place so fixing it properly is hard :)

> > > -  // If the action is a known action, return that value. Otherwise, ask the
> > > -  // user. Note that the preference defaults to 0, which is not a valid value
> > > -  // for the enum.
> > 
> > I think the comment about 0 being default was useful. Why do you need to
> > remove it?
> 
> Because the preference does NOT default to 0, neither before nor after my
> patch.
> Since long, we've defaulted to 3 (Send both).
OK, but Seamonkey defaults to 0 in mailnews.js. So you can reword the comment, like "Note the preference may default to 0..." .
Comment on attachment 8683673 [details] [diff] [review]
WIP patch v.4.1: Minimal fix: Make auto-downgrade optional; use send options for {allPlain} recipient scope if (convertible!=plain || AutoDowngrade=false)

Review of attachment 8683673 [details] [diff] [review]:
-----------------------------------------------------------------

Please drop the tooltip, and fix the stuff aceman pointed out. (Yes, you need to put what belongs in this bug in this bug, I'm not all the enthusiastic about bug 1222176...)
Attachment #8683673 - Flags: feedback?(mkmelin+mozilla) → feedback+
all *that*

Also, the patch can't be applied (at least using hg qimport bz:136502)
Seems there's some encoding error in the file.
> abort: decoding near 'Thomas D▒llmann <b': 'utf8' codec can't decode byte 0xfc in position 8: invalid start byte!

Looks like qimport is a bit picky and doesn't like ISO8859-1 encoding...
Either make it utf-8 or use applicable transcription rules into ASCII.
Auto-Downgrade checkbox label

Richard, Aceman, how's this?

[x] Send as plain text if the message has practically no HTML formatting

As short as possible, main action first (Send as plain text), and precise yet understandable description of the condition (if the message has practically no HTML formatting).
I think I still prefer the original wording. Yes we talk about HTML in this dialog, but without the context of this bug "practically no HTML formatting" is probably just crazy tech gibberish to the average user.
I'm thinking like Magnus, use the original wording. What if you use only one HTML formatting like one which is causing bug 1202276? This is practically no HTML.
(In reply to Magnus Melin from comment #149)
> I think I still prefer the original wording. Yes we talk about HTML in this
> dialog, but without the context of this bug "practically no HTML formatting"
> is probably just crazy tech gibberish to the average user.

Well, I'm not sure if this 4th-level option is really for the average user...
So I'm still not convinced that the meaning of "if possible" is sufficiently clear (more so as in the past, we allowed uncle Ben to force his ideosyncratic ideas of "if possible" onto everyone).
But maybe less is more in this case...

(In reply to Richard Marti (:Paenglab) from comment #150)
> I'm thinking like Magnus, use the original wording. What if you use only one
> HTML formatting like one which is causing bug 1202276? This is practically
> no HTML.

I suggested "practically no HTML *formatting*", so it's not about the amount of HTML, but about consequential formatting. More like Bug 584313. But you are right, even my wording might be misread in a quantitative way. So the more generic description might avoid that. In fact, it's hard to describe the conditions for Auto-Downgrade, which perhaps points to the fact that it's a very arguable feature. So it's high time we fix this bug and allow users to opt out of this...

OK, so we'll go for Magnus proposal:

[x] Send as plain text if possible

Nice. I actually like it. Easy to cite for documentation and support, too. Documentation might be needed.
All right, here we go. :)

Minimal fix v. 5 addresses all feedback, now even more minimalistic. Featuring:

a) Tooltipless Auto-Downgrade checkbox: [x] Send the message as plain text if possible. Sweet and simple. Let users rule.
b) Fully message-centric Auto-Downgrade behaviour (ignore any recipient preferences when auto-Downgrade==true && isConvertible::Plain); as mentioned by Aceman, this comes with performance improvements as we can skip recipient iteration in this case!
c) Auto-Downgrade messages to newsgroups, too - no more useless nagging with 'HTML mail question' when the message is effectively plaintext.

Reduced patch to get this landed soon and safely:
This patch preserves the current dataloss behaviour for (allPlain) scope as-is:
When all recipients are prefers-plain, force-downgrade the message by stripping all HTML formatting regardless of (non-)convertibility.
We need bug 1222176 before we can use send option for {allPlain} recipient scope if (convertible!=plain || AutoDowngrade=false).
Anyway, there's no need to fix allPlain behaviour in this bug.
Attachment #8683673 - Attachment is obsolete: true
Attachment #8690421 - Flags: ui-review?(richard.marti)
Attachment #8690421 - Flags: review?(mkmelin+mozilla)
Build fails with this error:
 0:59.46 nsMsgCompose.obj
 1:01.62 Note: rebuild with "c:/mozilla-build/mozmake/mozmake.EXE VERBOSE=1 all-local" to show all compiler parameters.
 1:04.84 nsMsgCompose.cpp
 1:04.84 z:/Mozilla/comm-central/mailnews/compose/src/nsMsgCompose.cpp(4884) : error C2065: 'rv' : undeclared identifier
 1:04.84 z:/Mozilla/comm-central/mailnews/compose/src/nsMsgCompose.cpp(4885) : error C2065: 'rv' : undeclared identifier
 1:04.84 z:/Mozilla/comm-central/mailnews/compose/src/nsMsgCompose.cpp(4896) : error C2065: 'rv' : undeclared identifier
 1:04.84 z:/Mozilla/comm-central/mailnews/compose/src/nsMsgCompose.cpp(4897) : error C2065: 'rv' : undeclared identifier
 1:04.84 z:/Mozilla/comm-central/mailnews/compose/src/nsMsgCompose.cpp(5036) : error C2065: 'prefService' : undeclared identifier
 1:04.84 z:/Mozilla/comm-central/mailnews/compose/src/nsMsgCompose.cpp(5036) : error C2227: left of '->GetIntPref' must point to class/struct/union/generic type
 1:04.85         type is 'unknown-type'
(In reply to Richard Marti (:Paenglab) from comment #153)
> Build fails with this error:
>  0:59.46 nsMsgCompose.obj
>  1:01.62 Note: rebuild with "c:/mozilla-build/mozmake/mozmake.EXE VERBOSE=1
> all-local" to show all compiler parameters.
>  1:04.84 nsMsgCompose.cpp
>  1:04.84 z:/Mozilla/comm-central/mailnews/compose/src/nsMsgCompose.cpp(4884)
> : error C2065: 'rv' : undeclared identifier

I have no idea what could cause this, nor how to fix this.
Aceman told me to use prefBranch instead of prefService, to avoid creating two identical objects with different names.
I just copied what was there, and pushed prefBranch creation to the top of the function.
Please someone assist.

>       // Initialize prefService
> 4884  nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
>       NS_ENSURE_SUCCESS(rv, rv);
I removed this line further down which surprisingly does NOT have &rv in the object creation...
Because per Aceman's comment, I thought it's enough to create prefBranch object once at the top and then reuse that within our function.

> 4897  nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID));
> 4898  if (prefBranch)
>       {
>         NS_GetUnicharPreferenceWithDefault(prefBranch, "mailnews.plaintext_domains",
>                                            EmptyString(), plaintextDomains);

But other similar calls in other functions of same file have it:

> 92  *reply_header_type = 0;
> 93 nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
> 94  NS_ENSURE_SUCCESS(rv, rv);

I also removed this line from further down, again to merge with into that top prefbranch object creation:

>      // Otherwise, check the preference to see what action we should default to.
> 4977 nsCOMPtr<nsIPrefBranch> prefService(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
>      NS_ENSURE_SUCCESS(rv, rv);
>
>  int32_t action = nsIMsgCompSendFormat::AskUser;
>  rv = prefService->GetIntPref("mail.default_html_action", &action);
>  NS_ENSURE_SUCCESS(rv, rv);
rv is used before it's declared here: nsresult rv = LookupAddressBook(recipientsList);
Put nsresult nv; somewhere at the top before first use.

  nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
                          ^^^^^^^^^^
The variable declared here is prefBranch. But the code uses 'prefService'.
Change this to
nsCOMPtr<nsIPrefBranch> prefService(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
or change this
  rv = prefService->GetIntPref("mail.default_html_action", &action);
to this
  rv = prefBranch->GetIntPref("mail.default_html_action", &action);

Reading the compiler messages helps ;-)
(In reply to Jorg K (GMT+1) from comment #156)
> rv is used before it's declared here: nsresult rv =
> LookupAddressBook(recipientsList);

Hey, thanks! :) That explains it...

> Put nsresult nv; somewhere at the top before first use.

nsresult nv;   won't solve the problem but I'll try
nsresult rv;   :p

>   nsCOMPtr<nsIPrefBranch>
> prefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
>                           ^^^^^^^^^^
> The variable declared here is prefBranch. But the code uses 'prefService'.
> Change this to
> nsCOMPtr<nsIPrefBranch> prefService(do_GetService(NS_PREFSERVICE_CONTRACTID,
> &rv));
> or change this
>   rv = prefService->GetIntPref("mail.default_html_action", &action);
> to this
>   rv = prefBranch->GetIntPref("mail.default_html_action", &action);

Yeah, my mistake!

> Reading the compiler messages helps ;-)

I'm not familiar with compiler errors nor c++ coding conventions, but indeed close reading always helps! ;)
Fix compiler error of Comment 153 because of undeclared identifier 'rv'.

Reviewers, pls see Comment 152 for short description.

Bonus (same in last patch):
d) Streamlined code with better annotations.

Hope it works now...
Attachment #8690421 - Attachment is obsolete: true
Attachment #8690421 - Flags: ui-review?(richard.marti)
Attachment #8690421 - Flags: review?(mkmelin+mozilla)
Attachment #8690447 - Flags: ui-review?(richard.marti)
Attachment #8690447 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8690447 [details] [diff] [review]
Patch v.6: Minimal fix: Make Auto-Downgrade optional

It build now.

ui-r+ for the pref UI.
Attachment #8690447 - Flags: ui-review?(richard.marti) → ui-review+
Can someone please attach a screenshot to the what has changed in the UI.
Attached image Screenshot
Since no one has done the screenshot for me, I've done it myself. Great!
But doesn't seem to work. Setting the preference will tick/untick the box, but ticking/unticking the box doesn't change the preference value.
Good start, needs more work.
(In reply to Jorg K (GMT+1) [impatiently awaiting nine reviews] from comment #161)
> Created attachment 8690836 [details]
> Screenshot
> 
> Since no one has done the screenshot for me, I've done it myself.

Two screenshots were already available on bug 1222176, attachment 8690530 [details] / attachment 8690677 [details], and you had already studied the UI for 1 hour as you claimed in Bug 1222176 Comment 45. Since you are waiting for 9 reviews, I assume your code reading skills are sufficient to extract that patch attachment 8690447 [details] [diff] [review] adds a single simple checkbox with the following label:
<!ENTITY autoDowngrade.label          "Send the message as plain text if possible">.
Bug 1222176 also makes it very clear that patches are on top of this bug, so to find that label on those screenshots isn't very hard ;)
Besides, we're volunteers, so it's a bit odd to order your screenshot in comment 160 and then say only 6 hours later in comment 161 that "no one has done the screenshot for me".

> Great!
> But doesn't seem to work. Setting the preference will tick/untick the box,
> but ticking/unticking the box doesn't change the preference value.
> Good start, needs more work.

Fyi, imo the tone of that isn't clear and might be misunderstood as ironic or arrogant. Or is it sincere praise paired with a cool and factual "how to move on from here" advice? Asking questions like "Did I miss a trick to make this work?" might state the same thing in a better way. Given the history of this, like dozens of bugs with hundreds of comments, and thousands of users downloading "Always HTML" addon to fix exactly this problem, and years of analysis to find out how to fix this, against stubborn denial and aggressive resistance from the developer who once coded this, this isn't just a "good start". More like revolutionary for people affected by and desparate about the gaps in the old design, where you change all your settings to HTML, you compose in HTML all the way long (even add formatting like simple centered paragraphs or CSS styles on certain everyday tags like <a>) but the freaking thing just doesn't send HTML for unknown reasons and beyond user's control. Plus you may not even notice the distortion of your message because it happens silently after sending, so you'd have to get feedback from recipients or verify from your sent box.

Moreover, you're wrong. Sometimes it's good to slow down and check out the details before rushing to conclusions. I'm running your try-build and it works exactly as it should.

You just have to confirm *both* the Send Options dialogue AND the main options dialogue with *OK* after changing your settings, and the preference value will change and work flawlessly. We may not like that design (requiring to confirm both dialogues), but it's arguably a reasonable behaviour because you've triggered "Send Options" from "Main Options" so if you cancel "Main Options", it probably shouldn't change anything. You're free to have different expectations, but that's outside the scope of this bug, because I haven't changed that behaviour; it's the same for the existing dropdown box "When sending messages in HTML format...". Btw, your screenshot is very misleading because that dropdown box which is open on the screenshot has nothing to do with this patch here, only the checkbox.
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #162)

"Great" was said in a flippant way. I apologise. You should know me by now.
Always helpful but at times a little ironic ;-)

Each bug has to stand independently, the screenshot of another bug doesn't help me unless I study the code or compile it myself.

> You just have to confirm *both* the Send Options dialogue AND the main
> options dialogue with *OK* after changing your settings, and the preference
> value will change and work flawlessly.
My mistake. I apologise. Yes, when you dismiss both, the preference updates. Great ;-)

> Btw, your
> screenshot is very misleading because that dropdown box which is open on the
> screenshot has nothing to do with this patch here, only the checkbox.
It shows the full UX regardless of which bit comes from where.
I just wanted to show that you can screen-capture drop-downs.
Comment on attachment 8690447 [details] [diff] [review]
Patch v.6: Minimal fix: Make Auto-Downgrade optional

Review of attachment 8690447 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/compose/src/nsMsgCompose.cpp
@@ +4880,5 @@
>  {
>    NS_ENSURE_ARG_POINTER(result);
> +  nsresult rv;
> +
> +  // Initialize prefService

please remove this comment (it's obvious what the next line does)

@@ +4890,5 @@
> +  // AND if user accepts auto-downgrading (send options pref),
> +  // bypass auto-detection of recipients' preferences and just
> +  // send the message as plain text (silent, "lossless" conversion);
> +  // which will also avoid asking for newsgroups for this typical scenario.
> +  // The initial value of autoDowngrade will be overwritten by pref value (default: true).

and remove this last line too (again, it's just telling what the code does, but that's pretty clear)

@@ +4937,5 @@
>  
> +  // allHTML and allPlain are summary recipient scopes of format preference according to
> +  // address book and send options for recipient-centric Auto-Detect, used by
> +  // Auto-Detect to determine the appropriate message delivery format.
> +  // The initial values will be overwritten as we iterate through each recipient.

remove this last line too

@@ +5012,5 @@
> +  // If all recipients prefer plaintext, silently strip *all* HTML formatting,
> +  // regardless of (non-)convertibility, and send the message as plaintext.
> +  // **ToDo: UX-error-prevention, UX-wysiwyg: Force-Downgrading the message comes
> +  // with a massive dataloss potential (loss of formatting, inline images etc.).
> +  // By default, we must at least warn against that.**

WHile I agree with the todo, I don't think you should put that in the code

@@ +5030,5 @@
> +  // resolution options for prefers-plain with default setting for prefers-unknown.
> +  // As a result, both conflict resolution and default setting are crippled.**
> +  // The initial value of action will be overwritten by pref value.
> +  // For SeaMonkey, the pref defaults to 0 (Ask the user which format to send).
> +  // For Thunderbird, the pref defaults to 3 (Send both plain text and HTML).

Please remove all of this comment. I don't think it adds value + you shouldn't describe default prefs for each app in the code - what if it changes?

::: mailnews/mailnews.js
@@ +172,5 @@
> +// true:  If the message has practically no HTML formatting, bypass recipient-centric
> +//        auto-detection of delivery format; auto-downgrade and silently send as plain text.
> +// false: Don't auto-downgrade; use recipient-centric auto-detection of best delivery format,
> +//        including send options.
> +pref("mailnews.sendformat.auto_downgrade", true); 

nit: trailing whitespace
Attachment #8690447 - Flags: review?(mkmelin+mozilla) → feedback+
(In reply to Magnus Melin from comment #164)
> Comment on attachment 8690447 [details] [diff] [review]
> Patch v.6: Minimal fix: Make Auto-Downgrade optional
> Review of attachment 8690447 [details] [diff] [review]:
> -----------------------------------------------------------------

Thanks for the feedback.
This patch addresses the issues of comment 164: All comments mentioned have been tamed.

I kept a one-line to-do comment about the dataloss case of allPlain; it has always been good practice and helpful for the team of volunteer developers to have a small hint at the right spot in the code about identified problems, and it doesn't hurt anyone.

I've also morphed my lengthy critical comment about "mail.default_html_action" pref into a minimalist factual description of what this pref actually does.
I think that's very important and valuable for understanding this code and the resulting UX, more so in view of the sometimes insufficient understanding or even popular misconceptions that we have seen about this.
Attachment #8690447 - Attachment is obsolete: true
Attachment #8692154 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8692154 [details] [diff] [review]
Patch v.6.1 (comments clipped): Minimal fix: Make Auto-Downgrade optional

Review of attachment 8692154 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. r=mkmelin

I'll make these changes and check it in.

::: mail/locales/en-US/chrome/messenger/preferences/sendoptions.dtd
@@ +4,4 @@
>  
>  <!ENTITY dialog.title                 "Send Options">
>  <!ENTITY sendMail.title               "Text Format">
> +<!ENTITY autoDowngrade.label          "Send the message as plain text if possible">

"Send messages ... " is better (and consistent with the other language in the dialog main text)

::: mailnews/mailnews.js
@@ +172,5 @@
> +// true:  If the message has practically no HTML formatting, bypass recipient-centric
> +//        auto-detection of delivery format; auto-downgrade and silently send as plain text.
> +// false: Don't auto-downgrade; use recipient-centric auto-detection of best delivery format,
> +//        including send options.
> +pref("mailnews.sendformat.auto_downgrade", true); 

nit: trailing space
Attachment #8692154 - Flags: review?(mkmelin+mozilla) → review+
https://hg.mozilla.org/comm-central/rev/c4f064a83669 -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 45.0
Wow! Thanks for landing this. So good to see this fixed...

FTR: Final patch v. 6 has ui-review+ by :paenglab.

So starting from TB release version 45, planned for 8 March 2016, Auto-Downgrade will be optional/user-controllable via deeply-nested options UI.

This is the minimal fix indeed. Imho, still much too hidden, so mystery/surprise will remain for many users unless they dig themselves into 4th level send options. But at least the net effect of very popular "Always HTML" addon is now available from that inbuilt checkbox which we implemented here. Note that neither the addon nor the patch here guarantees 'always HTML', but just 'use action by user-defined send option without Auto-Downgrade interference'. The option/pref implemented here, "[x] Send messages as plaintext when possible", works technically more correct than 'Always HTML' addon because it does not fake an incorrect convertibility degree of the message, but just switches Auto-Downgrade on or off directly.

Thanks to everyone who contributed to make this fix possible, after all these years, especially WADA and Aceman, and Magnus for the green light into the product. Let's hope that this is just the beginninig of a series of improvements to tame delivery format 'auto-detect' into a more reasonable, more user-controlled, more predictable, and optional feature.

Note that this bug has not yet implemented the possibility of bypassing recipient-centric delivery format 'Auto-Detect' altogether and chose a stable default message delivery format instead, to start new compositions directly with delivery format HTML | both | plain(?).
Whiteboard: [GS][patchlove][has draft patch] Workaround: https://addons.mozilla.org/de/thunderbird/addon/always-html/ → [GS] Workaround < TB45: https://addons.mozilla.org/de/thunderbird/addon/always-html/
Blocks: 1228846
Blocks: 1229270
See Also: → 766860
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: