Bug 1501259 - HTMLEditor::InsertAsPlaintextQuotation() shouldn't refer |newNode| after calling its |forget()| r=m_kato

Regression of bug 1476897, HTMLEditor::InsertAsPlaintextQuotation() refers
|newNode| which is new created <span> element by the method at collapsing
Selection.  However, it's already been moved to its out param.  Therefore,
after the fix of bug 1476897, it won't collapse Selection.  This patch
moves the code moving |newNode| to the out param after collapsing Selection.

Additionally, this makes HTMLEditor::InsertAsCitedQuotationInternal() use
same style code around collapsing Selection since it's also tested by the
new test of nsIEditorMailSupport.insertAsCitedQuotation().

Differential Revision: https://phabricator.services.mozilla.com/D10002

--HG--
extra : moz-landing-system : lando

editor/libeditor/HTMLEditorDataTransfer.cpp

@@ -2007,33 +2007,40 @@ HTMLEditor::InsertAsPlaintextQuotation(const nsAString& aQuotedText,
 
   if (aAddCites) {
     rv = InsertWithQuotationsAsSubAction(aQuotedText);
-    NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
-      "Failed to insert the text with quotations");
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   } else {
     rv = InsertTextAsSubAction(aQuotedText);
-    NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
-      "Failed to insert the quoted text as plain text");
-  }
-  // Note that if !aAddCites, aNodeInserted isn't set.
-  // That's okay because the routines that use aAddCites
-  // don't need to know the inserted node.
-
-  if (aNodeInserted && NS_SUCCEEDED(rv)) {
-    newNode.forget(aNodeInserted);
-  }
-
-  // Set the selection to just after the inserted node:
-  if (NS_SUCCEEDED(rv) && newNode) {
-    EditorRawDOMPoint afterNewNode(newNode);
-    if (afterNewNode.AdvanceOffset()) {
-      DebugOnly<nsresult> rv = selection->Collapse(afterNewNode);
-      NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
-        "Failed to collapse after the new node");
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
     }
   }
 
+  if (!newNode) {
+    return NS_OK;
+  }
+
+  // Set the selection to just after the inserted node:
+  EditorRawDOMPoint afterNewNode(newNode);
+  bool advanced = afterNewNode.AdvanceOffset();
+  NS_WARNING_ASSERTION(advanced,
+    "Failed to advance offset to after the new <span> element");
+  if (advanced) {
+    DebugOnly<nsresult> rvIgnored = selection->Collapse(afterNewNode);
+    NS_WARNING_ASSERTION(NS_SUCCEEDED(rvIgnored),
+      "Failed to collapse after the new node");
+  }
+
+  // Note that if !aAddCites, aNodeInserted isn't set.
+  // That's okay because the routines that use aAddCites
+  // don't need to know the inserted node.
+  if (aNodeInserted) {
+    newNode.forget(aNodeInserted);
+  }
+
   // XXX Why don't we call HTMLEditRules::DidDoAction() here?
-  return rv;
+  return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -2159,24 +2166,36 @@ HTMLEditor::InsertAsCitedQuotationInternal(const nsAString& aQuotedText,
 
   if (aInsertHTML) {
     rv = LoadHTML(aQuotedText);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   } else {
     rv = InsertTextAsSubAction(aQuotedText);  // XXX ignore charset
-    NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to insert the quoted text");
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   }
 
-  if (aNodeInserted && NS_SUCCEEDED(rv)) {
-    *aNodeInserted = newNode;
-    NS_IF_ADDREF(*aNodeInserted);
+  if (!newNode) {
+    return NS_OK;
   }
 
   // Set the selection to just after the inserted node:
-  if (NS_SUCCEEDED(rv) && newNode) {
-    EditorRawDOMPoint afterNewNode(newNode);
-    if (afterNewNode.AdvanceOffset()) {
-      selection->Collapse(afterNewNode);
-    }
+  EditorRawDOMPoint afterNewNode(newNode);
+  bool advanced = afterNewNode.AdvanceOffset();
+  NS_WARNING_ASSERTION(advanced,
+    "Failed advance offset to after the new <blockquote> element");
+  if (advanced) {
+    DebugOnly<nsresult> rvIgnored = selection->Collapse(afterNewNode);
+    NS_WARNING_ASSERTION(NS_SUCCEEDED(rvIgnored),
+      "Failed to collapse after the new node");
   }
-  return rv;
+
+  if (aNodeInserted) {
+    newNode.forget(aNodeInserted);
+  }
+
+  return NS_OK;
 }
 
 

editor/libeditor/tests/mochitest.ini

@@ -282,6 +282,7 @@ skip-if = android_version == '24'
 [test_middle_click_paste.html]
 subsuite = clipboard
 skip-if = android_version == '24'
+[test_nsIEditorMailSupport_insertAsCitedQuotation.html]
 [test_nsIHTMLEditor_getSelectedElement.html]
 [test_nsIHTMLEditor_selectElement.html]
 [test_nsIHTMLEditor_setCaretAfterElement.html]

editor/libeditor/tests/test_nsIEditorMailSupport_insertAsCitedQuotation.html

@@ -0,0 +1,84 @@
+<!DOCTYPE>
+<html>
+<head>
+  <title>Test for nsIEditorMailSupport.insertAsCitedQuotation()</title>
+  <script src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" href="/tests/SimpleTest/test.css">
+</head>
+<body>
+<div id="display">
+</div>
+<div id="content" contenteditable></div>
+<pre id="test">
+</pre>
+
+<script class="testbody" type="application/javascript">
+
+SimpleTest.waitForExplicitFinish();
+SimpleTest.waitForFocus(function() {
+  let editor = document.getElementById("content");
+  let selection = document.getSelection();
+
+  editor.focus();
+  selection.collapse(editor, 0);
+
+  // Tests when the editor is in plaintext mode.
+
+  getEditor().flags |= SpecialPowers.Ci.nsIPlaintextEditor.eEditorPlaintextMask;
+
+  getEditorMailSupport().insertAsCitedQuotation("this is quoted text\nAnd here is second line.", "this is cited text", false);
+
+  ok(selection.isCollapsed,
+     "Selection should be collapsed after calling nsIEditorMailSupport.insertAsCitedQuotation() of plaintext editor");
+  is(selection.focusNode, editor,
+     "focus node of Selection should be a child of the editing host after calling nsIEditorMailSupport.insertAsCitedQuotation() of plaintext editor");
+  is(selection.focusOffset, 1,
+     "focus offset of Selection should be next to inserted <span> element after calling nsIEditorMailSupport.insertAsCitedQuotation() of plaintext editor");
+  is(editor.innerHTML, '<span style="white-space: pre-wrap;">&gt; this is quoted text<br>&gt; And here is second line.<br><br></span>',
+     "The quoted text should be inserted as plaintext into the plaintext editor");
+
+  // Tests when the editor is in HTML editor mode.
+  getEditor().flags &= ~SpecialPowers.Ci.nsIPlaintextEditor.eEditorPlaintextMask;
+
+  editor.innerHTML = "";
+
+  getEditorMailSupport().insertAsCitedQuotation("this is quoted text<br>", "this is cited text", false);
+
+  ok(selection.isCollapsed,
+     "Selection should be collapsed after calling nsIEditorMailSupport.insertAsCitedQuotation() of HTMLEditor editor (inserting as plaintext)");
+  is(selection.focusNode, editor,
+     "focus node of Selection should be a child of the editing host after calling nsIEditorMailSupport.insertAsCitedQuotation() of HTMLEditor editor (inserting as plaintext)");
+  is(selection.focusOffset, 1,
+     "focus offset of Selection should be next to inserted <span> element after calling nsIEditorMailSupport.insertAsCitedQuotation() of HTMLEditor editor (inserting as plaintext)");
+  is(editor.innerHTML,
+     '<blockquote type="cite" cite="this is cited text">this is quoted text&lt;br&gt;</blockquote>', "The quoted text should be inserted as plaintext into the HTML editor");
+
+  editor.innerHTML = "";
+
+  getEditorMailSupport().insertAsCitedQuotation("this is quoted text<br>And here is second line.", "this is cited text", true);
+
+  ok(selection.isCollapsed,
+     "Selection should be collapsed after calling nsIEditorMailSupport.insertAsCitedQuotation() of HTMLEditor editor (inserting as HTML source)");
+  is(selection.focusNode, editor,
+     "focus node of Selection should be a child of the editing host after calling nsIEditorMailSupport.insertAsCitedQuotation() of HTMLEditor editor (inserting as HTML source)");
+  is(selection.focusOffset, 1,
+     "focus offset of Selection should be next to inserted <span> element after calling nsIEditorMailSupport.insertAsCitedQuotation() of HTMLEditor editor (inserting as HTML source)");
+  is(editor.innerHTML, '<blockquote type="cite" cite="this is cited text">this is quoted text<br>And here is second line.</blockquote>',
+     "The quoted text should be inserted as HTML source into the HTML editor");
+
+  SimpleTest.finish();
+});
+
+function getEditor() {
+  var editingSession = SpecialPowers.wrap(window).docShell.editingSession;
+  return editingSession.getEditorForWindow(window);
+}
+
+function getEditorMailSupport() {
+  return getEditor().QueryInterface(SpecialPowers.Ci.nsIEditorMailSupport);
+}
+
+</script>
+</body>
+
+</html>