Bug 1451672 - part 11: Rename EditorBase::SetAttribute(), EditorBase::RemoveAttribute() and EditorBase::CloneAttribute() with "WithTransaction" postfix r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 12 Apr 2018 16:58:33 +0900
changeset 468364 4076543894020d6614e523e805b13b35e511cb45
parent 468363 81798a8610e1b350998ea29ef177fcd5a8c0b801
child 468365 4b372e4cc463abb94c3fbc5ccaf2870ffceec894
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1451672
milestone61.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 1451672 - part 11: Rename EditorBase::SetAttribute(), EditorBase::RemoveAttribute() and EditorBase::CloneAttribute() with "WithTransaction" postfix r=m_kato MozReview-Commit-ID: I8T9MNkY8Yq
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditor.cpp
editor/libeditor/HTMLEditorObjectResizer.cpp
editor/libeditor/HTMLStyleEditor.cpp
editor/libeditor/HTMLTableEditor.cpp
editor/libeditor/TextEditRules.cpp
editor/libeditor/TextEditor.cpp
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -1194,32 +1194,33 @@ EditorBase::CanPasteTransferable(nsITran
 }
 
 NS_IMETHODIMP
 EditorBase::SetAttribute(nsIDOMElement* aElement,
                          const nsAString& aAttribute,
                          const nsAString& aValue)
 {
   if (NS_WARN_IF(aAttribute.IsEmpty())) {
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_INVALID_ARG;
   }
   nsCOMPtr<Element> element = do_QueryInterface(aElement);
-  NS_ENSURE_TRUE(element, NS_ERROR_NULL_POINTER);
+  if (NS_WARN_IF(!element)) {
+    return NS_ERROR_INVALID_ARG;
+  }
   RefPtr<nsAtom> attribute = NS_Atomize(aAttribute);
-
-  return SetAttribute(element, attribute, aValue);
+  return SetAttributeWithTransaction(*element, *attribute, aValue);
 }
 
 nsresult
-EditorBase::SetAttribute(Element* aElement,
-                         nsAtom* aAttribute,
-                         const nsAString& aValue)
+EditorBase::SetAttributeWithTransaction(Element& aElement,
+                                        nsAtom& aAttribute,
+                                        const nsAString& aValue)
 {
   RefPtr<ChangeAttributeTransaction> transaction =
-    ChangeAttributeTransaction::Create(*aElement, *aAttribute, aValue);
+    ChangeAttributeTransaction::Create(aElement, aAttribute, aValue);
   return DoTransaction(transaction);
 }
 
 NS_IMETHODIMP
 EditorBase::GetAttributeValue(nsIDOMElement* aElement,
                               const nsAString& aAttribute,
                               nsAString& aResultValue,
                               bool* aResultIsSet)
@@ -1239,31 +1240,35 @@ EditorBase::GetAttributeValue(nsIDOMElem
   return NS_OK;
 }
 
 NS_IMETHODIMP
 EditorBase::RemoveAttribute(nsIDOMElement* aElement,
                             const nsAString& aAttribute)
 {
   if (NS_WARN_IF(aAttribute.IsEmpty())) {
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_INVALID_ARG;
   }
   nsCOMPtr<Element> element = do_QueryInterface(aElement);
-  NS_ENSURE_TRUE(element, NS_ERROR_NULL_POINTER);
+  if (NS_WARN_IF(!element)) {
+    return NS_ERROR_INVALID_ARG;
+  }
   RefPtr<nsAtom> attribute = NS_Atomize(aAttribute);
-
-  return RemoveAttribute(element, attribute);
+  return RemoveAttributeWithTransaction(*element, *attribute);
 }
 
 nsresult
-EditorBase::RemoveAttribute(Element* aElement,
-                            nsAtom* aAttribute)
-{
+EditorBase::RemoveAttributeWithTransaction(Element& aElement,
+                                           nsAtom& aAttribute)
+{
+  // XXX If aElement doesn't have aAttribute, shouldn't we stop creating
+  //     the transaction?  Otherwise, there will be added a transaction
+  //     which does nothing at doing undo/redo.
   RefPtr<ChangeAttributeTransaction> transaction =
-    ChangeAttributeTransaction::CreateToRemove(*aElement, *aAttribute);
+    ChangeAttributeTransaction::CreateToRemove(aElement, aAttribute);
   return DoTransaction(transaction);
 }
 
 NS_IMETHODIMP
 EditorBase::MarkNodeDirty(nsIDOMNode* aNode)
 {
   // Mark the node dirty, but not for webpages (bug 599983)
   if (!OutputsMozDirty()) {
@@ -2520,32 +2525,34 @@ EditorBase::CloneAttribute(const nsAStri
 {
   NS_ENSURE_TRUE(aDestNode && aSourceNode, NS_ERROR_NULL_POINTER);
   if (NS_WARN_IF(aAttribute.IsEmpty())) {
     return NS_ERROR_FAILURE;
   }
 
   nsCOMPtr<Element> destElement = do_QueryInterface(aDestNode);
   nsCOMPtr<Element> sourceElement = do_QueryInterface(aSourceNode);
-  NS_ENSURE_TRUE(destElement && sourceElement, NS_ERROR_NO_INTERFACE);
+  if (NS_WARN_IF(!destElement) || NS_WARN_IF(!sourceElement)) {
+    return NS_ERROR_INVALID_ARG;
+  }
 
   RefPtr<nsAtom> attribute = NS_Atomize(aAttribute);
-  return CloneAttribute(attribute, destElement, sourceElement);
+  return CloneAttributeWithTransaction(*attribute, *destElement, *sourceElement);
 }
 
 nsresult
-EditorBase::CloneAttribute(nsAtom* aAttribute,
-                           Element* aDestElement,
-                           Element* aSourceElement)
+EditorBase::CloneAttributeWithTransaction(nsAtom& aAttribute,
+                                          Element& aDestElement,
+                                          Element& aSourceElement)
 {
   nsAutoString attrValue;
-  if (aSourceElement->GetAttr(kNameSpaceID_None, aAttribute, attrValue)) {
-    return SetAttribute(aDestElement, aAttribute, attrValue);
-  }
-  return RemoveAttribute(aDestElement, aAttribute);
+  if (aSourceElement.GetAttr(kNameSpaceID_None, &aAttribute, attrValue)) {
+    return SetAttributeWithTransaction(aDestElement, aAttribute, attrValue);
+  }
+  return RemoveAttributeWithTransaction(aDestElement, aAttribute);
 }
 
 /**
  * @param aDest     Must be a DOM element.
  * @param aSource   Must be a DOM element.
  */
 NS_IMETHODIMP
 EditorBase::CloneAttributes(nsIDOMNode* aDest,
@@ -2574,17 +2581,17 @@ EditorBase::CloneAttributes(Element* aDe
   // document
   NS_ENSURE_TRUE(GetRoot(), );
   bool destInBody = GetRoot()->Contains(aDest);
 
   // Clear existing attributes
   RefPtr<nsDOMAttributeMap> destAttributes = aDest->Attributes();
   while (RefPtr<Attr> attr = destAttributes->Item(0)) {
     if (destInBody) {
-      RemoveAttribute(aDest, attr->NodeInfo()->NameAtom());
+      RemoveAttributeWithTransaction(*aDest, *attr->NodeInfo()->NameAtom());
     } else {
       aDest->UnsetAttr(kNameSpaceID_None, attr->NodeInfo()->NameAtom(), true);
     }
   }
 
   // Set just the attributes that the source element has
   RefPtr<nsDOMAttributeMap> sourceAttributes = aSource->Attributes();
   uint32_t sourceCount = sourceAttributes->Length();
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -450,24 +450,55 @@ public:
    * @param aError              The result.  If this succeeds to move children,
    *                            returns NS_OK.  Otherwise, an error.
    */
   void MoveChildren(nsIContent& aFirstChild,
                     nsIContent& aLastChild,
                     const EditorRawDOMPoint& aPointToInsert,
                     ErrorResult& aError);
 
-  nsresult CloneAttribute(nsAtom* aAttribute, Element* aDestElement,
-                          Element* aSourceElement);
-  nsresult RemoveAttribute(Element* aElement, nsAtom* aAttribute);
+  /**
+   * CloneAttributeWithTransaction() copies aAttribute of aSourceElement to
+   * aDestElement.  If aSourceElement doesn't have aAttribute, this removes
+   * aAttribute from aDestElement.
+   *
+   * @param aAttribute          Attribute name to be cloned.
+   * @param aDestElement        Element node which will be set aAttribute or
+   *                            whose aAttribute will be removed.
+   * @param aSourceElement      Element node which provides the value of
+   *                            aAttribute in aDestElement.
+   */
+  nsresult CloneAttributeWithTransaction(nsAtom& aAttribute,
+                                         Element& aDestElement,
+                                         Element& aSourceElement);
+
+  /**
+   * RemoveAttributeWithTransaction() removes aAttribute from aElement.
+   *
+   * @param aElement        Element node which will lose aAttribute.
+   * @param aAttribute      Attribute name to be removed from aElement.
+   */
+  nsresult RemoveAttributeWithTransaction(Element& aElement,
+                                          nsAtom& aAttribute);
+
   virtual nsresult RemoveAttributeOrEquivalent(Element* aElement,
                                                nsAtom* aAttribute,
                                                bool aSuppressTransaction) = 0;
-  nsresult SetAttribute(Element* aElement, nsAtom* aAttribute,
-                        const nsAString& aValue);
+
+  /**
+   * SetAttributeWithTransaction() sets aAttribute of aElement to aValue.
+   *
+   * @param aElement        Element node which will have aAttribute.
+   * @param aAttribute      Attribute name to be set.
+   * @param aValue          Attribute value be set to aAttribute.
+   */
+  nsresult SetAttributeWithTransaction(Element& aElement,
+                                       nsAtom& aAttribute,
+                                       const nsAString& aValue);
+
   virtual nsresult SetAttributeOrEquivalent(Element* aElement,
                                             nsAtom* aAttribute,
                                             const nsAString& aValue,
                                             bool aSuppressTransaction) = 0;
 
   /**
    * Method to replace certain CreateElementNS() calls.
    *
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -3802,24 +3802,29 @@ HTMLEditRules::WillMakeList(Selection* a
         }
         if (!curNode->IsHTMLElement(itemType)) {
           newBlock = htmlEditor->ReplaceContainer(curNode->AsElement(),
                                                   itemType);
           NS_ENSURE_STATE(newBlock);
         }
       }
       nsCOMPtr<Element> curElement = do_QueryInterface(curNode);
+      if (NS_WARN_IF(!curElement)) {
+        return NS_ERROR_FAILURE;
+      }
       if (aBulletType && !aBulletType->IsEmpty()) {
-        rv = htmlEditor->SetAttribute(curElement, nsGkAtoms::type,
-                                      *aBulletType);
+        rv = htmlEditor->SetAttributeWithTransaction(*curElement,
+                                                     *nsGkAtoms::type,
+                                                     *aBulletType);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
       } else {
-        rv = htmlEditor->RemoveAttribute(curElement, nsGkAtoms::type);
+        rv = htmlEditor->RemoveAttributeWithTransaction(*curElement,
+                                                        *nsGkAtoms::type);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
       }
       continue;
     }
 
     // if we hit a div clear our prevListItem, insert divs contents
@@ -7296,17 +7301,18 @@ HTMLEditRules::SplitParagraph(
   if (aNextBRNode && htmlEditor->IsVisibleBRElement(aNextBRNode)) {
     rv = htmlEditor->DeleteNodeWithTransaction(*aNextBRNode);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   }
 
   // Remove ID attribute on the paragraph from the existing right node.
-  rv = htmlEditor->RemoveAttribute(aParentDivOrP.AsElement(), nsGkAtoms::id);
+  rv = htmlEditor->RemoveAttributeWithTransaction(*aParentDivOrP.AsElement(),
+                                                  *nsGkAtoms::id);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // 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.
@@ -9260,19 +9266,22 @@ HTMLEditRules::RemoveAlignment(nsINode& 
       NS_ENSURE_STATE(mHTMLEditor);
       rv = mHTMLEditor->RemoveContainer(child->AsElement());
       NS_ENSURE_SUCCESS(rv, rv);
     } else if (IsBlockNode(*child) || child->IsHTMLElement(nsGkAtoms::hr)) {
       // the current node is a block element
       if (HTMLEditUtils::SupportsAlignAttr(*child)) {
         // remove the ALIGN attribute if this element can have it
         NS_ENSURE_STATE(mHTMLEditor);
-        nsresult rv = mHTMLEditor->RemoveAttribute(child->AsElement(),
-                                                   nsGkAtoms::align);
-        NS_ENSURE_SUCCESS(rv, rv);
+        nsresult rv =
+          mHTMLEditor->RemoveAttributeWithTransaction(*child->AsElement(),
+                                                      *nsGkAtoms::align);
+        if (NS_WARN_IF(NS_FAILED(rv))) {
+          return rv;
+        }
       }
       if (useCSS) {
         if (child->IsAnyOfHTMLElements(nsGkAtoms::table, nsGkAtoms::hr)) {
           NS_ENSURE_STATE(mHTMLEditor);
           nsresult rv =
             mHTMLEditor->SetAttributeOrEquivalent(child->AsElement(),
                                                   nsGkAtoms::align,
                                                   aAlignType, false);
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -2755,69 +2755,84 @@ HTMLEditor::InsertLinkAroundSelection(ns
 }
 
 nsresult
 HTMLEditor::SetHTMLBackgroundColor(const nsAString& aColor)
 {
   MOZ_ASSERT(IsInitialized(), "The HTMLEditor hasn't been initialized yet");
 
   // Find a selected or enclosing table element to set background on
-  nsCOMPtr<nsIDOMElement> element;
+  nsCOMPtr<nsIDOMElement> domElement;
   int32_t selectedCount;
   nsAutoString tagName;
   nsresult rv = GetSelectedOrParentTableElement(tagName, &selectedCount,
-                                                getter_AddRefs(element));
+                                                getter_AddRefs(domElement));
   NS_ENSURE_SUCCESS(rv, rv);
 
   bool setColor = !aColor.IsEmpty();
 
-  NS_NAMED_LITERAL_STRING(bgcolor, "bgcolor");
-  if (element) {
+  nsCOMPtr<Element> element = nullptr;
+  RefPtr<nsAtom> bgColorAtom = NS_Atomize("bgcolor");
+  if (domElement) {
     if (selectedCount > 0) {
       // Traverse all selected cells
-      nsCOMPtr<nsIDOMElement> cell;
-      rv = GetFirstSelectedCell(nullptr, getter_AddRefs(cell));
-      if (NS_SUCCEEDED(rv) && cell) {
-        while (cell) {
-          rv = setColor ? SetAttribute(cell, bgcolor, aColor) :
-                          RemoveAttribute(cell, bgcolor);
+      nsCOMPtr<nsIDOMElement> domCell;
+      rv = GetFirstSelectedCell(nullptr, getter_AddRefs(domCell));
+      if (NS_SUCCEEDED(rv) && domCell) {
+        while (domCell) {
+          nsCOMPtr<Element> cell = do_QueryInterface(domCell);
+          if (NS_WARN_IF(!cell)) {
+            return NS_ERROR_FAILURE;
+          }
+          rv = setColor ?
+                 SetAttributeWithTransaction(*cell, *bgColorAtom, aColor) :
+                 RemoveAttributeWithTransaction(*cell, *bgColorAtom);
           if (NS_FAILED(rv)) {
             return rv;
           }
-
-          GetNextSelectedCell(nullptr, getter_AddRefs(cell));
+          GetNextSelectedCell(nullptr, getter_AddRefs(domCell));
         }
         return NS_OK;
       }
     }
     // If we failed to find a cell, fall through to use originally-found element
+    element = do_QueryInterface(domElement);
+    if (NS_WARN_IF(!element)) {
+      return NS_ERROR_FAILURE;
+    }
   } else {
     // No table element -- set the background color on the body tag
-    element = do_QueryInterface(GetRoot());
-    NS_ENSURE_TRUE(element, NS_ERROR_NULL_POINTER);
+    element = GetRoot();
+    if (NS_WARN_IF(!element)) {
+      return NS_ERROR_FAILURE;
+    }
   }
   // Use the editor method that goes through the transaction system
-  return setColor ? SetAttribute(element, bgcolor, aColor) :
-                    RemoveAttribute(element, bgcolor);
+  return setColor ?
+           SetAttributeWithTransaction(*element, *bgColorAtom, aColor) :
+           RemoveAttributeWithTransaction(*element, *bgColorAtom);
 }
 
 NS_IMETHODIMP
 HTMLEditor::SetBodyAttribute(const nsAString& aAttribute,
                              const nsAString& aValue)
 {
   // TODO: Check selection for Cell, Row, Column or table and do color on appropriate level
 
   MOZ_ASSERT(IsInitialized(), "The HTMLEditor hasn't been initialized yet");
 
   // Set the background color attribute on the body tag
-  nsCOMPtr<nsIDOMElement> bodyElement = do_QueryInterface(GetRoot());
-  NS_ENSURE_TRUE(bodyElement, NS_ERROR_NULL_POINTER);
+  RefPtr<Element> rootElement = GetRoot();
+  if (NS_WARN_IF(!rootElement)) {
+    return NS_ERROR_FAILURE;
+  }
 
   // Use the editor method that goes through the transaction system
-  return SetAttribute(bodyElement, aAttribute, aValue);
+  RefPtr<nsAtom> attributeAtom = NS_Atomize(aAttribute);
+  return SetAttributeWithTransaction(*rootElement, *attributeAtom, aValue);
 }
 
 NS_IMETHODIMP
 HTMLEditor::GetLinkedObjects(nsIArray** aNodeList)
 {
   NS_ENSURE_TRUE(aNodeList, NS_ERROR_NULL_POINTER);
 
   nsresult rv;
@@ -4194,56 +4209,56 @@ HTMLEditor::SetAttributeOrEquivalent(Ele
     // we are not in an HTML+CSS editor; let's set the attribute the HTML way
     if (mCSSEditUtils) {
       mCSSEditUtils->RemoveCSSEquivalentToHTMLStyle(aElement, nullptr,
                                                     aAttribute, nullptr,
                                                     aSuppressTransaction);
     }
     return aSuppressTransaction ?
              aElement->SetAttr(kNameSpaceID_None, aAttribute, aValue, true) :
-             SetAttribute(aElement, aAttribute, aValue);
+             SetAttributeWithTransaction(*aElement, *aAttribute, aValue);
   }
 
   int32_t count =
     mCSSEditUtils->SetCSSEquivalentToHTMLStyle(aElement, nullptr,
                                                aAttribute, &aValue,
                                                aSuppressTransaction);
   if (count) {
     // we found an equivalence ; let's remove the HTML attribute itself if it
     // is set
     nsAutoString existingValue;
     if (!aElement->GetAttr(kNameSpaceID_None, aAttribute, existingValue)) {
       return NS_OK;
     }
 
     return aSuppressTransaction ?
              aElement->UnsetAttr(kNameSpaceID_None, aAttribute, true) :
-             RemoveAttribute(aElement, aAttribute);
+             RemoveAttributeWithTransaction(*aElement, *aAttribute);
   }
 
   // count is an integer that represents the number of CSS declarations applied
   // to the element. If it is zero, we found no equivalence in this
   // implementation for the attribute
   if (aAttribute == nsGkAtoms::style) {
     // if it is the style attribute, just add the new value to the existing
     // style attribute's value
     nsAutoString existingValue;
     aElement->GetAttr(kNameSpaceID_None, nsGkAtoms::style, existingValue);
     existingValue.Append(' ');
     existingValue.Append(aValue);
     return aSuppressTransaction ?
        aElement->SetAttr(kNameSpaceID_None, aAttribute, existingValue, true) :
-      SetAttribute(aElement, aAttribute, existingValue);
+      SetAttributeWithTransaction(*aElement, *aAttribute, existingValue);
   }
 
   // we have no CSS equivalence for this attribute and it is not the style
   // attribute; let's set it the good'n'old HTML way
   return aSuppressTransaction ?
            aElement->SetAttr(kNameSpaceID_None, aAttribute, aValue, true) :
-           SetAttribute(aElement, aAttribute, aValue);
+           SetAttributeWithTransaction(*aElement, *aAttribute, aValue);
 }
 
 nsresult
 HTMLEditor::RemoveAttributeOrEquivalent(Element* aElement,
                                         nsAtom* aAttribute,
                                         bool aSuppressTransaction)
 {
   MOZ_ASSERT(aElement);
@@ -4257,17 +4272,17 @@ HTMLEditor::RemoveAttributeOrEquivalent(
   }
 
   if (!aElement->HasAttr(kNameSpaceID_None, aAttribute)) {
     return NS_OK;
   }
 
   return aSuppressTransaction ?
     aElement->UnsetAttr(kNameSpaceID_None, aAttribute, /* aNotify = */ true) :
-    RemoveAttribute(aElement, aAttribute);
+    RemoveAttributeWithTransaction(*aElement, *aAttribute);
 }
 
 nsresult
 HTMLEditor::SetIsCSSEnabled(bool aIsCSSPrefChecked)
 {
   if (!mCSSEditUtils) {
     return NS_ERROR_NOT_INITIALIZED;
   }
--- a/editor/libeditor/HTMLEditorObjectResizer.cpp
+++ b/editor/libeditor/HTMLEditorObjectResizer.cpp
@@ -871,22 +871,22 @@ HTMLEditor::SetFinalSize(int32_t aX,
       mCSSEditUtils->SetCSSPropertyPixels(*mResizedObject, *nsGkAtoms::top, y);
     }
     if (setWidth) {
       mCSSEditUtils->SetCSSPropertyPixels(*mResizedObject, *nsGkAtoms::left, x);
     }
   }
   if (IsCSSEnabled() || mResizedObjectIsAbsolutelyPositioned) {
     if (setWidth && mResizedObject->HasAttr(kNameSpaceID_None, nsGkAtoms::width)) {
-      RemoveAttribute(mResizedObject, nsGkAtoms::width);
+      RemoveAttributeWithTransaction(*mResizedObject, *nsGkAtoms::width);
     }
 
     if (setHeight && mResizedObject->HasAttr(kNameSpaceID_None,
                                              nsGkAtoms::height)) {
-      RemoveAttribute(mResizedObject, nsGkAtoms::height);
+      RemoveAttributeWithTransaction(*mResizedObject, *nsGkAtoms::height);
     }
 
     if (setWidth) {
       mCSSEditUtils->SetCSSPropertyPixels(*mResizedObject, *nsGkAtoms::width,
                                           width);
     }
     if (setHeight) {
       mCSSEditUtils->SetCSSPropertyPixels(*mResizedObject, *nsGkAtoms::height,
@@ -904,22 +904,22 @@ HTMLEditor::SetFinalSize(int32_t aX,
     }
     if (setHeight) {
       mCSSEditUtils->SetCSSPropertyPixels(*mResizedObject, *nsGkAtoms::height,
                                           height);
     }
     if (setWidth) {
       nsAutoString w;
       w.AppendInt(width);
-      SetAttribute(mResizedObject, nsGkAtoms::width, w);
+      SetAttributeWithTransaction(*mResizedObject, *nsGkAtoms::width, w);
     }
     if (setHeight) {
       nsAutoString h;
       h.AppendInt(height);
-      SetAttribute(mResizedObject, nsGkAtoms::height, h);
+      SetAttributeWithTransaction(*mResizedObject, *nsGkAtoms::height, h);
     }
 
     if (setWidth) {
       mCSSEditUtils->RemoveCSSProperty(*mResizedObject, *nsGkAtoms::width,
                                        EmptyString());
     }
     if (setHeight) {
       mCSSEditUtils->RemoveCSSProperty(*mResizedObject, *nsGkAtoms::height,
--- a/editor/libeditor/HTMLStyleEditor.cpp
+++ b/editor/libeditor/HTMLStyleEditor.cpp
@@ -412,18 +412,21 @@ HTMLEditor::SetInlinePropertyOnNodeImpl(
     mCSSEditUtils->SetCSSEquivalentToHTMLStyle(tmp,
                                                &aProperty, aAttribute,
                                                &aValue, false);
     return NS_OK;
   }
 
   // is it already the right kind of node, but with wrong attribute?
   if (aNode.IsHTMLElement(&aProperty)) {
+    if (NS_WARN_IF(!aAttribute)) {
+      return NS_ERROR_FAILURE;
+    }
     // Just set the attribute on it.
-    return SetAttribute(aNode.AsElement(), aAttribute, aValue);
+    return SetAttributeWithTransaction(*aNode.AsElement(), *aAttribute, aValue);
   }
 
   // ok, chuck it in its very own container
   RefPtr<Element> tmp = InsertContainerAbove(&aNode, &aProperty, aAttribute,
                                              &aValue);
   NS_ENSURE_STATE(tmp);
 
   return NS_OK;
@@ -721,38 +724,46 @@ HTMLEditor::RemoveStyleInside(nsIContent
         aNode.AsElement()->HasAttr(kNameSpaceID_None, nsGkAtoms::_class);
       if (aProperty && (hasStyleAttr || hasClassAttr)) {
         // aNode carries inline styles or a class attribute so we can't
         // just remove the element... We need to create above the element
         // a span that will carry those styles or class, then we can delete
         // the node.
         RefPtr<Element> spanNode =
           InsertContainerAbove(&aNode, nsGkAtoms::span);
-        NS_ENSURE_STATE(spanNode);
+        if (NS_WARN_IF(!spanNode)) {
+          return NS_ERROR_FAILURE;
+        }
         nsresult rv =
-          CloneAttribute(nsGkAtoms::style, spanNode, aNode.AsElement());
-        NS_ENSURE_SUCCESS(rv, rv);
-        rv =
-          CloneAttribute(nsGkAtoms::_class, spanNode, aNode.AsElement());
-        NS_ENSURE_SUCCESS(rv, rv);
+          CloneAttributeWithTransaction(*nsGkAtoms::style, *spanNode,
+                                        *aNode.AsElement());
+        if (NS_WARN_IF(NS_FAILED(rv))) {
+          return rv;
+        }
+        rv = CloneAttributeWithTransaction(*nsGkAtoms::_class, *spanNode,
+                                           *aNode.AsElement());
+        if (NS_WARN_IF(NS_FAILED(rv))) {
+          return rv;
+        }
       }
       nsresult rv = RemoveContainer(&aNode);
       NS_ENSURE_SUCCESS(rv, rv);
     } else if (aNode.IsElement()) {
       // otherwise we just want to eliminate the attribute
       if (aNode.AsElement()->HasAttr(kNameSpaceID_None, aAttribute)) {
         // if this matching attribute is the ONLY one on the node,
         // then remove the whole node.  Otherwise just nix the attribute.
         if (IsOnlyAttribute(aNode.AsElement(), aAttribute)) {
           nsresult rv = RemoveContainer(&aNode);
           if (NS_WARN_IF(NS_FAILED(rv))) {
             return rv;
           }
         } else {
-          nsresult rv = RemoveAttribute(aNode.AsElement(), aAttribute);
+          nsresult rv =
+            RemoveAttributeWithTransaction(*aNode.AsElement(), *aAttribute);
           if (NS_WARN_IF(NS_FAILED(rv))) {
             return rv;
           }
         }
       }
     }
   }
 
--- a/editor/libeditor/HTMLTableEditor.cpp
+++ b/editor/libeditor/HTMLTableEditor.cpp
@@ -150,33 +150,45 @@ HTMLEditor::InsertCell(nsIDOMElement* aD
   }
 
   // Don't let Rules System change the selection.
   AutoTransactionsConserveSelection dontChangeSelection(this);
   return InsertNodeWithTransaction(*newCell, pointToInsert);
 }
 
 nsresult
-HTMLEditor::SetColSpan(nsIDOMElement* aCell,
+HTMLEditor::SetColSpan(nsIDOMElement* aDOMCell,
                        int32_t aColSpan)
 {
-  NS_ENSURE_TRUE(aCell, NS_ERROR_NULL_POINTER);
+  if (NS_WARN_IF(!aDOMCell)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+  nsCOMPtr<Element> cell = do_QueryInterface(aDOMCell);
+  if (NS_WARN_IF(!cell)) {
+    return NS_ERROR_INVALID_ARG;
+  }
   nsAutoString newSpan;
   newSpan.AppendInt(aColSpan, 10);
-  return SetAttribute(aCell, NS_LITERAL_STRING("colspan"), newSpan);
+  return SetAttributeWithTransaction(*cell, *nsGkAtoms::colspan, newSpan);
 }
 
 nsresult
-HTMLEditor::SetRowSpan(nsIDOMElement* aCell,
+HTMLEditor::SetRowSpan(nsIDOMElement* aDOMCell,
                        int32_t aRowSpan)
 {
-  NS_ENSURE_TRUE(aCell, NS_ERROR_NULL_POINTER);
+  if (NS_WARN_IF(!aDOMCell)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+  nsCOMPtr<Element> cell = do_QueryInterface(aDOMCell);
+  if (NS_WARN_IF(!cell)) {
+    return NS_ERROR_INVALID_ARG;
+  }
   nsAutoString newSpan;
   newSpan.AppendInt(aRowSpan, 10);
-  return SetAttribute(aCell, NS_LITERAL_STRING("rowspan"), newSpan);
+  return SetAttributeWithTransaction(*cell, *nsGkAtoms::rowspan, newSpan);
 }
 
 NS_IMETHODIMP
 HTMLEditor::InsertTableCell(int32_t aNumber,
                             bool aAfter)
 {
   nsCOMPtr<nsIDOMElement> table;
   nsCOMPtr<nsIDOMElement> curCell;
@@ -1761,33 +1773,40 @@ HTMLEditor::SplitTableCell()
     }
     // Point to the new cell and repeat
     rowIndex++;
   }
   return NS_OK;
 }
 
 nsresult
-HTMLEditor::CopyCellBackgroundColor(nsIDOMElement* destCell,
-                                    nsIDOMElement* sourceCell)
+HTMLEditor::CopyCellBackgroundColor(nsIDOMElement* aDOMDestCell,
+                                    nsIDOMElement* aDOMSourceCell)
 {
-  NS_ENSURE_TRUE(destCell && sourceCell, NS_ERROR_NULL_POINTER);
-
-  // Copy backgournd color to new cell
-  NS_NAMED_LITERAL_STRING(bgcolor, "bgcolor");
+  if (NS_WARN_IF(!aDOMDestCell) || NS_WARN_IF(!aDOMSourceCell)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+  nsCOMPtr<Element> destCell = do_QueryInterface(aDOMDestCell);
+  if (NS_WARN_IF(!destCell)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
+  // Copy backgournd color to new cell.
   nsAutoString color;
   bool isSet;
-  nsresult rv = GetAttributeValue(sourceCell, bgcolor, color, &isSet);
+  nsresult rv =
+    GetAttributeValue(aDOMSourceCell, NS_LITERAL_STRING("bgcolor"),
+                      color, &isSet);
   if (NS_FAILED(rv)) {
     return rv;
   }
   if (!isSet) {
     return NS_OK;
   }
-  return SetAttribute(destCell, bgcolor, color);
+  return SetAttributeWithTransaction(*destCell, *nsGkAtoms::bgcolor, color);
 }
 
 nsresult
 HTMLEditor::SplitCellIntoColumns(nsIDOMElement* aTable,
                                  int32_t aRowIndex,
                                  int32_t aColIndex,
                                  int32_t aColSpanLeft,
                                  int32_t aColSpanRight,
--- a/editor/libeditor/TextEditRules.cpp
+++ b/editor/libeditor/TextEditRules.cpp
@@ -1675,18 +1675,19 @@ TextEditRules::CreateBRInternal(const Ed
   RefPtr<Element> brElement = textEditor->CreateBR(aPointToInsert);
   if (NS_WARN_IF(!brElement)) {
     return nullptr;
   }
 
   // give it special moz attr
   if (aCreateMozBR) {
     // XXX Why do we need to set this attribute with transaction?
-    nsresult rv = textEditor->SetAttribute(brElement, nsGkAtoms::type,
-                                           NS_LITERAL_STRING("_moz"));
+    nsresult rv =
+      textEditor->SetAttributeWithTransaction(*brElement, *nsGkAtoms::type,
+                                              NS_LITERAL_STRING("_moz"));
     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;
     }
   }
 
   return brElement.forget();
--- a/editor/libeditor/TextEditor.cpp
+++ b/editor/libeditor/TextEditor.cpp
@@ -308,20 +308,20 @@ TextEditor::UpdateMetaCharset(nsIDocumen
                         nsCaseInsensitiveStringComparator())) {
       continue;
     }
 
     // set attribute to <original prefix> charset=text/html
     RefPtr<Element> metaElement = metaNode->AsElement();
     MOZ_ASSERT(metaElement);
     nsresult rv =
-      EditorBase::SetAttribute(metaElement, nsGkAtoms::content,
-                               Substring(originalStart, start) +
-                                 charsetEquals +
-                                 NS_ConvertASCIItoUTF16(aCharacterSet));
+      SetAttributeWithTransaction(*metaElement, *nsGkAtoms::content,
+                                  Substring(originalStart, start) +
+                                    charsetEquals +
+                                    NS_ConvertASCIItoUTF16(aCharacterSet));
     return NS_SUCCEEDED(rv);
   }
   return false;
 }
 
 NS_IMETHODIMP
 TextEditor::InitRules()
 {
@@ -1999,20 +1999,26 @@ TextEditor::GetDOMEventTarget()
 
 
 nsresult
 TextEditor::SetAttributeOrEquivalent(Element* aElement,
                                      nsAtom* aAttribute,
                                      const nsAString& aValue,
                                      bool aSuppressTransaction)
 {
-  return EditorBase::SetAttribute(aElement, aAttribute, aValue);
+  if (NS_WARN_IF(!aElement) || NS_WARN_IF(!aAttribute)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+  return SetAttributeWithTransaction(*aElement, *aAttribute, aValue);
 }
 
 nsresult
 TextEditor::RemoveAttributeOrEquivalent(Element* aElement,
                                         nsAtom* aAttribute,
                                         bool aSuppressTransaction)
 {
-  return EditorBase::RemoveAttribute(aElement, aAttribute);
+  if (NS_WARN_IF(!aElement) || NS_WARN_IF(!aAttribute)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+  return RemoveAttributeWithTransaction(*aElement, *aAttribute);
 }
 
 } // namespace mozilla