Bug 1803044 - part 3: Make `EditorBase::RemoveAttributeWithTransactio…

…n` stop creating transaction if the element does not have the removing attribute r=m_kato Depends on D163997 Differential Revision: https://phabricator.services.mozilla.com/D163998
dothq · Dec 13, 2022 · 5255ec9 · 5255ec9
1 parent 512839a
commit 5255ec9
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 25 deletions.
diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp
@@ -1856,12 +1856,15 @@ NS_IMETHODIMP EditorBase::RemoveAttribute(Element* aElement,
 
 nsresult 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.
+  if (!aElement.HasAttr(&aAttribute)) {
+    return NS_OK;
+  }
   RefPtr<ChangeAttributeTransaction> transaction =
       ChangeAttributeTransaction::CreateToRemove(aElement, aAttribute);
   nsresult rv = DoTransactionInternal(transaction);
+  if (NS_WARN_IF(Destroyed())) {
+    return NS_ERROR_EDITOR_DESTROYED;
+  }
   NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
                        "EditorBase::DoTransactionInternal() failed");
   return rv;

diff --git a/editor/libeditor/EditorBase.h b/editor/libeditor/EditorBase.h
@@ -1776,7 +1776,7 @@ class EditorBase : public nsIEditor,
    * @param aElement        Element node which will lose aAttribute.
    * @param aAttribute      Attribute name to be removed from aElement.
    */
-  MOZ_CAN_RUN_SCRIPT nsresult
+  [[nodiscard]] MOZ_CAN_RUN_SCRIPT nsresult
   RemoveAttributeWithTransaction(Element& aElement, nsAtom& aAttribute);
 
   MOZ_CAN_RUN_SCRIPT virtual nsresult RemoveAttributeOrEquivalent(

diff --git a/editor/libeditor/HTMLEditSubActionHandler.cpp b/editor/libeditor/HTMLEditSubActionHandler.cpp
@@ -3659,9 +3659,6 @@ Result<EditActionResult, nsresult> HTMLEditor::ConvertContentAroundRangesToList(
       }
       nsresult rv = RemoveAttributeWithTransaction(MOZ_KnownLive(*element),
                                                    *nsGkAtoms::type);
-      if (NS_WARN_IF(Destroyed())) {
-        return Err(NS_ERROR_EDITOR_DESTROYED);
-      }
       if (NS_FAILED(rv)) {
         NS_WARNING(
             "EditorBase::RemoveAttributeWithTransaction(nsGkAtoms::type) "
@@ -7867,9 +7864,6 @@ Result<SplitNodeResult, nsresult> HTMLEditor::SplitParagraphWithTransaction(
   // unwrappedSplitDivOrPResult.
   nsresult rv = RemoveAttributeWithTransaction(
       MOZ_KnownLive(*rightDivOrParagraphElement), *nsGkAtoms::id);
-  if (NS_WARN_IF(Destroyed())) {
-    return Err(NS_ERROR_EDITOR_DESTROYED);
-  }
   if (NS_FAILED(rv)) {
     NS_WARNING(
         "EditorBase::RemoveAttributeWithTransaction(nsGkAtoms::id) failed");
@@ -10184,9 +10178,6 @@ Result<EditorDOMPoint, nsresult> HTMLEditor::RemoveAlignFromDescendants(
     if (HTMLEditUtils::SupportsAlignAttr(blockOrHRElement)) {
       nsresult rv =
           RemoveAttributeWithTransaction(blockOrHRElement, *nsGkAtoms::align);
-      if (NS_WARN_IF(Destroyed())) {
-        return Err(NS_ERROR_EDITOR_DESTROYED);
-      }
       if (NS_FAILED(rv)) {
         NS_WARNING(
             "EditorBase::RemoveAttributeWithTransaction(nsGkAtoms::align) "

diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp
@@ -3574,9 +3574,6 @@ nsresult HTMLEditor::SetHTMLBackgroundColorWithTransaction(
           // is stack only class and keeps grabbing it until it's destroyed.
           nsresult rv = RemoveAttributeWithTransaction(
               MOZ_KnownLive(cellElement), *nsGkAtoms::bgcolor);
-          if (NS_WARN_IF(Destroyed())) {
-            return NS_ERROR_EDITOR_DESTROYED;
-          }
           if (NS_FAILED(rv)) {
             NS_WARNING(
                 "EditorBase::RemoveAttributeWithTransaction(nsGkAtoms::bgcolor)"

diff --git a/editor/libeditor/HTMLEditorObjectResizer.cpp b/editor/libeditor/HTMLEditorObjectResizer.cpp
@@ -3,6 +3,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#include "ErrorList.h"
 #include "HTMLEditor.h"
 
 #include "CSSEditUtils.h"
@@ -1302,26 +1303,32 @@ nsresult HTMLEditor::SetFinalSizeWithTransaction(int32_t aX, int32_t aY) {
   if (IsCSSEnabled() || mResizedObjectIsAbsolutelyPositioned) {
     if (setWidth &&
         resizedElement->HasAttr(kNameSpaceID_None, nsGkAtoms::width)) {
-      DebugOnly<nsresult> rvIgnored =
+      nsresult rv =
           RemoveAttributeWithTransaction(*resizedElement, *nsGkAtoms::width);
-      if (NS_WARN_IF(Destroyed())) {
+      if (MOZ_UNLIKELY(rv == NS_ERROR_EDITOR_DESTROYED)) {
+        NS_WARNING(
+            "EditorBase::RemoveAttributeWithTransaction(nsGkAtoms::width) "
+            "failed");
         return NS_ERROR_EDITOR_DESTROYED;
       }
       NS_WARNING_ASSERTION(
-          NS_SUCCEEDED(rvIgnored),
+          NS_SUCCEEDED(rv),
           "EditorBase::RemoveAttributeWithTransaction(nsGkAtoms::width) "
           "failed, but ignored");
     }
 
     if (setHeight &&
         resizedElement->HasAttr(kNameSpaceID_None, nsGkAtoms::height)) {
-      DebugOnly<nsresult> rvIgnored =
+      nsresult rv =
           RemoveAttributeWithTransaction(*resizedElement, *nsGkAtoms::height);
-      if (NS_WARN_IF(Destroyed())) {
+      if (MOZ_UNLIKELY(rv == NS_ERROR_EDITOR_DESTROYED)) {
+        NS_WARNING(
+            "EditorBase::RemoveAttributeWithTransaction(nsGkAtoms::height) "
+            "failed");
         return NS_ERROR_EDITOR_DESTROYED;
       }
       NS_WARNING_ASSERTION(
-          NS_SUCCEEDED(rvIgnored),
+          NS_SUCCEEDED(rv),
           "EditorBase::RemoveAttributeWithTransaction(nsGkAtoms::height) "
           "failed, but ignored");
     }

diff --git a/editor/libeditor/HTMLStyleEditor.cpp b/editor/libeditor/HTMLStyleEditor.cpp
@@ -1695,9 +1695,6 @@ Result<EditorDOMPoint, nsresult> HTMLEditor::RemoveStyleInside(
       } else {
         nsresult rv = RemoveAttributeWithTransaction(
             aElement, *aStyleToRemove.mAttribute);
-        if (NS_WARN_IF(Destroyed())) {
-          return Err(NS_ERROR_EDITOR_DESTROYED);
-        }
         if (NS_FAILED(rv)) {
           NS_WARNING("EditorBase::RemoveAttributeWithTransaction() failed");
           return Err(rv);