Bug 1385905 - part2: HTMLEditRules::SplitParagraph() should insert normal <br> element rather than moz-<br> element if split element and/or new element is empty r=m_kato a=jorgk THUNDERBIRD560b1_2017080501_RELBRANCH
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 01 Aug 2017 22:38:50 +0900
branchTHUNDERBIRD560b1_2017080501_RELBRANCH
changeset 420983 395cb2c77448f9d3f305a189c191c7c6c360844b
parent 420982 7a172bce1a933689179ec584f2273855748030e1
child 420984 74e49a52007c609495c6cbc673487ed457838d9e
push id7573
push usermozilla@jorgk.com
push dateSat, 05 Aug 2017 13:03:00 +0000
treeherdermozilla-beta@74e49a52007c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato, jorgk
bugs1385905
milestone56.0
Bug 1385905 - part2: HTMLEditRules::SplitParagraph() should insert normal <br> element rather than moz-<br> element if split element and/or new element is empty r=m_kato a=jorgk Currently, HTMLEditRules::SplitParagraph() inserts moz-<br> element when split element and/or new element causes empty line. However, PlaintextSerializer ignores moz-<br> elements. Therefore, empty lines which are created by SplitParagraph() will be removed when it's converted to plaintext. So, it should insert normal <br> element for placeholder of empty lines instead. Note that moz-<br> elements are appeared as normal <br> elements in the result of Element.innerHTML. Additionally, Chromium always inserts a <br> element for empty block elements which are created by Enter key press. Therefore, using normal <br> element in this case shouldn't cause any compatibility problems. MozReview-Commit-ID: FNV41zEFWqQ
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
editor/libeditor/TextEditRules.cpp
editor/libeditor/TextEditRules.h
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -6611,20 +6611,25 @@ HTMLEditRules::SplitParagraph(nsIDOMNode
   }
 
   // remove ID attribute on the paragraph we just created
   RefPtr<Element> rightElt = rightPara->AsElement();
   NS_ENSURE_STATE(mHTMLEditor);
   rv = mHTMLEditor->RemoveAttribute(rightElt, nsGkAtoms::id);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // check both halves of para to see if we need mozBR
-  rv = InsertMozBRIfNeeded(*leftPara);
+  // We need to ensure to both paragraphs visible even if they are empty.
+  // However, moz-<br> element isn't useful in this case because moz-<br>
+  // elements will be ignored by PlaintextSerializer.  Additionally,
+  // moz-<br> will be exposed as <br> with Element.innerHTML.  Therefore,
+  // we can use normal <br> elements for placeholder in this case.
+  // Note that Chromium also behaves so.
+  rv = InsertBRIfNeeded(*leftPara);
   NS_ENSURE_SUCCESS(rv, rv);
-  rv = InsertMozBRIfNeeded(*rightPara);
+  rv = InsertBRIfNeeded(*rightPara);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // selection to beginning of right hand para;
   // look inside any containers that are up front.
   nsCOMPtr<nsINode> rightParaNode = do_QueryInterface(rightPara);
   NS_ENSURE_STATE(mHTMLEditor && rightParaNode);
   nsCOMPtr<nsIDOMNode> child =
     GetAsDOMNode(mHTMLEditor->GetLeftmostChild(rightParaNode, true));
@@ -8231,31 +8236,37 @@ HTMLEditRules::UpdateDocChangeRange(nsRa
       rv = mDocChangeRange->SetEnd(endNode, endOffset);
       NS_ENSURE_SUCCESS(rv, rv);
     }
   }
   return NS_OK;
 }
 
 nsresult
-HTMLEditRules::InsertMozBRIfNeeded(nsINode& aNode)
+HTMLEditRules::InsertBRIfNeededInternal(nsINode& aNode,
+                                        bool aInsertMozBR)
 {
   if (!IsBlockNode(aNode)) {
     return NS_OK;
   }
 
+  if (NS_WARN_IF(!mHTMLEditor)) {
+    return NS_ERROR_UNEXPECTED;
+  }
   bool isEmpty;
-  NS_ENSURE_STATE(mHTMLEditor);
   nsresult rv = mHTMLEditor->IsEmptyNode(&aNode, &isEmpty);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
   if (!isEmpty) {
     return NS_OK;
   }
 
-  return CreateMozBR(aNode.AsDOMNode(), 0);
+  return aInsertMozBR ? CreateMozBR(aNode.AsDOMNode(), 0) :
+                        CreateBR(aNode.AsDOMNode(), 0);
 }
 
 NS_IMETHODIMP
 HTMLEditRules::WillCreateNode(const nsAString& aTag,
                               nsIDOMNode* aParent,
                               int32_t aPosition)
 {
   return NS_OK;
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -166,16 +166,28 @@ protected:
   nsresult WillDeleteSelection(Selection* aSelection,
                                nsIEditor::EDirection aAction,
                                nsIEditor::EStripWrappers aStripWrappers,
                                bool* aCancel, bool* aHandled);
   nsresult DidDeleteSelection(Selection* aSelection,
                               nsIEditor::EDirection aDir,
                               nsresult aResult);
   nsresult InsertBRIfNeeded(Selection* aSelection);
+
+  /**
+   * Insert a normal <br> element or a moz-<br> element to aNode when
+   * aNode is a block and it has no children.
+   *
+   * @param aNode           Reference to a block parent.
+   * @param aInsertMozBR    true if this should insert a moz-<br> element.
+   *                        Otherwise, i.e., this should insert a normal <br>
+   *                        element, false.
+   */
+  nsresult InsertBRIfNeededInternal(nsINode& aNode, bool aInsertMozBR);
+
   mozilla::EditorDOMPoint GetGoodSelPointForNode(nsINode& aNode,
                                                  nsIEditor::EDirection aAction);
 
   /**
    * TryToJoinBlocks() tries to join two block elements.  The right element is
    * always joined to the left element.  If the elements are the same type and
    * not nested within each other, JoinNodesSmart() is called (example, joining
    * two list items together into one).  If the elements are not the same type,
@@ -397,17 +409,35 @@ protected:
    * table element is its own nearest table element ancestor.
    */
   bool InDifferentTableElements(nsIDOMNode* aNode1, nsIDOMNode* aNode2);
   bool InDifferentTableElements(nsINode* aNode1, nsINode* aNode2);
   nsresult RemoveEmptyNodes();
   nsresult SelectionEndpointInNode(nsINode* aNode, bool* aResult);
   nsresult UpdateDocChangeRange(nsRange* aRange);
   nsresult ConfirmSelectionInBody();
-  nsresult InsertMozBRIfNeeded(nsINode& aNode);
+
+  /**
+   * Insert normal <br> element into aNode when aNode is a block and it has
+   * no children.
+   */
+  nsresult InsertBRIfNeeded(nsINode& aNode)
+  {
+    return InsertBRIfNeededInternal(aNode, false);
+  }
+
+  /**
+   * Insert moz-<br> element (<br type="_moz">) into aNode when aNode is a
+   * block and it has no children.
+   */
+  nsresult InsertMozBRIfNeeded(nsINode& aNode)
+  {
+    return InsertBRIfNeededInternal(aNode, true);
+  }
+
   bool IsEmptyInline(nsINode& aNode);
   bool ListIsEmptyLine(nsTArray<OwningNonNull<nsINode>>& arrayOfNodes);
   nsresult RemoveAlignment(nsINode& aNode, const nsAString& aAlignType,
                            bool aChildrenOnly);
   nsresult MakeSureElemStartsOrEndsOnCR(nsINode& aNode, bool aStarts);
   enum class ContentsOnly { no, yes };
   nsresult AlignBlock(Element& aElement,
                       const nsAString& aAlignType, ContentsOnly aContentsOnly);
--- a/editor/libeditor/TextEditRules.cpp
+++ b/editor/libeditor/TextEditRules.cpp
@@ -1643,34 +1643,32 @@ TextEditRules::FillBufWithPWChars(nsAStr
   char16_t passwordChar = LookAndFeel::GetPasswordCharacter();
 
   aOutString->Truncate();
   for (int32_t i = 0; i < aLength; i++) {
     aOutString->Append(passwordChar);
   }
 }
 
-/**
- * CreateMozBR() puts a BR node with moz attribute at {inParent, inOffset}.
- */
 nsresult
-TextEditRules::CreateMozBR(nsIDOMNode* inParent,
-                           int32_t inOffset,
-                           nsIDOMNode** outBRNode)
+TextEditRules::CreateBRInternal(nsIDOMNode* inParent,
+                                int32_t inOffset,
+                                bool aMozBR,
+                                nsIDOMNode** outBRNode)
 {
   NS_ENSURE_TRUE(inParent, NS_ERROR_NULL_POINTER);
 
   nsCOMPtr<nsIDOMNode> brNode;
   NS_ENSURE_STATE(mTextEditor);
   nsresult rv = mTextEditor->CreateBR(inParent, inOffset, address_of(brNode));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // give it special moz attr
   nsCOMPtr<Element> brElem = do_QueryInterface(brNode);
-  if (brElem) {
+  if (aMozBR && brElem) {
     rv = mTextEditor->SetAttribute(brElem, nsGkAtoms::type,
                                    NS_LITERAL_STRING("_moz"));
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   if (outBRNode) {
     brNode.forget(outBRNode);
   }
--- a/editor/libeditor/TextEditRules.h
+++ b/editor/libeditor/TextEditRules.h
@@ -207,18 +207,43 @@ protected:
                                      int32_t aMaxLength,
                                      bool* aTruncated);
 
   /**
    * Remove IME composition text from password buffer.
    */
   void RemoveIMETextFromPWBuf(uint32_t& aStart, nsAString* aIMEString);
 
-  nsresult CreateMozBR(nsIDOMNode* inParent, int32_t inOffset,
-                       nsIDOMNode** outBRNode = nullptr);
+  /**
+   * Create a normal <br> element and insert it to aOffset at aParent.
+   *
+   * @param aParent     The parent node which will have new <br> element.
+   * @param aOffset     The offset in aParent where the new <br> element will
+   *                    be inserted.
+   * @param aOutBRNode  Returns created <br> element.
+   */
+  nsresult CreateBR(nsIDOMNode* aParent, int32_t aOffset,
+                    nsIDOMNode** aOutBRNode = nullptr)
+  {
+    return CreateBRInternal(aParent, aOffset, false, aOutBRNode);
+  }
+
+  /**
+   * Create a moz-<br> element and insert it to aOffset at aParent.
+   *
+   * @param aParent     The parent node which will have new <br> element.
+   * @param aOffset     The offset in aParent where the new <br> element will
+   *                    be inserted.
+   * @param aOutBRNode  Returns created <br> element.
+   */
+  nsresult CreateMozBR(nsIDOMNode* aParent, int32_t aOffset,
+                       nsIDOMNode** aOutBRNode = nullptr)
+  {
+    return CreateBRInternal(aParent, aOffset, true, aOutBRNode);
+  }
 
   void UndefineCaretBidiLevel(Selection* aSelection);
 
   nsresult CheckBidiLevelForDeletion(Selection* aSelection,
                                      nsIDOMNode* aSelNode,
                                      int32_t aSelOffset,
                                      nsIEditor::EDirection aAction,
                                      bool* aCancel);
@@ -234,16 +259,33 @@ protected:
   bool IsDisabled() const;
   bool IsMailEditor() const;
   bool DontEchoPassword() const;
 
 private:
   // Note that we do not refcount the editor.
   TextEditor* mTextEditor;
 
+  /**
+   * Create a normal <br> element or a moz-<br> element and insert it to
+   * aOffset at aParent.
+   *
+   * @param aParent     The parent node which will have new <br> element.
+   * @param aOffset     The offset in aParent where the new <br> element will
+   *                    be inserted.
+   * @param aMozBR      true if the caller wants to create a moz-<br> element.
+   *                    Otherwise, false.
+   * @param aOutBRNode  Returns created <br> element.
+   */
+  nsresult CreateBRInternal(nsIDOMNode* aParent,
+                            int32_t aOffset,
+                            bool aMozBR,
+                            nsIDOMNode** aOutBRNode = nullptr);
+
+
 protected:
   // A buffer we use to store the real value of password editors.
   nsString mPasswordText;
   // A buffer we use to track the IME composition string.
   nsString mPasswordIMEText;
   uint32_t mPasswordIMEIndex;
   // Magic node acts as placeholder in empty doc.
   nsCOMPtr<nsIContent> mBogusNode;