Bug 735380 - Fix signature switching in plaintext compose windows. r=bienvenu. AURORA_BASE_20120313
authorMike Conley <mconley@mozilla.com>
Tue, 13 Mar 2012 16:57:54 -0400
changeset 11100 3eea95c4774822f87037945ef591ce68a229667e
parent 11099 a4020ebe7917e8685777ee2a1712aef1ac6789c3
child 11101 aff723a9987d5bdff8ddd13a6f6bb5622f11ede2
push id463
push userbugzilla@standard8.plus.com
push dateTue, 24 Apr 2012 17:34:51 +0000
treeherdercomm-beta@e53588e8f7b0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbienvenu
bugs735380
Bug 735380 - Fix signature switching in plaintext compose windows. r=bienvenu.
mail/test/mozmill/composition/test-signature-updating.js
mailnews/compose/src/nsMsgCompose.cpp
--- a/mail/test/mozmill/composition/test-signature-updating.js
+++ b/mail/test/mozmill/composition/test-signature-updating.js
@@ -100,51 +100,65 @@ function plaintextComposeWindowSwitchSig
   let contentFrame = cwc.e("content-frame");
   let node = contentFrame.contentDocument.body.lastChild;
 
   // In plaintext compose, the signature is followed by two <br> elements.
   assert_equals(node.localName, "br");
   node = node.previousSibling;
   assert_equals(node.localName, "br");
   node = node.previousSibling;
-  assert_equals(node.nodeValue, "Tinderbox is soo 90ies");
-  if (!suppressSigSep) {
-    // a <br> element, then the next text node
-    node = node.previousSibling.previousSibling;
-    assert_equals(node.nodeValue, "-- ");
-  }
+
+  // Now we should have the DIV node that contains the signature, with
+  // the class moz-signature.
+  assert_equals(node.localName, "div");
+
+  const sigClass = "moz-signature";
+  assert_equals(node.className, sigClass);
+
+  let expectedText = "Tinderbox is soo 90ies\n";
+
+  if (!suppressSigSep)
+    expectedText = "-- \n" + expectedText;
+
+  assert_equals(node.textContent, expectedText);
 
   // Now switch identities!
   let menuID = cwc.e("msgIdentity");
   menuID.value = "id2";
   menuID.click();
 
   node = contentFrame.contentDocument.body.lastChild;
 
   // In plaintext compose, the signature is followed by two <br> elements.
   assert_equals(node.localName, "br");
   node = node.previousSibling;
   assert_equals(node.localName, "br");
   node = node.previousSibling;
-  assert_equals(node.nodeValue, "Tinderboxpushlog is the new *hotness!*");
-  if (!suppressSigSep) {
-    // a <br> element, then the next text node
-    node = node.previousSibling.previousSibling;
-    assert_equals(node.nodeValue, "-- ");
-  }
+
+  assert_equals(node.localName, "div");
+  assert_equals(node.className, sigClass);
+
+  expectedText = "Tinderboxpushlog is the new *hotness!*\n";
+
+  if (!suppressSigSep)
+    expectedText = "-- \n" + expectedText;
+
+  assert_equals(node.textContent, expectedText);
 
-  // Now check that the original signature has been removed!
+  // Now check that the original signature has been removed by ensuring
+  // that there's only one node with class moz-signature.
+  let sigs = contentFrame.contentDocument.querySelectorAll("." + sigClass);
+  assert_equals(sigs.length, 1);
+
+  // And ensure that the text we wrote wasn't altered
   let bodyFirstChild =  contentFrame.contentDocument.body.firstChild;
-  while (node != bodyFirstChild) {
+
+  while (node != bodyFirstChild)
     node = node.previousSibling;
-    if (node) {
-      assert_not_equals(node.nodeValue, "Tinderbox is soo 90ies");
-      assert_not_equals(node.nodeValue, "-- ");
-    }
-  }
+
   assert_equals(node.nodeValue, "Body, first line.");
 
   composeHelper.close_compose_window(cwc);
 }
 
 function testPlaintextComposeWindowSwitchSignatures() {
   plaintextComposeWindowSwitchSignatures(false);
 }
--- a/mailnews/compose/src/nsMsgCompose.cpp
+++ b/mailnews/compose/src/nsMsgCompose.cpp
@@ -563,17 +563,16 @@ nsMsgCompose::InsertDivWrappedTextAtSele
   nsresult rv = htmlEditor->CreateElementWithDefaults(NS_LITERAL_STRING("div"),
                                                       getter_AddRefs(divElem));
 
   NS_ENSURE_SUCCESS(rv,);
 
   nsCOMPtr<nsIDOMNode> divNode (do_QueryInterface(divElem));
   divNode->SetTextContent(aText);
 
-  textEditor->InsertLineBreak();
   htmlEditor->InsertElementAtSelection(divElem, true);
   nsCOMPtr<nsIDOMNode> parent;
   PRInt32 offset;
 
   rv = GetNodeLocation(divNode, address_of(parent), &offset);
   if (NS_SUCCEEDED(rv))
   {
     nsCOMPtr<nsISelection> selection;
@@ -692,18 +691,21 @@ nsMsgCompose::ConvertAndLoadComposeWindo
     {
       //we cannot add it on top earlier, because TagEmbeddedObjects will mark all images in the signature as "moz-do-not-send"
       if( sigOnTop )
         m_editor->BeginningOfDocument();
 
       if (aHTMLEditor && htmlEditor)
         htmlEditor->InsertHTML(aSignature);
       else if (htmlEditor)
+      {
+        textEditor->InsertLineBreak();
         InsertDivWrappedTextAtSelection(aSignature,
                                         NS_LITERAL_STRING("moz-signature"));
+      }
 
       if( sigOnTop )
         m_editor->EndOfDocument();
     }
   }
   else
   {
     if (aHTMLEditor && htmlEditor)
@@ -745,16 +747,17 @@ nsMsgCompose::ConvertAndLoadComposeWindo
         if (mailEditor)
           mailEditor->InsertTextWithQuotations(aBuf);
         else
           textEditor->InsertText(aBuf);
         m_editor->EndOfDocument();
       }
 
       if (!sigOnTop && !aSignature.IsEmpty())
+        textEditor->InsertLineBreak();
         InsertDivWrappedTextAtSelection(aSignature,
                                         NS_LITERAL_STRING("moz-signature"));
     }
   }
 
   if (aBuf.IsEmpty())
     m_editor->BeginningOfDocument();
   else
@@ -4329,18 +4332,16 @@ nsMsgCompose::ProcessSignature(nsIMsgIde
     if (m_composeHTML)
     {
       sigOutput.AppendLiteral(htmlBreak);
       if (htmlSig)
         sigOutput.AppendLiteral(htmlsigopen);
       else
         sigOutput.Append(NS_ConvertASCIItoUTF16(preopen));
     }
-    else
-      sigOutput.AppendLiteral(CRLF);
 
     if ((reply_on_top != 1 || sig_bottom || !aQuoted) &&
         sigData.Find("\r-- \r", true) < 0 &&
         sigData.Find("\n-- \n", true) < 0 &&
         sigData.Find("\n-- \r", true) < 0)
     {
       nsDependentSubstring firstFourChars(sigData, 0, 4);
 
@@ -5389,127 +5390,66 @@ nsMsgCompose::SetIdentity(nsIMsgIdentity
   nsCOMPtr<nsIDOMNode> node;
   nsCOMPtr<nsIDOMNode> tempNode;
   nsAutoString tagLocalName;
 
   rv = rootElement->GetLastChild(getter_AddRefs(lastNode));
   if (NS_SUCCEEDED(rv) && nsnull != lastNode)
   {
     node = lastNode;
-    if (m_composeHTML)
+    // In html, the signature is inside an element with
+    // class="moz-signature"
+    bool signatureFound = false;
+    nsAutoString attributeName;
+    attributeName.AssignLiteral("class");
+
+    do
     {
-      /* In html, the signature is inside an element with
-         class="moz-signature", it's must be the last node */
       nsCOMPtr<nsIDOMElement> element = do_QueryInterface(node);
       if (element)
       {
-        nsAutoString attributeName;
         nsAutoString attributeValue;
-        attributeName.AssignLiteral("class");
 
         rv = element->GetAttribute(attributeName, attributeValue);
-        if (NS_SUCCEEDED(rv))
-        {
-          if (attributeValue.Find("moz-signature", true) != kNotFound)
-          {
-            //Now, I am sure I get the right node!
-            m_editor->BeginTransaction();
-            node->GetPreviousSibling(getter_AddRefs(tempNode));
-            rv = m_editor->DeleteNode(node);
-            if (NS_FAILED(rv))
-            {
-              m_editor->EndTransaction();
-              return rv;
-            }
-
-            //Also, remove the <br> right before the signature.
-            if (tempNode)
-            {
-              tempNode->GetLocalName(tagLocalName);
-              if (tagLocalName.EqualsLiteral("br"))
-                m_editor->DeleteNode(tempNode);
-            }
-            m_editor->EndTransaction();
-          }
+
+        if (attributeValue.Find("moz-signature", true) != kNotFound) {
+          signatureFound = true;
+          break;
         }
       }
-    }
-    else
+    } while (!signatureFound &&
+             node &&
+             NS_SUCCEEDED(node->GetPreviousSibling(getter_AddRefs(node))));
+
+    if (signatureFound)
     {
-      //In plain text, we have to walk back the dom look for the pattern <br>-- <br>
-      PRUint16 nodeType;
-      PRInt32 searchState = 0; //0=nothing, 1=br 2='-- '+br, 3=br+'-- '+br
-
-      do
+      m_editor->BeginTransaction();
+      node->GetPreviousSibling(getter_AddRefs(tempNode));
+      rv = m_editor->DeleteNode(node);
+      if (NS_FAILED(rv))
       {
-        node->GetNodeType(&nodeType);
-        node->GetLocalName(tagLocalName);
-        switch (searchState)
-        {
-          case 0:
-            if (nodeType == nsIDOMNode::ELEMENT_NODE && tagLocalName.EqualsLiteral("br"))
-              searchState = 1;
-            break;
-
-          case 1:
-            searchState = 0;
-            if (nodeType == nsIDOMNode::TEXT_NODE)
-            {
-              nsString nodeValue;
-              node->GetNodeValue(nodeValue);
-              if (nodeValue.EqualsLiteral("-- "))
-                searchState = 2;
-            }
-            else
-              if (nodeType == nsIDOMNode::ELEMENT_NODE && tagLocalName.EqualsLiteral("br"))
-              {
-                searchState = 1;
-                break;
-              }
-            break;
-
-          case 2:
-            if (nodeType == nsIDOMNode::ELEMENT_NODE && tagLocalName.EqualsLiteral("br"))
-              searchState = 3;
-            else
-              searchState = 0;
-            break;
-        }
-
-        tempNode = node;
-      } while (searchState != 3 && NS_SUCCEEDED(tempNode->GetPreviousSibling(getter_AddRefs(node))) && node);
-
-      if (searchState == 3)
+        m_editor->EndTransaction();
+        return rv;
+      }
+
+      //Also, remove the <br> right before the signature.
+      if (tempNode)
       {
-        //Now, I am sure I get the right node!
-        m_editor->BeginTransaction();
-
-        tempNode = lastNode;
-        lastNode = node;
-        do
-        {
-          node = tempNode;
-          node->GetPreviousSibling(getter_AddRefs(tempNode));
-          rv = m_editor->DeleteNode(node);
-          if (NS_FAILED(rv))
-          {
-            m_editor->EndTransaction();
-            return rv;
-          }
-
-        } while (node != lastNode && tempNode);
-        m_editor->EndTransaction();
+        tempNode->GetLocalName(tagLocalName);
+        if (tagLocalName.EqualsLiteral("br"))
+          m_editor->DeleteNode(tempNode);
       }
+      m_editor->EndTransaction();
     }
   }
 
   if (!CheckIncludeSignaturePrefs(aIdentity))
     return NS_OK;
 
-  //Then add the new one if needed
+  // Then add the new one if needed
   nsAutoString aSignature;
 
   // No delimiter needed if not a compose window
   bool noDelimiter;
   switch (mType)
   {
     case nsIMsgCompType::New :
     case nsIMsgCompType::NewsPost :
@@ -5533,26 +5473,26 @@ nsMsgCompose::SetIdentity(nsIMsgIdentity
     bool sig_bottom = true;
     aIdentity->GetReplyOnTop(&reply_on_top);
     aIdentity->GetSigBottom(&sig_bottom);
     bool sigOnTop = (reply_on_top == 1 && !sig_bottom);
     if (sigOnTop && noDelimiter)
       m_editor->BeginningOfDocument();
     else
       m_editor->EndOfDocument();
+
+    nsCOMPtr<nsIHTMLEditor> htmlEditor (do_QueryInterface(m_editor));
+
     if (m_composeHTML)
-    {
-      nsCOMPtr<nsIHTMLEditor> htmlEditor (do_QueryInterface(m_editor));
       rv = htmlEditor->InsertHTML(aSignature);
+    else {
+      rv = NS_OK;
+      InsertDivWrappedTextAtSelection(aSignature, NS_LITERAL_STRING("moz-signature"));
     }
-    else
-    {
-      nsCOMPtr<nsIPlaintextEditor> textEditor (do_QueryInterface(m_editor));
-      rv = textEditor->InsertText(aSignature);
-    }
+
     if (sigOnTop && noDelimiter)
       m_editor->EndOfDocument();
     m_editor->EndTransaction();
   }
 
   return rv;
 }