Bug 1336531 - Don't put <br> before signatures in paragraph mode. r=aceman
authorJorg K <jorgk@jorgk.com>
Mon, 13 Feb 2017 08:49:05 +0100
changeset 21163 a25600ed9d2e8630919133744622e33956e2e3ad
parent 21162 4095f1ae7fdfeb5ba724b925d1860a022da1c5ed
child 21164 da2e8137b3dfe320d00c5b79f5f3c07e82225efe
push id12852
push usermozilla@jorgk.com
push dateMon, 13 Feb 2017 07:53:09 +0000
treeherdercomm-central@da2e8137b3df [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaceman
bugs1336531
Bug 1336531 - Don't put <br> before signatures in paragraph mode. r=aceman
mail/components/compose/content/MsgComposeCommands.js
mail/test/mozmill/composition/test-signature-updating.js
mailnews/compose/src/nsMsgCompose.cpp
--- a/mail/components/compose/content/MsgComposeCommands.js
+++ b/mail/components/compose/content/MsgComposeCommands.js
@@ -273,16 +273,25 @@ function toggleAffectedChrome(aHide)
     // restore the Status Bar
     statusbar.hidden = gChromeState.statusbarWasHidden;
   }
 
   document.getElementById("compose-toolbox").hidden = aHide;
   document.getElementById("appcontent").collapsed = aHide;
 }
 
+/**
+ * Small helper function to check whether the node passed in is a signature.
+ * Note that a text node is not a DOM element, hence .localName can't be used.
+ */
+function isSignature(aNode) {
+  return ["DIV","PRE"].includes(aNode.nodeName) &&
+         aNode.classList.contains("moz-signature");
+}
+
 var stateListener = {
   NotifyComposeFieldsReady: function() {
     ComposeFieldsReady();
     updateSendCommands(true);
   },
 
   NotifyComposeBodyReady: function() {
     // Look all the possible compose types (nsIMsgComposeParams.idl):
@@ -346,45 +355,38 @@ var stateListener = {
       loadHTMLMsgPrefs();
     AdjustFocus();
   },
 
   NotifyComposeBodyReadyNew: function() {
     let useParagraph = Services.prefs.getBoolPref("mail.compose.default_to_paragraph");
     let insertParagraph = gMsgCompose.composeHTML && useParagraph;
 
+    let mailBody = getBrowser().contentDocument.querySelector("body");
     if (insertParagraph && gBodyFromArgs) {
-      // Check for empty body before allowing paragraph to be inserted.
-      // An "empty" body will have a <br> potentially followed by a
-      // <div class="moz-signature"> or <pre class="moz-signature">.
-      let mailBody = getBrowser().contentDocument.querySelector("body");
+      // Check for "empty" body before allowing paragraph to be inserted.
+      // Non-empty bodies in a new message can occur when clicking on a
+      // mailto link or when using the command line option -compose.
+      // An "empty" body can be one of these two cases:
+      // 1) <br> and nothing follows (no next sibling)
+      // 2) <div/pre class="moz-signature">
+      // Note that <br><div/pre class="moz-signature"> doesn't happen in
+      // paragraph mode.
       let firstChild = mailBody.firstChild;
-      let nextSibling = firstChild.nextSibling;
-      do {
-        if (firstChild.localName != 'br') {
-          insertParagraph = false;
-          break;
-        }
-
-        if (!nextSibling)
-          break;
-
-        if ((nextSibling.localName == "div" || nextSibling.localName == "pre") &&
-            nextSibling.getAttribute("class") == "moz-signature")
-          break;
-
+      if ((firstChild.nodeName != "BR" || firstChild.nextSibling) &&
+          !isSignature(firstChild))
         insertParagraph = false;
-      } while (false);
     }
 
     // Control insertion of line breaks.
     if (insertParagraph) {
       let editor = GetCurrentEditor();
       editor.enableUndo(false);
 
+      editor.selection.collapse(mailBody, 0);
       let pElement = editor.createElementWithDefaults("p");
       pElement.appendChild(editor.createElementWithDefaults("br"));
       editor.insertElementAtSelection(pElement, false);
 
       document.getElementById("cmd_paragraphState").setAttribute("state", "p");
 
       editor.beginningOfDocument();
       editor.enableUndo(true);
@@ -396,16 +398,21 @@ var stateListener = {
 
   NotifyComposeBodyReadyReply: function() {
     // Control insertion of line breaks.
     let useParagraph = Services.prefs.getBoolPref("mail.compose.default_to_paragraph");
     if (gMsgCompose.composeHTML && useParagraph) {
       let mailBody = getBrowser().contentDocument.querySelector("body");
       let editor = GetCurrentEditor();
       let selection = editor.selection;
+
+      // Make sure the selection isn't inside the signature.
+      if (isSignature(mailBody.firstChild))
+        selection.collapse(mailBody, 0);
+
       let range = selection.getRangeAt(0);
       let start = range.startOffset;
 
       if (start != range.endOffset) {
         // The selection is not collapsed, most likely due to the
         // "select the quote" option. In this case we do nothing.
         return;
       }
--- a/mail/test/mozmill/composition/test-signature-updating.js
+++ b/mail/test/mozmill/composition/test-signature-updating.js
@@ -196,18 +196,20 @@ function HTMLComposeWindowSwitchSignatur
   node = contentFrame.contentDocument.body.firstChild;
   let textNode;
   if (paragraphFormat) {
     textNode = node.firstChild;
   } else {
     textNode = node;
   }
   assert_equals(textNode.nodeValue, "Body, first line.");
-  node = node.nextSibling;
-  assert_equals(node.localName, "br");
+  if (!paragraphFormat) {
+    node = node.nextSibling;
+    assert_equals(node.localName, "br");
+  }
   node = node.nextSibling;
   // check that the signature is immediately after the message text.
   assert_equals(node.className, "moz-signature");
   // check that that the signature is the last node.
   assert_equals(node, contentFrame.contentDocument.body.lastChild);
 
   close_compose_window(cwc);
 }
--- a/mailnews/compose/src/nsMsgCompose.cpp
+++ b/mailnews/compose/src/nsMsgCompose.cpp
@@ -4517,23 +4517,31 @@ nsMsgCompose::ProcessSignature(nsIMsgIde
   static const char      preclose[] = "</pre>";
 
   int32_t wrapLength = 72; // setup default value in case GetWrapLength failed
   GetWrapLength(&wrapLength);
   preopen = PR_smprintf(_preopen, wrapLength);
   if (!preopen)
     return NS_ERROR_OUT_OF_MEMORY;
 
+#ifdef MOZ_THUNDERBIRD
+  bool paragraphMode =
+    mozilla::Preferences::GetBool("mail.compose.default_to_paragraph", false);
+#endif
+
   if (imageSig)
   {
     // We have an image signature. If we're using the in HTML composer, we
     // should put in the appropriate HTML for inclusion, otherwise, do nothing.
     if (m_composeHTML)
     {
-      sigOutput.AppendLiteral(htmlBreak);
+#ifdef MOZ_THUNDERBIRD
+      if (!paragraphMode)
+#endif
+        sigOutput.AppendLiteral(htmlBreak);
       sigOutput.AppendLiteral(htmlsigopen);
       if ((mType == nsIMsgCompType::NewsPost || !suppressSigSep) &&
           (reply_on_top != 1 || sig_bottom || !aQuoted)) {
         sigOutput.AppendLiteral(dashes);
       }
 
       sigOutput.AppendLiteral(htmlBreak);
       sigOutput.AppendLiteral("<img src='");
@@ -4613,17 +4621,21 @@ nsMsgCompose::ProcessSignature(nsIMsgIde
   }
 
   // Now that sigData holds data...if any, append it to the body in a nice
   // looking manner
   if (!sigData.IsEmpty())
   {
     if (m_composeHTML)
     {
-      sigOutput.AppendLiteral(htmlBreak);
+#ifdef MOZ_THUNDERBIRD
+      if (!paragraphMode)
+#endif
+        sigOutput.AppendLiteral(htmlBreak);
+
       if (htmlSig)
         sigOutput.AppendLiteral(htmlsigopen);
       else
         sigOutput.Append(NS_ConvertASCIItoUTF16(preopen));
     }
 
     if ((reply_on_top != 1 || sig_bottom || !aQuoted) &&
         sigData.Find("\r-- \r", true) < 0 &&