Bug 1424676 - part 2: Redesign TextEditRules::CreateBR(), TextEditRules::CreateMozBR() and TextEditRules::CreateBRInternal() with |const EditorRawDOMPoint&| r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 11 Dec 2017 16:37:10 +0900
changeset 450398 67385db77eaf38a4c09d323268a14bec1022dc65
parent 450397 23ee038862c03eb38bcf7910cb69d9f2f4fd7057
child 450399 ffb14ef93c51bc0b415adf873d3fada1bedc8fcb
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1424676
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1424676 - part 2: Redesign TextEditRules::CreateBR(), TextEditRules::CreateMozBR() and TextEditRules::CreateBRInternal() with |const EditorRawDOMPoint&| r=m_kato TextEditRules has some wrappers of TextEditor::CreateBR(). Also they should take |const EditorRawDOMPoint&| to specify the insertion point of new <br> element. Additionally, this patch makes them just return add-refed Element instead of nsresult. So, we can get rid of out param from it. MozReview-Commit-ID: 3wfKGaTM89c
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/TextEditRules.cpp
editor/libeditor/TextEditRules.h
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -5083,18 +5083,20 @@ HTMLEditRules::WillAlign(Selection& aSel
     NS_ENSURE_STATE(div);
     // Remember our new block for postprocessing
     mNewBlock = div;
     // Set up the alignment on the div, using HTML or CSS
     rv = AlignBlock(*div, aAlignType, ContentsOnly::yes);
     NS_ENSURE_SUCCESS(rv, rv);
     *aHandled = true;
     // Put in a moz-br so that it won't get deleted
-    rv = CreateMozBR(*div, 0);
-    NS_ENSURE_SUCCESS(rv, rv);
+    RefPtr<Element> brElement = CreateMozBR(EditorRawDOMPoint(div, 0));
+    if (NS_WARN_IF(!brElement)) {
+      return NS_ERROR_FAILURE;
+    }
     EditorRawDOMPoint atStartOfDiv(div, 0);
     ErrorResult error;
     aSelection.Collapse(atStartOfDiv, error);
     // Don't restore the selection
     selectionRestorer.Abort();
     if (NS_WARN_IF(error.Failed())) {
       return error.StealNSResult();
     }
@@ -6779,18 +6781,20 @@ HTMLEditRules::ReturnInHeader(Selection&
   nsCOMPtr<nsIContent> prevItem = htmlEditor->GetPriorHTMLSibling(&aHeader);
   if (prevItem) {
     MOZ_DIAGNOSTIC_ASSERT(
       HTMLEditUtils::IsHeader(*prevItem));
     bool isEmptyNode;
     rv = htmlEditor->IsEmptyNode(prevItem, &isEmptyNode);
     NS_ENSURE_SUCCESS(rv, rv);
     if (isEmptyNode) {
-      rv = CreateMozBR(*prevItem, 0);
-      NS_ENSURE_SUCCESS(rv, rv);
+      RefPtr<Element> brElement = CreateMozBR(EditorRawDOMPoint(prevItem, 0));
+      if (NS_WARN_IF(!brElement)) {
+        return NS_ERROR_FAILURE;
+      }
     }
   }
 
   // If the new (righthand) header node is empty, delete it
   bool isEmpty;
   rv = IsEmptyBlock(aHeader, &isEmpty, MozBRCounts::no);
   NS_ENSURE_SUCCESS(rv, rv);
   if (isEmpty) {
@@ -7155,18 +7159,20 @@ HTMLEditRules::ReturnInListItem(Selectio
   // extra-inclusive, I have to manually detect certain list items that may be
   // left empty.
   nsCOMPtr<nsIContent> prevItem = htmlEditor->GetPriorHTMLSibling(&aListItem);
   if (prevItem && HTMLEditUtils::IsListItem(prevItem)) {
     bool isEmptyNode;
     rv = htmlEditor->IsEmptyNode(prevItem, &isEmptyNode);
     NS_ENSURE_SUCCESS(rv, rv);
     if (isEmptyNode) {
-      rv = CreateMozBR(*prevItem, 0);
-      NS_ENSURE_SUCCESS(rv, rv);
+      RefPtr<Element> brElement = CreateMozBR(EditorRawDOMPoint(prevItem, 0));
+      if (NS_WARN_IF(!brElement)) {
+        return NS_ERROR_FAILURE;
+      }
     } else {
       rv = htmlEditor->IsEmptyNode(&aListItem, &isEmptyNode, true);
       NS_ENSURE_SUCCESS(rv, rv);
       if (isEmptyNode) {
         RefPtr<nsAtom> nodeAtom = aListItem.NodeInfo()->NameAtom();
         if (nodeAtom == nsGkAtoms::dd || nodeAtom == nsGkAtoms::dt) {
           nsCOMPtr<nsINode> list = aListItem.GetParentNode();
           int32_t itemOffset = list ? list->IndexOf(&aListItem) : -1;
@@ -7879,18 +7885,20 @@ HTMLEditRules::AdjustSpecialBreaks()
   iter.AppendList(functor, nodeArray);
 
   // Put moz-br's into these empty li's and td's
   for (auto& node : nodeArray) {
     // Need to put br at END of node.  It may have empty containers in it and
     // still pass the "IsEmptyNode" test, and we want the br's to be after
     // them.  Also, we want the br to be after the selection if the selection
     // is in this node.
-    nsresult rv = CreateMozBR(*node, (int32_t)node->Length());
-    if (NS_WARN_IF(NS_FAILED(rv))) {
+    EditorRawDOMPoint endOfNode;
+    endOfNode.SetToEndOf(node);
+    RefPtr<Element> brElement = CreateMozBR(endOfNode);
+    if (NS_WARN_IF(!brElement)) {
       return;
     }
   }
 }
 
 nsresult
 HTMLEditRules::AdjustWhitespace(Selection* aSelection)
 {
@@ -8033,135 +8041,136 @@ HTMLEditRules::CheckInterlinePosition(Se
     aSelection.SetInterlinePosition(false);
   }
 }
 
 nsresult
 HTMLEditRules::AdjustSelection(Selection* aSelection,
                                nsIEditor::EDirection aAction)
 {
-  NS_ENSURE_TRUE(aSelection, NS_ERROR_NULL_POINTER);
+  if (NS_WARN_IF(!aSelection)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
+  if (NS_WARN_IF(!mHTMLEditor)) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+  RefPtr<HTMLEditor> htmlEditor(mHTMLEditor);
 
   // if the selection isn't collapsed, do nothing.
   // moose: one thing to do instead is check for the case of
   // only a single break selected, and collapse it.  Good thing?  Beats me.
   if (!aSelection->Collapsed()) {
     return NS_OK;
   }
 
   // get the (collapsed) selection location
   EditorDOMPoint point(EditorBase::GetStartPoint(aSelection));
   if (NS_WARN_IF(!point.IsSet())) {
     return NS_ERROR_FAILURE;
   }
 
   // are we in an editable node?
-  NS_ENSURE_STATE(mHTMLEditor);
-  while (!mHTMLEditor->IsEditable(point.GetContainer())) {
+  while (!htmlEditor->IsEditable(point.GetContainer())) {
     // scan up the tree until we find an editable place to be
     point.Set(point.GetContainer());
     if (NS_WARN_IF(!point.IsSet())) {
       return NS_ERROR_FAILURE;
     }
   }
 
   // make sure we aren't in an empty block - user will see no cursor.  If this
   // is happening, put a <br> in the block if allowed.
-  NS_ENSURE_STATE(mHTMLEditor);
-  nsCOMPtr<Element> theblock = mHTMLEditor->GetBlock(*point.GetContainer());
-
-  if (theblock && mHTMLEditor->IsEditable(theblock)) {
+  RefPtr<Element> theblock = htmlEditor->GetBlock(*point.GetContainer());
+
+  if (theblock && htmlEditor->IsEditable(theblock)) {
     bool bIsEmptyNode;
-    NS_ENSURE_STATE(mHTMLEditor);
     nsresult rv =
-      mHTMLEditor->IsEmptyNode(theblock, &bIsEmptyNode, false, false);
+      htmlEditor->IsEmptyNode(theblock, &bIsEmptyNode, false, false);
     NS_ENSURE_SUCCESS(rv, rv);
     // check if br can go into the destination node
-    NS_ENSURE_STATE(mHTMLEditor);
     if (bIsEmptyNode &&
-        mHTMLEditor->CanContainTag(*point.GetContainer(), *nsGkAtoms::br)) {
-      NS_ENSURE_STATE(mHTMLEditor);
-      nsCOMPtr<Element> rootNode = mHTMLEditor->GetRoot();
-      NS_ENSURE_TRUE(rootNode, NS_ERROR_FAILURE);
-      if (point.GetContainer() == rootNode) {
+        htmlEditor->CanContainTag(*point.GetContainer(), *nsGkAtoms::br)) {
+      Element* rootElement = htmlEditor->GetRoot();
+      if (NS_WARN_IF(!rootElement)) {
+        return NS_ERROR_FAILURE;
+      }
+      if (point.GetContainer() == rootElement) {
         // Our root node is completely empty. Don't add a <br> here.
         // AfterEditInner() will add one for us when it calls
         // CreateBogusNodeIfNeeded()!
         return NS_OK;
       }
 
       // we know we can skip the rest of this routine given the cirumstance
-      return CreateMozBR(*point.GetContainer(), point.Offset());
+      RefPtr<Element> brElement = CreateMozBR(point.AsRaw());
+      if (NS_WARN_IF(!brElement)) {
+        return NS_ERROR_FAILURE;
+      }
+      return NS_OK;
     }
   }
 
   // are we in a text node?
   if (point.IsInTextNode()) {
     return NS_OK; // we LIKE it when we are in a text node.  that RULZ
   }
 
   // do we need to insert a special mozBR?  We do if we are:
   // 1) prior node is in same block where selection is AND
   // 2) prior node is a br AND
   // 3) that br is not visible
 
-  NS_ENSURE_STATE(mHTMLEditor);
   nsCOMPtr<nsIContent> nearNode =
-    mHTMLEditor->GetPreviousEditableHTMLNode(point.AsRaw());
+    htmlEditor->GetPreviousEditableHTMLNode(point.AsRaw());
   if (nearNode) {
     // is nearNode also a descendant of same block?
-    NS_ENSURE_STATE(mHTMLEditor);
-    nsCOMPtr<Element> block = mHTMLEditor->GetBlock(*point.GetContainer());
-    nsCOMPtr<Element> nearBlock = mHTMLEditor->GetBlockNodeParent(nearNode);
+    RefPtr<Element> block = htmlEditor->GetBlock(*point.GetContainer());
+    RefPtr<Element> nearBlock = htmlEditor->GetBlockNodeParent(nearNode);
     if (block && block == nearBlock) {
       if (nearNode && TextEditUtils::IsBreak(nearNode)) {
-        NS_ENSURE_STATE(mHTMLEditor);
-        if (!mHTMLEditor->IsVisibleBRElement(nearNode)) {
+        if (!htmlEditor->IsVisibleBRElement(nearNode)) {
           // need to insert special moz BR. Why?  Because if we don't
           // the user will see no new line for the break.  Also, things
           // like table cells won't grow in height.
-          RefPtr<Element> br;
-          nsresult rv =
-            CreateMozBR(*point.GetContainer(), point.Offset(),
-                        getter_AddRefs(br));
-          NS_ENSURE_SUCCESS(rv, rv);
-          point.Set(br);
+          RefPtr<Element> brElement = CreateMozBR(point.AsRaw());
+          if (NS_WARN_IF(!brElement)) {
+            return NS_ERROR_FAILURE;
+          }
+          point.Set(brElement);
           // selection stays *before* moz-br, sticking to it
           aSelection->SetInterlinePosition(true);
           ErrorResult error;
           aSelection->Collapse(point.AsRaw(), error);
           if (NS_WARN_IF(error.Failed())) {
             return error.StealNSResult();
           }
         } else {
-          NS_ENSURE_STATE(mHTMLEditor);
           nsCOMPtr<nsIContent> nextNode =
-            mHTMLEditor->GetNextEditableHTMLNodeInBlock(*nearNode);
+            htmlEditor->GetNextEditableHTMLNodeInBlock(*nearNode);
           if (nextNode && TextEditUtils::IsMozBR(nextNode)) {
             // selection between br and mozbr.  make it stick to mozbr
             // so that it will be on blank line.
             aSelection->SetInterlinePosition(true);
           }
         }
       }
     }
   }
 
   // we aren't in a textnode: are we adjacent to text or a break or an image?
-  NS_ENSURE_STATE(mHTMLEditor);
-  nearNode = mHTMLEditor->GetPreviousEditableHTMLNodeInBlock(point.AsRaw());
+  nearNode = htmlEditor->GetPreviousEditableHTMLNodeInBlock(point.AsRaw());
   if (nearNode && (TextEditUtils::IsBreak(nearNode) ||
                    EditorBase::IsTextNode(nearNode) ||
                    HTMLEditUtils::IsImage(nearNode) ||
                    nearNode->IsHTMLElement(nsGkAtoms::hr))) {
     // this is a good place for the caret to be
     return NS_OK;
   }
-  NS_ENSURE_STATE(mHTMLEditor);
-  nearNode = mHTMLEditor->GetNextEditableHTMLNodeInBlock(point.AsRaw());
+  nearNode = htmlEditor->GetNextEditableHTMLNodeInBlock(point.AsRaw());
   if (nearNode && (TextEditUtils::IsBreak(nearNode) ||
                    EditorBase::IsTextNode(nearNode) ||
                    nearNode->IsAnyOfHTMLElements(nsGkAtoms::img,
                                                  nsGkAtoms::hr))) {
     return NS_OK; // this is a good place for the caret to be
   }
 
   // look for a nearby text node.
@@ -8757,18 +8766,22 @@ HTMLEditRules::InsertBRIfNeededInternal(
   nsresult rv = mHTMLEditor->IsEmptyNode(&aNode, &isEmpty);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
   if (!isEmpty) {
     return NS_OK;
   }
 
-  return aInsertMozBR ? CreateMozBR(aNode, 0) :
-                        CreateBR(aNode, 0);
+  RefPtr<Element> brElement =
+    CreateBRInternal(EditorRawDOMPoint(&aNode, 0), aInsertMozBR);
+  if (NS_WARN_IF(!brElement)) {
+    return NS_ERROR_FAILURE;
+  }
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 HTMLEditRules::WillCreateNode(const nsAString& aTag,
                               nsIDOMNode* aNextSiblingOfNewNode)
 {
   return NS_OK;
 }
--- a/editor/libeditor/TextEditRules.cpp
+++ b/editor/libeditor/TextEditRules.cpp
@@ -1371,17 +1371,23 @@ TextEditRules::CreateTrailingBRIfNeeded(
   NS_ENSURE_TRUE(body, NS_ERROR_NULL_POINTER);
 
   nsIContent* lastChild = body->GetLastChild();
   // assuming CreateBogusNodeIfNeeded() has been called first
   NS_ENSURE_TRUE(lastChild, NS_ERROR_NULL_POINTER);
 
   if (!lastChild->IsHTMLElement(nsGkAtoms::br)) {
     AutoTransactionsConserveSelection dontChangeMySelection(mTextEditor);
-    return CreateMozBR(*body, body->Length());
+    EditorRawDOMPoint endOfBody;
+    endOfBody.SetToEndOf(body);
+    RefPtr<Element> brElement = CreateMozBR(endOfBody);
+    if (NS_WARN_IF(!brElement)) {
+      return NS_ERROR_FAILURE;
+    }
+    return NS_OK;
   }
 
   // Check to see if the trailing BR is a former bogus node - this will have
   // stuck around if we previously morphed a trailing node into a bogus node.
   if (!mTextEditor->IsMozEditorBogusNode(lastChild)) {
     return NS_OK;
   }
 
@@ -1633,44 +1639,47 @@ TextEditRules::FillBufWithPWChars(nsAStr
   char16_t passwordChar = LookAndFeel::GetPasswordCharacter();
 
   aOutString->Truncate();
   for (int32_t i = 0; i < aLength; i++) {
     aOutString->Append(passwordChar);
   }
 }
 
-nsresult
-TextEditRules::CreateBRInternal(nsINode& inParent,
-                                int32_t inOffset,
-                                bool aMozBR,
-                                Element** aOutBRElement)
+already_AddRefed<Element>
+TextEditRules::CreateBRInternal(const EditorRawDOMPoint& aPointToInsert,
+                                bool aCreateMozBR)
 {
+  if (NS_WARN_IF(!aPointToInsert.IsSet())) {
+    return nullptr;
+  }
+
   if (NS_WARN_IF(!mTextEditor)) {
-    return NS_ERROR_NOT_AVAILABLE;
+    return nullptr;
   }
   RefPtr<TextEditor> textEditor = mTextEditor;
 
-  RefPtr<Element> brElem =
-    textEditor->CreateBR(EditorRawDOMPoint(&inParent, inOffset));
-  if (NS_WARN_IF(!brElem)) {
-    return NS_ERROR_FAILURE;
+  RefPtr<Element> brElement = textEditor->CreateBR(aPointToInsert);
+  if (NS_WARN_IF(!brElement)) {
+    return nullptr;
   }
 
   // give it special moz attr
-  if (aMozBR) {
-    nsresult rv = textEditor->SetAttribute(brElem, nsGkAtoms::type,
+  if (aCreateMozBR) {
+    // XXX Why do we need to set this attribute with transaction?
+    nsresult rv = textEditor->SetAttribute(brElement, nsGkAtoms::type,
                                            NS_LITERAL_STRING("_moz"));
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      // XXX Don't we need to remove the new <br> element from the DOM tree
+      //     in this case?
+      return nullptr;
+    }
   }
 
-  if (aOutBRElement) {
-    brElem.forget(aOutBRElement);
-  }
-  return NS_OK;
+  return brElement.forget();
 }
 
 NS_IMETHODIMP
 TextEditRules::DocumentModified()
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
--- a/editor/libeditor/TextEditRules.h
+++ b/editor/libeditor/TextEditRules.h
@@ -213,43 +213,53 @@ protected:
                                      bool* aTruncated);
 
   /**
    * Remove IME composition text from password buffer.
    */
   void RemoveIMETextFromPWBuf(uint32_t& aStart, nsAString* aIMEString);
 
   /**
-   * Create a normal <br> element and insert it to aOffset at aParent.
+   * Create a normal <br> element and insert it to aPointToInsert.
    *
-   * @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.
+   * @param aPointToInsert  The point where the new <br> element will be
+   *                        inserted.
+   * @return                Returns created <br> element.
    */
-  nsresult CreateBR(nsINode& aParent, int32_t aOffset,
-                    Element** aOutBRNode = nullptr)
+  already_AddRefed<Element> CreateBR(const EditorRawDOMPoint& aPointToInsert)
   {
-    return CreateBRInternal(aParent, aOffset, false, aOutBRNode);
+    return CreateBRInternal(aPointToInsert, false);
   }
 
   /**
-   * Create a moz-<br> element and insert it to aOffset at aParent.
+   * Create a moz-<br> element and insert it to aPointToInsert.
    *
-   * @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.
+   * @param aPointToInsert  The point where the new moz-<br> element will be
+   *                        inserted.
+   * @return                Returns created moz-<br> element.
    */
-  nsresult CreateMozBR(nsINode& aParent, int32_t aOffset,
-                       Element** aOutBRNode = nullptr)
+  already_AddRefed<Element> CreateMozBR(const EditorRawDOMPoint& aPointToInsert)
   {
-    return CreateBRInternal(aParent, aOffset, true, aOutBRNode);
+    return CreateBRInternal(aPointToInsert, true);
   }
 
+  /**
+   * Create a normal <br> element or a moz-<br> element and insert it to
+   * aPointToInsert.
+   *
+   * @param aParentToInsert     The point where the new <br> element will be
+   *                            inserted.
+   * @param aCreateMozBR        true if the caller wants to create a moz-<br>
+   *                            element.  Otherwise, false.
+   * @return                    Returns created <br> element.
+   */
+  already_AddRefed<Element>
+  CreateBRInternal(const EditorRawDOMPoint& aPointToInsert,
+                   bool aCreateMozBR);
+
   void UndefineCaretBidiLevel(Selection* aSelection);
 
   nsresult CheckBidiLevelForDeletion(Selection* aSelection,
                                      nsIDOMNode* aSelNode,
                                      int32_t aSelOffset,
                                      nsIEditor::EDirection aAction,
                                      bool* aCancel);
 
@@ -264,33 +274,16 @@ 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(nsINode& aParent,
-                            int32_t aOffset,
-                            bool aMozBR,
-                            Element** 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;