Bug 302748 - HTML+plain-text (multipart) email message with wrong encoding does not upgrade to UTF-8 properly. r=irving
authorMagnus Melin <mkmelin+mozilla@iki.fi>
Thu, 17 Oct 2013 13:51:06 +0300
changeset 13157 8014698e4cd4f872cbb7ef5b10f1571e7f388339
parent 13156 3a1898b9a844114b40571ba037415a35c8b4e051
child 13158 05c27e13899d6d42efc6097ac042e9d6a125fccf
push id9569
push usermkmelin@iki.fi
push dateThu, 17 Oct 2013 10:53:22 +0000
treeherdercomm-central@8014698e4cd4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersirving
bugs302748
Bug 302748 - HTML+plain-text (multipart) email message with wrong encoding does not upgrade to UTF-8 properly. r=irving Characters outside current charset are converted into entities - should use utf-8 instead.
mail/test/mozmill/composition/test-charset-upgrade.js
mailnews/base/util/nsMsgI18N.cpp
mailnews/compose/src/nsMsgCompose.cpp
mailnews/compose/src/nsMsgCompose.h
mailnews/compose/src/nsMsgSend.cpp
new file mode 100644
--- /dev/null
+++ b/mail/test/mozmill/composition/test-charset-upgrade.js
@@ -0,0 +1,234 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+/**
+ * Tests that we do the right thing wrt. message encoding, especially when
+ * all characters doesn't fit the selected charset.
+ */
+
+// make SOLO_TEST=composition/test-charset-upgrade.js mozmill-one
+
+const MODULE_NAME = "test-charset-upgrade";
+
+const RELATIVE_ROOT = "../shared-modules";
+const MODULE_REQUIRES = ["folder-display-helpers", "window-helpers", "compose-helpers"];
+
+var os = {};
+Cu.import("resource://mozmill/stdlib/os.js", os);
+Cu.import('resource://gre/modules/Services.jsm');
+Cu.import("resource:///modules/mailServices.js");
+var elib = {};
+Cu.import("resource://mozmill/modules/elementslib.js", elib);
+
+var draftsFolder;
+var outboxFolder;
+
+function setupModule(module) {
+  for (let req of MODULE_REQUIRES) {
+    collector.getModule(req).installInto(module);
+  }
+  if (!MailServices.accounts
+                   .localFoldersServer
+                   .rootFolder
+                   .containsChildNamed("Drafts")) {
+     create_folder("Drafts", [Ci.nsMsgFolderFlags.Drafts]);
+  }
+  draftsFolder = MailServices.accounts
+                             .localFoldersServer
+                             .rootFolder
+                             .getChildNamed("Drafts");
+  if (!draftsFolder)
+    throw new Error("draftsFolder not found");
+
+  if (!MailServices.accounts
+                   .localFoldersServer
+                   .rootFolder
+                   .containsChildNamed("Outbox")) {
+     create_folder("Outbox", [Ci.nsMsgFolderFlags.Outbox]);
+  }
+  outboxFolder = MailServices.accounts
+                             .localFoldersServer
+                             .rootFolder
+                             .getChildNamed("Outbox");
+  if (!outboxFolder)
+    throw new Error("outboxFolder not found");
+}
+
+/**
+ * Helper to get the full message content.
+ *
+ * @param aMsgHdr: nsIMsgDBHdr object whose text body will be read
+ * @return string with full message source
+ */
+function getMsgSource(aMsgHdr) {
+  let msgFolder = aMsgHdr.folder;
+  let msgUri = msgFolder.getUriForMsg(aMsgHdr);
+
+  let messenger = Cc["@mozilla.org/messenger;1"]
+                    .createInstance(Ci.nsIMessenger);
+  let streamListener = Cc["@mozilla.org/network/sync-stream-listener;1"]
+                         .createInstance(Ci.nsISyncStreamListener);
+  messenger.messageServiceFromURI(msgUri).streamMessage(msgUri,
+                                                        streamListener,
+                                                        null,
+                                                        null,
+                                                        false,
+                                                        "",
+                                                        false);
+  let sis = Cc["@mozilla.org/scriptableinputstream;1"]
+              .createInstance(Ci.nsIScriptableInputStream);
+  sis.init(streamListener.inputStream);
+  const MAX_MESSAGE_LENGTH = 65536;
+  let content = sis.read(MAX_MESSAGE_LENGTH);
+
+  return Cc["@mozilla.org/intl/utf8converterservice;1"]
+           .getService(Ci.nsIUTF8ConverterService)
+           .convertURISpecToUTF8(content, "UTF-8");
+}
+
+/**
+ * Test that if all characters don't fit the current charset selection,
+ * we upgrade properly to UTF-8. In HTML composition.
+ */
+function test_encoding_upgrade_html_compose() {
+  let compWin = open_compose_new_mail();
+
+  compWin.type(null, "someone@example.com");
+  compWin.type(compWin.eid("msgSubject"), "encoding upgrade test - html mode")
+  compWin.type(compWin.eid("content-frame"), "so far, this is latin1\n");
+
+  // Ctrl+S = save as draft.
+  compWin.keypress(null, "s", {shiftKey: false, accelKey: true});
+
+  be_in_folder(draftsFolder);
+  let draftMsg = select_click_row(0);
+
+  // Charset should still be the default.
+  assert_equals(draftMsg.Charset, "ISO-8859-1");
+
+  let draftMsgContent = getMsgSource(draftMsg);
+  if (!draftMsgContent.contains('content="text/html; charset=ISO-8859-1"'))
+    throw new Error("Expected content type not in msg; draftMsgContent=" +
+                    draftMsgContent);
+
+  const CHINESE = "漢皇重色思傾國漢皇重色思傾國";
+  compWin.type(compWin.eid("content-frame"),
+    "but now, we enter some chinese: " + CHINESE +"\n");
+
+  // Ctrl+U = Underline (so we can check multipart/alternative gets right,
+  // without it html->plaintext conversion will it as send plain text only)
+  compWin.keypress(null, "U", {shiftKey: false, accelKey: true});
+
+  compWin.type(compWin.eid("content-frame"),
+    "content need to be upgraded to utf-8 now.");
+
+  // Ctrl+S = save as draft.
+  compWin.keypress(null, "s", {shiftKey: false, accelKey: true});
+
+  be_in_folder(draftsFolder);
+  let draftMsg2 = select_click_row(0);
+  // Charset should have be upgraded to UTF-8.
+  assert_equals(draftMsg2.Charset, "UTF-8");
+
+  let draftMsg2Content = getMsgSource(draftMsg2);
+  if (!draftMsg2Content.contains('content="text/html; charset=UTF-8"'))
+    throw new Error("Expected content type not in msg; draftMsg2Content=" +
+                    draftMsg2Content);
+
+  if (!draftMsg2Content.contains(CHINESE))
+    throw new Error("Chinese text not in msg; CHINESE=" + CHINESE +
+                    ", draftMsg2Content=" + draftMsg2Content);
+
+  // Ctrl+Shift+Return = Send Later
+  compWin.keypress(null, "VK_RETURN", {shiftKey: true, accelKey: true});
+
+  be_in_folder(outboxFolder);
+  let outMsg = select_click_row(0);
+  let outMsgContent = getMsgSource(outMsg);
+
+  // This message should be multipart/alternative.
+  if (!outMsgContent.contains("Content-Type: multipart/alternative"))
+    throw new Error("Expected multipart/alternative; content=" + outMsgContent);
+
+  let chinesePlainIdx = outMsgContent.indexOf(CHINESE);
+  assert_true(chinesePlainIdx > 0, "chinesePlainIdx=" + chinesePlainIdx +
+                                   ", outMsgContent=" + outMsgContent);
+
+  let chineseHTMLIdx = outMsgContent.indexOf(CHINESE, chinesePlainIdx);
+  assert_true(chineseHTMLIdx > 0, "chineseHTMLIdx=" + chineseHTMLIdx +
+                                  ", outMsgContent=" + outMsgContent);
+
+  // Make sure the actual html also got the content type set correctly.
+  if (!outMsgContent.contains('content="text/html; charset=UTF-8"'))
+    throw new Error("Expected content type not in html; outMsgContent=" +
+                    outMsgContent);
+
+  press_delete(); // Delete the msg from Outbox.
+}
+
+/**
+ * Test that if all characters don't fit the current charset selection,
+ * we upgrade properly to UTF-8. In plaintext composition.
+ */
+function test_encoding_upgrade_plaintext_compose() {
+  Services.prefs.setBoolPref("mail.identity.default.compose_html", false);
+  let compWin = open_compose_new_mail();
+  Services.prefs.setBoolPref("mail.identity.default.compose_html", true);
+
+  compWin.type(null, "someone-else@example.com");
+  compWin.type(compWin.eid("msgSubject"), "encoding upgrade test - plaintext");
+  compWin.type(compWin.eid("content-frame"), "this is plaintext latin1\n");
+
+  // Ctrl+S = Save as Draft.
+  compWin.keypress(null, "s", {shiftKey: false, accelKey: true});
+
+  be_in_folder(draftsFolder);
+  let draftMsg = select_click_row(0);
+
+  // Charset should still be the default.
+  assert_equals(draftMsg.Charset, "ISO-8859-1");
+
+  const CHINESE = "漢皇重色思傾國漢皇重色思傾國";
+  compWin.type(compWin.eid("content-frame"),
+    "enter some plain text chinese: " + CHINESE +"\n");
+
+  compWin.type(compWin.eid("content-frame"),
+    "content need to be upgraded to utf-8 now.");
+
+  // Ctrl+S = Save as Draft.
+  compWin.keypress(null, "s", {shiftKey: false, accelKey: true});
+
+  be_in_folder(draftsFolder);
+  let draftMsg2 = select_click_row(0);
+  // Charset should have be upgraded to UTF-8.
+  assert_equals(draftMsg2.Charset, "UTF-8");
+
+  let draftMsg2Content = getMsgSource(draftMsg2);
+  if (draftMsg2Content.contains("<html>"))
+    throw new Error("Plaintext draft contained <html>; "+
+                    "draftMsg2Content=" + draftMsg2Content);
+
+  if (!draftMsg2Content.contains(CHINESE))
+    throw new Error("Chinese text not in msg; CHINESE=" + CHINESE +
+                    ", draftMsg2Content=" + draftMsg2Content);
+
+  // Ctrl+Shift+Return = Send Later.
+  compWin.keypress(null, "VK_RETURN", {shiftKey: true, accelKey: true});
+
+  be_in_folder(outboxFolder);
+  let outMsg = select_click_row(0);
+  let outMsgContent = getMsgSource(outMsg);
+
+  // This message should be text/plain;
+  if (!outMsgContent.contains("Content-Type: text/plain"))
+    throw new Error("Expected text/plain; content=" + outMsgContent);
+
+  if (!outMsgContent.contains(CHINESE))
+    throw new Error("Chinese text not in msg; CHINESE=" + CHINESE +
+                    ", outMsgContent=" + outMsgContent);
+
+  press_delete(); // Delete the msg from Outbox.
+}
+
+
--- a/mailnews/base/util/nsMsgI18N.cpp
+++ b/mailnews/base/util/nsMsgI18N.cpp
@@ -395,35 +395,24 @@ nsresult nsMsgI18NSaveAsCharset(const ch
   nsCOMPtr <nsICharsetConverterManager> ccm =
     do_GetService(NS_CHARSETCONVERTERMANAGER_CONTRACTID, &res);
   NS_ENSURE_SUCCESS(res, res);
 
   nsAutoCString charsetName;
   res = ccm->GetCharsetAlias(charset, charsetName);
   NS_ENSURE_SUCCESS(res, res);
 
-  // charset converter plus entity, NCR generation
   nsCOMPtr <nsISaveAsCharset> conv = do_CreateInstance(NS_SAVEASCHARSET_CONTRACTID, &res);
   NS_ENSURE_SUCCESS(res, res);
 
-  // attribute: 
-  // html text - charset conv then fallback to entity or NCR
-  // plain text - charset conv then fallback to '?'
-  if (bTEXT_HTML)
-    // For ISO-8859-1 only, convert to entity first (always generate entites like &nbsp;).
-    res = conv->Init(charsetName.get(),
-                     charsetName.EqualsLiteral("ISO-8859-1") ?
-                     nsISaveAsCharset::attr_htmlTextDefault :
-                     nsISaveAsCharset::attr_EntityAfterCharsetConv + nsISaveAsCharset::attr_FallbackDecimalNCR, 
-                     nsIEntityConverter::html32);
-  else
-    // fallback for text/plain: first try transliterate then '?'
-    res = conv->Init(charsetName.get(), 
-                     nsISaveAsCharset::attr_FallbackQuestionMark + nsISaveAsCharset::attr_EntityAfterCharsetConv, 
-                     nsIEntityConverter::transliterate);
+  // First try to transliterate, if that fails use '?' for "bad" chars.
+  res = conv->Init(charsetName.get(),
+                   nsISaveAsCharset::attr_FallbackQuestionMark +
+                     nsISaveAsCharset::attr_EntityNone,
+                   nsIEntityConverter::transliterate);
   NS_ENSURE_SUCCESS(res, res);
 
   const PRUnichar *input = inString;
 
   // Convert to charset
   res = conv->Convert(input, outString);
 
   // If the converer cannot encode to the charset,
@@ -452,19 +441,18 @@ nsresult nsMsgI18NSaveAsCharset(const ch
 
     res = conv->Convert(input, outString);
     NS_ENSURE_SUCCESS(res, res);
 
     // get the actual charset used for the conversion
     if (NS_FAILED(conv->GetCharset(fallbackCharset)))
       *fallbackCharset = nullptr;
   }
-  // In case of HTML, non ASCII may be encoded as CER, NCR.
   // Exclude stateful charset which is 7 bit but not ASCII only.
-  else if (isAsciiOnly && bTEXT_HTML && *outString &&
+  else if (isAsciiOnly && *outString &&
            !nsMsgI18Nstateful_charset(charsetName.get()))
     *isAsciiOnly = NS_IsAscii(*outString);
 
   return res;
 }
 
 nsresult nsMsgI18NShrinkUTF8Str(const nsCString &inString,
                                 uint32_t aMaxLength,
--- a/mailnews/compose/src/nsMsgCompose.cpp
+++ b/mailnews/compose/src/nsMsgCompose.cpp
@@ -1021,17 +1021,17 @@ NS_IMETHODIMP nsMsgCompose::AddMsgSendLi
 
 NS_IMETHODIMP nsMsgCompose::RemoveMsgSendListener( nsIMsgSendListener *aMsgSendListener )
 {
   NS_ENSURE_ARG_POINTER(aMsgSendListener);
   return mExternalSendListeners.RemoveElement(aMsgSendListener) ? NS_OK : NS_ERROR_FAILURE;
 }
 
 nsresult nsMsgCompose::_SendMsg(MSG_DeliverMode deliverMode, nsIMsgIdentity *identity,
-                                const char *accountKey, bool entityConversionDone)
+                                const char *accountKey)
 {
   nsresult rv = NS_OK;
 
   // clear saved message id if sending, so we don't send out the same message-id.
   if (deliverMode == nsIMsgCompDeliverMode::Now ||
       deliverMode == nsIMsgCompDeliverMode::Later ||
       deliverMode == nsIMsgCompDeliverMode::Background)
     m_compFields->SetMessageId("");
@@ -1056,41 +1056,16 @@ nsresult nsMsgCompose::_SendMsg(MSG_Deli
     }
 
     m_compFields->SetFrom(sender.IsEmpty() ? email.get() : sender.get());
     m_compFields->SetOrganization(organization);
     mMsgSend = do_CreateInstance(NS_MSGSEND_CONTRACTID);
     if (mMsgSend)
     {
       nsCString bodyString(m_compFields->GetBody());
-      const char  attachment1_type[] = TEXT_HTML;  // we better be "text/html" at this point
-
-      if (!entityConversionDone)
-      {
-        // Convert body to mail charset
-        nsAutoCString outCString;
-
-        if (!bodyString.IsEmpty())
-        {
-          // Apply entity conversion then convert to a mail charset.
-          bool isAsciiOnly;
-          rv = nsMsgI18NSaveAsCharset(attachment1_type, m_compFields->GetCharacterSet(),
-                                      NS_ConvertUTF8toUTF16(bodyString).get(),
-                                      getter_Copies(outCString),
-                                      nullptr, &isAsciiOnly);
-          if (NS_SUCCEEDED(rv))
-          {
-            if (m_compFields->GetForceMsgEncoding())
-              isAsciiOnly = false;
-
-            m_compFields->SetBodyIsAsciiOnly(isAsciiOnly);
-            bodyString = outCString;
-          }
-        }
-      }
 
       // Create the listener for the send operation...
       nsCOMPtr<nsIMsgComposeSendListener> composeSendListener = do_CreateInstance(NS_MSGCOMPOSESENDLISTENER_CONTRACTID);
       if (!composeSendListener)
         return NS_ERROR_OUT_OF_MEMORY;
 
       // right now, AutoSaveAsDraft is identical to SaveAsDraft as
       // far as the msg send code is concerned. This way, we don't have
@@ -1143,97 +1118,103 @@ nsresult nsMsgCompose::_SendMsg(MSG_Deli
   if (NS_FAILED(rv))
     NotifyStateListeners(nsIMsgComposeNotificationType::ComposeProcessDone, rv);
 
   return rv;
 }
 
 NS_IMETHODIMP nsMsgCompose::SendMsg(MSG_DeliverMode deliverMode, nsIMsgIdentity *identity, const char *accountKey, nsIMsgWindow *aMsgWindow, nsIMsgProgress *progress)
 {
+
+  NS_ENSURE_TRUE(m_compFields, NS_ERROR_NOT_INITIALIZED);
   nsresult rv = NS_OK;
-  bool entityConversionDone = false;
   nsCOMPtr<nsIPrompt> prompt;
 
   // i'm assuming the compose window is still up at this point...
   if (!prompt && m_window)
      m_window->GetPrompter(getter_AddRefs(prompt));
 
-  if (m_compFields && !m_composeHTML)
+  // Set content type based on which type of compose window we had.
+  nsString contentType = (m_composeHTML) ? NS_LITERAL_STRING("text/html"):
+                                           NS_LITERAL_STRING("text/plain");
+  nsString msgBody;
+  if (m_editor)
   {
-    // The plain text compose window was used
-    const char contentType[] = "text/plain";
-    nsString msgBody;
-    uint32_t flags = nsIDocumentEncoder::OutputFormatted | nsIDocumentEncoder::OutputCRLineBreak |
-      nsIDocumentEncoder::OutputLFLineBreak;
-    if (m_editor)
+    // Reset message body previously stored in the compose fields
+    // There is 2 nsIMsgCompFields::SetBody() functions using a pointer as argument,
+    // therefore a casting is required.
+    m_compFields->SetBody((const char *)nullptr);
+
+    const char *charset = m_compFields->GetCharacterSet();
+    uint32_t flags = nsIDocumentEncoder::OutputFormatted |
+                     nsIDocumentEncoder::OutputCRLineBreak |
+                     nsIDocumentEncoder::OutputLFLineBreak;
+    if (UseFormatFlowed(charset))
+        flags |= nsIDocumentEncoder::OutputFormatFlowed;
+    rv = m_editor->OutputToString(contentType, flags, msgBody);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+  else
+  {
+    m_compFields->GetBody(msgBody);
+  }
+  if (!msgBody.IsEmpty())
+  {
+    // Convert body to mail charset
+    nsCString outCString;
+    nsCString fallbackCharset;
+    bool isAsciiOnly;
+    // Check if the body text is covered by the current charset.
+    rv = nsMsgI18NSaveAsCharset(NS_ConvertUTF16toUTF8(contentType).get(),
+                                m_compFields->GetCharacterSet(),
+                                msgBody.get(), getter_Copies(outCString),
+                                getter_Copies(fallbackCharset), &isAsciiOnly);
+    if (m_compFields->GetForceMsgEncoding())
+      isAsciiOnly = false;
+    if (NS_SUCCEEDED(rv) && !outCString.IsEmpty())
     {
-      // Reset message body previously stored in the compose fields
-      // There is 2 nsIMsgCompFields::SetBody() functions using a pointer as argument,
-      // therefore a casting is required.
-      m_compFields->SetBody((const char *)nullptr);
-
-      const char *charset = m_compFields->GetCharacterSet();
-      if(UseFormatFlowed(charset))
-          flags |= nsIDocumentEncoder::OutputFormatFlowed;
-
-      rv = m_editor->OutputToString(NS_LITERAL_STRING("text/plain"), flags, msgBody);
+      // If the body contains characters outside the repertoire of the current
+      // charset, just convert to UTF-8 and be done with it
+      // unless disable_fallback_to_utf8 is set for this charset.
+      if (NS_ERROR_UENC_NOMAPPING == rv && m_editor)
+      {
+        bool needToCheckCharset;
+        m_compFields->GetNeedToCheckCharset(&needToCheckCharset);
+        if (needToCheckCharset)
+        {
+          bool disableFallback = false;
+          nsCOMPtr<nsIPrefBranch> prefBranch (do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
+          if (prefBranch)
+          {
+            nsCString prefName("mailnews.disable_fallback_to_utf8.");
+            prefName.Append(m_compFields->GetCharacterSet());
+            prefBranch->GetBoolPref(prefName.get(), &disableFallback);
+          }
+          if (!disableFallback)
+          {
+            CopyUTF16toUTF8(msgBody, outCString);
+            m_compFields->SetCharacterSet("UTF-8");
+            SetDocumentCharset("UTF-8");
+          }
+        }
+      }
+      else if (!fallbackCharset.IsEmpty())
+      {
+        // re-label to the fallback charset
+        m_compFields->SetCharacterSet(fallbackCharset.get());
+        SetDocumentCharset(fallbackCharset.get());
+      }
+      m_compFields->SetBodyIsAsciiOnly(isAsciiOnly);
+      m_compFields->SetBody(outCString.get());
     }
     else
     {
-      m_compFields->GetBody(msgBody);
-    }
-    if (NS_SUCCEEDED(rv) && !msgBody.IsEmpty())
-    {
-      // Convert body to mail charset
-      nsCString outCString;
-      nsCString fallbackCharset;
-      bool isAsciiOnly;
-      // check if the body text is covered by the current charset.
-      rv = nsMsgI18NSaveAsCharset(contentType, m_compFields->GetCharacterSet(),
-                                  msgBody.get(), getter_Copies(outCString),
-                                  getter_Copies(fallbackCharset), &isAsciiOnly);
-      if (m_compFields->GetForceMsgEncoding())
-        isAsciiOnly = false;
-      if (NS_SUCCEEDED(rv) && !outCString.IsEmpty())
-      {
-        // If the body contains characters outside the repertoire of the current
-        // charset, just convert to UTF-8 and be done with it
-        // unless disable_fallback_to_utf8 is set for this charset.
-        if (NS_ERROR_UENC_NOMAPPING == rv && m_editor)
-        {
-          bool needToCheckCharset;
-          m_compFields->GetNeedToCheckCharset(&needToCheckCharset);
-          if (needToCheckCharset)
-          {
-            bool disableFallback = false;
-            nsCOMPtr<nsIPrefBranch> prefBranch (do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
-            if (prefBranch)
-            {
-              nsCString prefName("mailnews.disable_fallback_to_utf8.");
-              prefName.Append(m_compFields->GetCharacterSet());
-              prefBranch->GetBoolPref(prefName.get(), &disableFallback);
-            }
-            if (!disableFallback)
-            {
-              CopyUTF16toUTF8(msgBody, outCString);
-              m_compFields->SetCharacterSet("UTF-8");
-            }
-          }
-        }
-        else if (!fallbackCharset.IsEmpty())
-        {
-          // re-label to the fallback charset
-          m_compFields->SetCharacterSet(fallbackCharset.get());
-        }
-        m_compFields->SetBodyIsAsciiOnly(isAsciiOnly);
-        m_compFields->SetBody(outCString.get());
-        entityConversionDone = true;
-      }
-      else
-        m_compFields->SetBody(NS_LossyConvertUTF16toASCII(msgBody).get());
+      m_compFields->SetBody(NS_LossyConvertUTF16toASCII(msgBody).get());
+      m_compFields->SetCharacterSet("US-ASCII");
+      SetDocumentCharset("US-ASCII");
     }
   }
 
   // Let's open the progress dialog
   if (progress)
   {
     mProgress = progress;
 
@@ -1262,18 +1243,17 @@ NS_IMETHODIMP nsMsgCompose::SendMsg(MSG_
         }
       }
     }
 
     mProgress->OnStateChange(nullptr, nullptr, nsIWebProgressListener::STATE_START, NS_OK);
   }
 
   bool attachVCard = false;
-  if (m_compFields)
-      m_compFields->GetAttachVCard(&attachVCard);
+  m_compFields->GetAttachVCard(&attachVCard);
 
   if (attachVCard && identity &&
       (deliverMode == nsIMsgCompDeliverMode::Now ||
        deliverMode == nsIMsgCompDeliverMode::Later ||
        deliverMode == nsIMsgCompDeliverMode::Background))
   {
       nsCString escapedVCard;
       // make sure, if there is no card, this returns an empty string, or NS_ERROR_FAILURE
@@ -1317,17 +1297,17 @@ NS_IMETHODIMP nsMsgCompose::SendMsg(MSG_
               m_compFields->AddAttachment(attachment);
           }
       }
   }
 
   // Save the identity being sent for later use.
   m_identity = identity;
 
-  rv = _SendMsg(deliverMode, identity, accountKey, entityConversionDone);
+  rv = _SendMsg(deliverMode, identity, accountKey);
   if (NS_FAILED(rv))
   {
     nsCOMPtr<nsIMsgSendReport> sendReport;
     if (mMsgSend)
       mMsgSend->GetSendReport(getter_AddRefs(sendReport));
     if (sendReport)
     {
       nsresult theError;
--- a/mailnews/compose/src/nsMsgCompose.h
+++ b/mailnews/compose/src/nsMsgCompose.h
@@ -72,17 +72,17 @@ private:
 
   bool                          CheckIncludeSignaturePrefs(nsIMsgIdentity *identity);
   //m_folderName to store the value of the saved drafts folder.
   nsCString                     m_folderName;
   void InsertDivWrappedTextAtSelection(const nsAString &aText,
                                        const nsAString &classStr);
 
  private:
-  nsresult _SendMsg(MSG_DeliverMode deliverMode, nsIMsgIdentity *identity, const char *accountKey, bool entityConversionDone);
+  nsresult _SendMsg(MSG_DeliverMode deliverMode, nsIMsgIdentity *identity, const char *accountKey);
   nsresult CreateMessage(const char * originalMsgURI, MSG_ComposeType type, nsIMsgCompFields* compFields);
   void CleanUpRecipients(nsString& recipients);
   nsresult GetABDirectories(const nsACString& aDirUri,
                             nsCOMArray<nsIAbDirectory> &aDirArray);
   nsresult BuildMailListArray(nsIAbDirectory* parentDir,
                               nsTArray<nsMsgMailList>& array);
   nsresult GetMailListAddresses(nsString& name,
                                 nsTArray<nsMsgMailList>& mailListArray,
--- a/mailnews/compose/src/nsMsgSend.cpp
+++ b/mailnews/compose/src/nsMsgSend.cpp
@@ -1518,34 +1518,30 @@ nsMsgComposeAndSend::GetBodyFromEditor()
   // Now we have to fix up and get the HTML from the editor. After we
   // get the HTML data, we need to store it in the m_attachment_1_body
   // member variable after doing the necessary charset conversion.
   //
 
   //
   // Query the editor, get the body of HTML!
   //
-  nsString  format;
-  format.AssignLiteral(TEXT_HTML);
   uint32_t  flags = nsIDocumentEncoder::OutputFormatted  | nsIDocumentEncoder::OutputNoFormattingInPre;
   nsAutoString bodyStr;
   PRUnichar* bodyText = nullptr;
   nsresult rv;
   PRUnichar *origHTMLBody = nullptr;
 
   // Ok, get the body...the DOM should have been whacked with
   // Content ID's already
   if (mEditor)
-    mEditor->OutputToString(format, flags, bodyStr);
+    mEditor->OutputToString(NS_LITERAL_STRING(TEXT_HTML), flags, bodyStr);
   else
     bodyStr = NS_ConvertASCIItoUTF16(m_attachment1_body);
 
- //
   // If we really didn't get a body, just return NS_OK
-  //
   if (bodyStr.IsEmpty())
     return NS_OK;
   bodyText = ToNewUnicode(bodyStr);
   if (!bodyText)
     return NS_ERROR_OUT_OF_MEMORY;
 
   // If we are forcing this to be plain text, we should not be
   // doing this conversion.
@@ -1578,51 +1574,47 @@ nsMsgComposeAndSend::GetBodyFromEditor()
         // We should have what the user typed in stored in mOriginalHTMLBody
         origHTMLBody = bodyText;
         bodyText = wresult;
       }
     }
   }
 
   nsCString attachment1_body;
-  // we'd better be "text/html" at this point
-  const char  *attachment1_type = TEXT_HTML;
 
   // Convert body to mail charset
   nsCString    outCString;
   const char  *aCharset = mCompFields->GetCharacterSet();
 
   if (aCharset && *aCharset)
   {
-    // Convert to entities.
-    // If later Editor generates entities then we can remove this.
     bool isAsciiOnly;
-    rv = nsMsgI18NSaveAsCharset(mCompFields->GetForcePlainText() ? TEXT_PLAIN : attachment1_type,
+    rv = nsMsgI18NSaveAsCharset(mCompFields->GetForcePlainText() ? TEXT_PLAIN : TEXT_HTML,
                                 aCharset, bodyText, getter_Copies(outCString), nullptr, &isAsciiOnly);
 
     if (mCompFields->GetForceMsgEncoding())
       isAsciiOnly = false;
 
     mCompFields->SetBodyIsAsciiOnly(isAsciiOnly);
 
-    // body contains characters outside the current mail charset,
-    // ask whether to convert to UTF-8 (bug 233361). do this only for text/plain
-    if ((NS_ERROR_UENC_NOMAPPING == rv) && mCompFields->GetForcePlainText()) {
+    // If the body contains characters outside the current mail charset,
+    // convert to UTF-8.
+    if (NS_ERROR_UENC_NOMAPPING == rv) {
       // if nbsp then replace it by sp and try again
       PRUnichar *bodyTextPtr = bodyText;
       while (*bodyTextPtr) {
         if (0x00A0 == *bodyTextPtr)
           *bodyTextPtr = 0x0020;
         bodyTextPtr++;
       }
 
       nsCString fallbackCharset;
-      rv = nsMsgI18NSaveAsCharset(TEXT_PLAIN, aCharset, bodyText,
-           getter_Copies(outCString), getter_Copies(fallbackCharset));
-
+      rv = nsMsgI18NSaveAsCharset(mCompFields->GetForcePlainText() ? TEXT_PLAIN : TEXT_HTML,
+                                 aCharset, bodyText, getter_Copies(outCString),
+                                 getter_Copies(fallbackCharset));
       if (NS_ERROR_UENC_NOMAPPING == rv)
       {
         bool needToCheckCharset;
         mCompFields->GetNeedToCheckCharset(&needToCheckCharset);
         if (needToCheckCharset)
         {
           // Just use UTF-8 and be done with it
           // unless disable_fallback_to_utf8 is set for this charset.
@@ -1652,39 +1644,39 @@ nsMsgComposeAndSend::GetBodyFromEditor()
       attachment1_body = outCString;
 
     // If we have an origHTMLBody that is not null, this means that it is
     // different than the bodyText because of formatting conversions. Because of
     // this we need to do the charset conversion on this part separately
     if (origHTMLBody)
     {
       char      *newBody = nullptr;
-      rv = nsMsgI18NSaveAsCharset(mCompFields->GetUseMultipartAlternative() ? TEXT_PLAIN : attachment1_type,
+      rv = nsMsgI18NSaveAsCharset(mCompFields->GetUseMultipartAlternative() ? TEXT_PLAIN : TEXT_HTML,
                                   aCharset, origHTMLBody, &newBody);
       if (NS_SUCCEEDED(rv))
       {
         PR_FREEIF(origHTMLBody);
         origHTMLBody = (PRUnichar *)newBody;
       }
     }
 
     NS_Free(bodyText);    //Don't need it anymore
   }
   else
     return NS_ERROR_FAILURE;
 
   // If our holder for the original body text is STILL null, then just
   // just copy what we have as the original body text.
-  //
+
   if (!origHTMLBody)
     mOriginalHTMLBody = ToNewCString(attachment1_body);
   else
     mOriginalHTMLBody = (char *)origHTMLBody; // Whoa, origHTMLBody is declared as a PRUnichar *, what's going on here?
 
-  rv = SnarfAndCopyBody(attachment1_body, attachment1_type);
+  rv = SnarfAndCopyBody(attachment1_body, TEXT_HTML);
 
   return rv;
 }
 
 // for SMTP, 16k
 // for our internal protocol buffers, 4k
 // for news < 1000
 // so we choose the minimum, because we could be sending and posting this message.