Bug 1660378 - part 3: Make `CSSEditUtils::RemoveCSSEquivalentToHTMLStyle()` take `nsStyledElement&` instead of `Element*` r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 26 Aug 2020 04:48:19 +0000
changeset 546251 ac6b4ebdf31688981f48b201eb933a5dd3dfcfeb
parent 546250 ee82ab27ad02e47fac66d0e558ef140f8236e226
child 546252 c4e47e9e0e38f80d2254014adfbb19c553c69afe
push id124997
push usermasayuki@d-toybox.com
push dateWed, 26 Aug 2020 06:09:10 +0000
treeherderautoland@d1a42f86dd61 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1660378
milestone82.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 1660378 - part 3: Make `CSSEditUtils::RemoveCSSEquivalentToHTMLStyle()` take `nsStyledElement&` instead of `Element*` r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D87984
editor/libeditor/CSSEditUtils.cpp
editor/libeditor/CSSEditUtils.h
editor/libeditor/HTMLEditor.cpp
editor/libeditor/HTMLStyleEditor.cpp
--- a/editor/libeditor/CSSEditUtils.cpp
+++ b/editor/libeditor/CSSEditUtils.cpp
@@ -850,49 +850,41 @@ int32_t CSSEditUtils::SetCSSEquivalentTo
       return 0;
     }
   }
   return count;
 }
 
 // Remove from aNode the CSS inline style equivalent to
 // HTMLProperty/aAttribute/aValue for the node
-nsresult CSSEditUtils::RemoveCSSEquivalentToHTMLStyle(
-    Element* aElement, nsAtom* aHTMLProperty, nsAtom* aAttribute,
+nsresult CSSEditUtils::RemoveCSSEquivalentToHTMLStyleInternal(
+    nsStyledElement& aStyledElement, nsAtom* aHTMLProperty, nsAtom* aAttribute,
     const nsAString* aValue, bool aSuppressTransaction) {
-  if (NS_WARN_IF(!aElement)) {
-    return NS_OK;
-  }
-
-  if (!IsCSSEditableProperty(aElement, aHTMLProperty, aAttribute)) {
+  if (!IsCSSEditableProperty(&aStyledElement, aHTMLProperty, aAttribute)) {
     return NS_OK;
   }
 
   // we can apply the styles only if the node is an element and if we have
   // an equivalence for the requested HTML style in this implementation
 
   // Find the CSS equivalence to the HTML style
   nsTArray<nsStaticAtom*> cssPropertyArray;
   nsTArray<nsString> cssValueArray;
-  GenerateCSSDeclarationsFromHTMLStyle(*aElement, aHTMLProperty, aAttribute,
-                                       aValue, cssPropertyArray, cssValueArray,
-                                       true);
+  GenerateCSSDeclarationsFromHTMLStyle(aStyledElement, aHTMLProperty,
+                                       aAttribute, aValue, cssPropertyArray,
+                                       cssValueArray, true);
 
   // remove the individual CSS inline styles
   const size_t count = cssPropertyArray.Length();
   if (!count) {
     return NS_OK;
   }
-  nsCOMPtr<nsStyledElement> styledElement = do_QueryInterface(aElement);
-  if (NS_WARN_IF(!styledElement)) {
-    return NS_ERROR_FAILURE;
-  }
   for (size_t index = 0; index < count; index++) {
     nsresult rv = RemoveCSSPropertyInternal(
-        *styledElement, MOZ_KnownLive(*cssPropertyArray[index]),
+        aStyledElement, MOZ_KnownLive(*cssPropertyArray[index]),
         cssValueArray[index], aSuppressTransaction);
     if (NS_FAILED(rv)) {
       NS_WARNING("CSSEditUtils::RemoveCSSPropertyWithoutTransaction() failed");
       return rv;
     }
   }
   return NS_OK;
 }
--- a/editor/libeditor/CSSEditUtils.h
+++ b/editor/libeditor/CSSEditUtils.h
@@ -258,27 +258,37 @@ class CSSEditUtils final {
    */
   MOZ_CAN_RUN_SCRIPT int32_t SetCSSEquivalentToHTMLStyle(
       dom::Element* aElement, nsAtom* aProperty, nsAtom* aAttribute,
       const nsAString* aValue, bool aSuppressTransaction);
 
   /**
    * Removes from the node the CSS inline styles equivalent to the HTML style.
    *
-   * @param aElement       [IN] A DOM Element (must not be null).
+   * @param aStyledElement [IN] A DOM Element (must not be null).
    * @param aHTMLProperty  [IN] An atom containing an HTML property.
    * @param aAttribute     [IN] An atom to an attribute name or nullptr if
    *                            irrelevant.
    * @param aValue         [IN] The attribute value.
-   * @param aSuppressTransaction [IN] A boolean indicating, when true,
-   *                                  that no transaction should be recorded.
    */
-  [[nodiscard]] MOZ_CAN_RUN_SCRIPT nsresult RemoveCSSEquivalentToHTMLStyle(
-      dom::Element* aElement, nsAtom* aHTMLProperty, nsAtom* aAttribute,
-      const nsAString* aValue, bool aSuppressTransaction);
+  [[nodiscard]] MOZ_CAN_RUN_SCRIPT nsresult
+  RemoveCSSEquivalentToHTMLStyleWithTransaction(nsStyledElement& aStyledElement,
+                                                nsAtom* aHTMLProperty,
+                                                nsAtom* aAttribute,
+                                                const nsAString* aValue) {
+    return RemoveCSSEquivalentToHTMLStyleInternal(aStyledElement, aHTMLProperty,
+                                                  aAttribute, aValue, false);
+  }
+  [[nodiscard]] MOZ_CAN_RUN_SCRIPT nsresult
+  RemoveCSSEquivalentToHTMLStyleWithoutTransaction(
+      nsStyledElement& aStyledElement, nsAtom* aHTMLProperty,
+      nsAtom* aAttribute, const nsAString* aValue) {
+    return RemoveCSSEquivalentToHTMLStyleInternal(aStyledElement, aHTMLProperty,
+                                                  aAttribute, aValue, true);
+  }
 
   /**
    * Parses a "xxxx.xxxxxuuu" string where x is a digit and u an alpha char.
    *
    * @param aString        [IN] Input string to parse.
    * @param aValue         [OUT] Numeric part.
    * @param aUnit          [OUT] Unit part.
    */
@@ -404,16 +414,22 @@ class CSSEditUtils final {
       nsAString& aValue, StyleType aStyleType);
   MOZ_CAN_RUN_SCRIPT static bool HaveCSSEquivalentStylesInternal(
       nsIContent& aContent, nsAtom* aHTMLProperty, nsAtom* aAttribute,
       StyleType aStyleType);
 
   [[nodiscard]] MOZ_CAN_RUN_SCRIPT nsresult RemoveCSSPropertyInternal(
       nsStyledElement& aStyledElement, nsAtom& aProperty,
       const nsAString& aPropertyValue, bool aSuppressTxn = false);
+  [[nodiscard]] MOZ_CAN_RUN_SCRIPT nsresult
+  RemoveCSSEquivalentToHTMLStyleInternal(nsStyledElement& aStyledElement,
+                                         nsAtom* aHTMLProperty,
+                                         nsAtom* aAttribute,
+                                         const nsAString* aValue,
+                                         bool aSuppressTransaction);
 
  private:
   HTMLEditor* mHTMLEditor;
   bool mIsCSSPrefChecked;
 };
 
 #define NS_EDITOR_INDENT_INCREMENT_IN 0.4134f
 #define NS_EDITOR_INDENT_INCREMENT_CM 1.05f
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -53,16 +53,17 @@
 #include "nsIFrame.h"
 #include "nsIPrincipal.h"
 #include "nsISelectionController.h"
 #include "nsIURI.h"
 #include "nsIWidget.h"
 #include "nsNetUtil.h"
 #include "nsPresContext.h"
 #include "nsPIDOMWindow.h"
+#include "nsStyledElement.h"
 #include "nsTextFragment.h"
 #include "nsUnicharUtils.h"
 
 namespace mozilla {
 
 using namespace dom;
 using namespace widget;
 
@@ -5174,27 +5175,36 @@ nsresult HTMLEditor::SetAttributeOrEquiv
   MOZ_ASSERT(aElement);
   MOZ_ASSERT(aAttribute);
 
   nsAutoScriptBlocker scriptBlocker;
 
   if (!IsCSSEnabled() || !mCSSEditUtils) {
     // we are not in an HTML+CSS editor; let's set the attribute the HTML way
     if (mCSSEditUtils) {
-      nsresult rv = mCSSEditUtils->RemoveCSSEquivalentToHTMLStyle(
-          aElement, nullptr, aAttribute, nullptr, aSuppressTransaction);
-      if (rv == NS_ERROR_EDITOR_DESTROYED) {
-        NS_WARNING(
-            "CSSEditUtils::RemoveCSSEquivalentToHTMLStyle() destroyed the "
-            "editor");
-        return NS_ERROR_EDITOR_DESTROYED;
+      if (nsCOMPtr<nsStyledElement> styledElement =
+              do_QueryInterface(aElement)) {
+        nsresult rv =
+            aSuppressTransaction
+                ? mCSSEditUtils
+                      ->RemoveCSSEquivalentToHTMLStyleWithoutTransaction(
+                          *styledElement, nullptr, aAttribute, nullptr)
+                : mCSSEditUtils->RemoveCSSEquivalentToHTMLStyleWithTransaction(
+                      *styledElement, nullptr, aAttribute, nullptr);
+        if (rv == NS_ERROR_EDITOR_DESTROYED) {
+          NS_WARNING(
+              "CSSEditUtils::RemoveCSSEquivalentToHTMLStyle*Transaction() "
+              "destroyed the editor");
+          return NS_ERROR_EDITOR_DESTROYED;
+        }
+        NS_WARNING_ASSERTION(
+            NS_SUCCEEDED(rv),
+            "CSSEditUtils::RemoveCSSEquivalentToHTMLStyle*Transaction() "
+            "failed, but ignored");
       }
-      NS_WARNING_ASSERTION(
-          NS_SUCCEEDED(rv),
-          "CSSEditUtils::RemoveCSSEquivalentToHTMLStyle() failed, but ignored");
     }
     if (aSuppressTransaction) {
       nsresult rv =
           aElement->SetAttr(kNameSpaceID_None, aAttribute, aValue, true);
       NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Element::SetAttr() failed");
       return rv;
     }
     nsresult rv = SetAttributeWithTransaction(*aElement, *aAttribute, aValue);
@@ -5264,21 +5274,33 @@ nsresult HTMLEditor::SetAttributeOrEquiv
 }
 
 nsresult HTMLEditor::RemoveAttributeOrEquivalent(Element* aElement,
                                                  nsAtom* aAttribute,
                                                  bool aSuppressTransaction) {
   MOZ_ASSERT(aElement);
   MOZ_ASSERT(aAttribute);
 
-  if (IsCSSEnabled() && mCSSEditUtils) {
-    nsresult rv = mCSSEditUtils->RemoveCSSEquivalentToHTMLStyle(
-        aElement, nullptr, aAttribute, nullptr, aSuppressTransaction);
+  if (IsCSSEnabled() && mCSSEditUtils &&
+      CSSEditUtils::IsCSSEditableProperty(aElement, nullptr, aAttribute)) {
+    // XXX It might be keep handling attribute even if aElement is not
+    //     an nsStyledElement instance.
+    nsCOMPtr<nsStyledElement> styledElement = do_QueryInterface(aElement);
+    if (NS_WARN_IF(!styledElement)) {
+      return NS_ERROR_INVALID_ARG;
+    }
+    nsresult rv =
+        aSuppressTransaction
+            ? mCSSEditUtils->RemoveCSSEquivalentToHTMLStyleWithoutTransaction(
+                  *styledElement, nullptr, aAttribute, nullptr)
+            : mCSSEditUtils->RemoveCSSEquivalentToHTMLStyleWithTransaction(
+                  *styledElement, nullptr, aAttribute, nullptr);
     if (NS_FAILED(rv)) {
-      NS_WARNING("CSSEditUtils::RemoveCSSEquivalentToHTMLStyle() failed");
+      NS_WARNING(
+          "CSSEditUtils::RemoveCSSEquivalentToHTMLStyle*Transaction() failed");
       return rv;
     }
   }
 
   if (!aElement->HasAttr(kNameSpaceID_None, aAttribute)) {
     return NS_OK;
   }
 
--- a/editor/libeditor/HTMLStyleEditor.cpp
+++ b/editor/libeditor/HTMLStyleEditor.cpp
@@ -1133,28 +1133,33 @@ nsresult HTMLEditor::RemoveStyleInside(E
   }
 
   // Then, remove CSS style if specified.
   // XXX aElement may have already been removed from the DOM tree.  Why
   //     do we keep handling aElement here??
   if (CSSEditUtils::IsCSSEditableProperty(&aElement, aProperty, aAttribute) &&
       CSSEditUtils::HaveSpecifiedCSSEquivalentStyles(aElement, aProperty,
                                                      aAttribute)) {
-    // If aElement has CSS declaration of the given style, remove it.
-    nsresult rv = mCSSEditUtils->RemoveCSSEquivalentToHTMLStyle(
-        &aElement, aProperty, aAttribute, nullptr, false);
-    if (rv == NS_ERROR_EDITOR_DESTROYED) {
-      NS_WARNING(
-          "CSSEditUtils::RemoveCSSEquivalentToHTMLStyle() destroyed the "
-          "editor");
-      return NS_ERROR_EDITOR_DESTROYED;
+    if (nsCOMPtr<nsStyledElement> styledElement =
+            do_QueryInterface(&aElement)) {
+      // If aElement has CSS declaration of the given style, remove it.
+      nsresult rv =
+          mCSSEditUtils->RemoveCSSEquivalentToHTMLStyleWithTransaction(
+              *styledElement, aProperty, aAttribute, nullptr);
+      if (rv == NS_ERROR_EDITOR_DESTROYED) {
+        NS_WARNING(
+            "CSSEditUtils::RemoveCSSEquivalentToHTMLStyleWithTransaction() "
+            "destroyed the editor");
+        return NS_ERROR_EDITOR_DESTROYED;
+      }
+      NS_WARNING_ASSERTION(
+          NS_SUCCEEDED(rv),
+          "CSSEditUtils::RemoveCSSEquivalentToHTMLStyleWithTransaction() "
+          "failed, but ignored");
     }
-    NS_WARNING_ASSERTION(
-        NS_SUCCEEDED(rv),
-        "CSSEditUtils::RemoveCSSEquivalentToHTMLStyle() failed, but ignored");
     // Additionally, remove aElement itself if it's a `<span>` or `<font>`
     // and it does not have non-empty `style`, `id` nor `class` attribute.
     if (aElement.IsAnyOfHTMLElements(nsGkAtoms::span, nsGkAtoms::font) &&
         !HTMLEditor::HasStyleOrIdOrClassAttribute(aElement)) {
       DebugOnly<nsresult> rvIgnored = RemoveContainerWithTransaction(aElement);
       if (NS_WARN_IF(Destroyed())) {
         return NS_ERROR_EDITOR_DESTROYED;
       }