Issue #1621 - Part 1: CSSEditUtils should use atom for CSS property if possible.

There is a lot of string compare when using CSS property name. We should use nsGkAtoms instead.

Ref: Bug 1323138

5 files changed, 224 insertions, 188 deletions

diff --git a/editor/libeditor/CSSEditUtils.cpp b/editor/libeditor/CSSEditUtils.cpp
index dd15a8730..e15bc278f 100644
--- a/editor/libeditor/CSSEditUtils.cpp
+++ b/editor/libeditor/CSSEditUtils.cpp
@@ -323,6 +323,15 @@ CSSEditUtils::IsCSSEditableProperty(nsINode* aNode,
                                     nsIAtom* aProperty,
                                     const nsAString* aAttribute)
 {
+  nsCOMPtr<nsIAtom> attribute = aAttribute ? NS_Atomize(*aAttribute) : nullptr;
+  return IsCSSEditableProperty(aNode, aProperty, attribute);
+}
+
+bool
+CSSEditUtils::IsCSSEditableProperty(nsINode* aNode,
+                                    nsIAtom* aProperty,
+                                    nsIAtom* aAttribute)
+{
   MOZ_ASSERT(aNode);
 
   nsINode* node = aNode;
@@ -339,13 +348,12 @@ CSSEditUtils::IsCSSEditableProperty(nsINode* aNode,
       nsGkAtoms::u == aProperty ||
       nsGkAtoms::strike == aProperty ||
       (nsGkAtoms::font == aProperty && aAttribute &&
-       (aAttribute->EqualsLiteral("color") ||
-        aAttribute->EqualsLiteral("face")))) {
+       (aAttribute == nsGkAtoms::color || aAttribute == nsGkAtoms::face))) {
     return true;
   }
 
   // ALIGN attribute on elements supporting it
-  if (aAttribute && (aAttribute->EqualsLiteral("align")) &&
+  if (aAttribute == nsGkAtoms::align &&
       node->IsAnyOfHTMLElements(nsGkAtoms::div,
                                 nsGkAtoms::p,
                                 nsGkAtoms::h1,
@@ -368,7 +376,7 @@ CSSEditUtils::IsCSSEditableProperty(nsINode* aNode,
     return true;
   }
 
-  if (aAttribute && (aAttribute->EqualsLiteral("valign")) &&
+  if (aAttribute == nsGkAtoms::valign &&
       node->IsAnyOfHTMLElements(nsGkAtoms::col,
                                 nsGkAtoms::colgroup,
                                 nsGkAtoms::tbody,
@@ -381,59 +389,52 @@ CSSEditUtils::IsCSSEditableProperty(nsINode* aNode,
   }
 
   // attributes TEXT, BACKGROUND and BGCOLOR on BODY
-  if (aAttribute && node->IsHTMLElement(nsGkAtoms::body) &&
-      (aAttribute->EqualsLiteral("text")
-       || aAttribute->EqualsLiteral("background")
-       || aAttribute->EqualsLiteral("bgcolor"))) {
+  if (node->IsHTMLElement(nsGkAtoms::body) &&
+      (aAttribute == nsGkAtoms::text || aAttribute == nsGkAtoms::background ||
+       aAttribute == nsGkAtoms::bgcolor)) {
     return true;
   }
 
   // attribute BGCOLOR on other elements
-  if (aAttribute && aAttribute->EqualsLiteral("bgcolor")) {
+  if (aAttribute == nsGkAtoms::bgcolor) {
     return true;
   }
 
   // attributes HEIGHT, WIDTH and NOWRAP on TD and TH
-  if (aAttribute &&
-      node->IsAnyOfHTMLElements(nsGkAtoms::td, nsGkAtoms::th) &&
-      (aAttribute->EqualsLiteral("height")
-       || aAttribute->EqualsLiteral("width")
-       || aAttribute->EqualsLiteral("nowrap"))) {
+  if (node->IsAnyOfHTMLElements(nsGkAtoms::td, nsGkAtoms::th) &&
+      (aAttribute == nsGkAtoms::height || aAttribute == nsGkAtoms::width ||
+       aAttribute == nsGkAtoms::nowrap)) {
     return true;
   }
 
   // attributes HEIGHT and WIDTH on TABLE
-  if (aAttribute && node->IsHTMLElement(nsGkAtoms::table) &&
-      (aAttribute->EqualsLiteral("height")
-       || aAttribute->EqualsLiteral("width"))) {
+  if (node->IsHTMLElement(nsGkAtoms::table) &&
+      (aAttribute == nsGkAtoms::height || aAttribute == nsGkAtoms::width)) {
     return true;
   }
 
   // attributes SIZE and WIDTH on HR
-  if (aAttribute && node->IsHTMLElement(nsGkAtoms::hr) &&
-      (aAttribute->EqualsLiteral("size")
-       || aAttribute->EqualsLiteral("width"))) {
+  if (node->IsHTMLElement(nsGkAtoms::hr) &&
+      (aAttribute == nsGkAtoms::size || aAttribute == nsGkAtoms::width)) {
     return true;
   }
 
   // attribute TYPE on OL UL LI
-  if (aAttribute &&
-      node->IsAnyOfHTMLElements(nsGkAtoms::ol, nsGkAtoms::ul,
+  if (node->IsAnyOfHTMLElements(nsGkAtoms::ol, nsGkAtoms::ul,
                                 nsGkAtoms::li) &&
-      aAttribute->EqualsLiteral("type")) {
+      aAttribute == nsGkAtoms::type) {
     return true;
   }
 
-  if (aAttribute && node->IsHTMLElement(nsGkAtoms::img) &&
-      (aAttribute->EqualsLiteral("border")
-       || aAttribute->EqualsLiteral("width")
-       || aAttribute->EqualsLiteral("height"))) {
+  if (node->IsHTMLElement(nsGkAtoms::img) &&
+      (aAttribute == nsGkAtoms::border || aAttribute == nsGkAtoms::width ||
+       aAttribute == nsGkAtoms::height)) {
     return true;
   }
 
   // other elements that we can align using CSS even if they
   // can't carry the html ALIGN attribute
-  if (aAttribute && aAttribute->EqualsLiteral("align") &&
+  if (aAttribute == nsGkAtoms::align &&
       node->IsAnyOfHTMLElements(nsGkAtoms::ul,
                                 nsGkAtoms::ol,
                                 nsGkAtoms::dl,
@@ -818,7 +819,7 @@ void
 CSSEditUtils::GenerateCSSDeclarationsFromHTMLStyle(
                 Element* aElement,
                 nsIAtom* aHTMLProperty,
-                const nsAString* aAttribute,
+                nsIAtom* aAttribute,
                 const nsAString* aValue,
                 nsTArray<nsIAtom*>& cssPropertyArray,
                 nsTArray<nsString>& cssValueArray,
@@ -838,21 +839,20 @@ CSSEditUtils::GenerateCSSDeclarationsFromHTMLStyle(
   } else if (nsGkAtoms::tt == aHTMLProperty) {
     equivTable = ttEquivTable;
   } else if (aAttribute) {
-    if (nsGkAtoms::font == aHTMLProperty &&
-        aAttribute->EqualsLiteral("color")) {
+    if (nsGkAtoms::font == aHTMLProperty && aAttribute == nsGkAtoms::color) {
       equivTable = fontColorEquivTable;
     } else if (nsGkAtoms::font == aHTMLProperty &&
-               aAttribute->EqualsLiteral("face")) {
+               aAttribute == nsGkAtoms::face) {
       equivTable = fontFaceEquivTable;
-    } else if (aAttribute->EqualsLiteral("bgcolor")) {
+    } else if (aAttribute == nsGkAtoms::bgcolor) {
       equivTable = bgcolorEquivTable;
-    } else if (aAttribute->EqualsLiteral("background")) {
+    } else if (aAttribute == nsGkAtoms::background) {
       equivTable = backgroundImageEquivTable;
-    } else if (aAttribute->EqualsLiteral("text")) {
+    } else if (aAttribute == nsGkAtoms::text) {
       equivTable = textColorEquivTable;
-    } else if (aAttribute->EqualsLiteral("border")) {
+    } else if (aAttribute == nsGkAtoms::border) {
       equivTable = borderEquivTable;
-    } else if (aAttribute->EqualsLiteral("align")) {
+    } else if (aAttribute == nsGkAtoms::align) {
       if (aElement->IsHTMLElement(nsGkAtoms::table)) {
         equivTable = tableAlignEquivTable;
       } else if (aElement->IsHTMLElement(nsGkAtoms::hr)) {
@@ -863,17 +863,17 @@ CSSEditUtils::GenerateCSSDeclarationsFromHTMLStyle(
       } else {
         equivTable = textAlignEquivTable;
       }
-    } else if (aAttribute->EqualsLiteral("valign")) {
+    } else if (aAttribute == nsGkAtoms::valign) {
       equivTable = verticalAlignEquivTable;
-    } else if (aAttribute->EqualsLiteral("nowrap")) {
+    } else if (aAttribute == nsGkAtoms::nowrap) {
       equivTable = nowrapEquivTable;
-    } else if (aAttribute->EqualsLiteral("width")) {
+    } else if (aAttribute == nsGkAtoms::width) {
       equivTable = widthEquivTable;
-    } else if (aAttribute->EqualsLiteral("height") ||
+    } else if (aAttribute == nsGkAtoms::height ||
                (aElement->IsHTMLElement(nsGkAtoms::hr) &&
-                aAttribute->EqualsLiteral("size"))) {
+                aAttribute == nsGkAtoms::size)) {
       equivTable = heightEquivTable;
-    } else if (aAttribute->EqualsLiteral("type") &&
+    } else if (aAttribute == nsGkAtoms::type &&
                aElement->IsAnyOfHTMLElements(nsGkAtoms::ol,
                                              nsGkAtoms::ul,
                                              nsGkAtoms::li)) {
@@ -890,40 +890,46 @@ CSSEditUtils::GenerateCSSDeclarationsFromHTMLStyle(
 // aValue for the node, and return in aCount the number of CSS properties set
 // by the call.  The Element version returns aCount instead.
 int32_t
-CSSEditUtils::SetCSSEquivalentToHTMLStyle(Element* aElement,
+CSSEditUtils::SetCSSEquivalentToHTMLStyle(nsIDOMNode* aNode,
                                           nsIAtom* aProperty,
                                           const nsAString* aAttribute,
                                           const nsAString* aValue,
                                           bool aSuppressTransaction)
 {
-  MOZ_ASSERT(aElement && aProperty);
   MOZ_ASSERT_IF(aAttribute, aValue);
-  int32_t count;
   // This can only fail if SetCSSProperty fails, which should only happen if
   // something is pretty badly wrong.  In this case we assert so that hopefully
   // someone will notice, but there's nothing more sensible to do than just
   // return the count and carry on.
-  nsresult rv = SetCSSEquivalentToHTMLStyle(aElement->AsDOMNode(),
-                                            aProperty, aAttribute,
-                                            aValue, &count,
-                                            aSuppressTransaction);
-  NS_ASSERTION(NS_SUCCEEDED(rv), "SetCSSEquivalentToHTMLStyle failed");
-  NS_ENSURE_SUCCESS(rv, count);
-  return count;
+  nsCOMPtr<Element> element = do_QueryInterface(aNode);
+  return SetCSSEquivalentToHTMLStyle(element,
+                                     aProperty, aAttribute,
+                                     aValue, aSuppressTransaction);
 }
 
-nsresult
-CSSEditUtils::SetCSSEquivalentToHTMLStyle(nsIDOMNode* aNode,
+int32_t
+CSSEditUtils::SetCSSEquivalentToHTMLStyle(Element* aElement,
                                           nsIAtom* aHTMLProperty,
                                           const nsAString* aAttribute,
                                           const nsAString* aValue,
-                                          int32_t* aCount,
                                           bool aSuppressTransaction)
 {
-  nsCOMPtr<Element> element = do_QueryInterface(aNode);
-  *aCount = 0;
-  if (!element || !IsCSSEditableProperty(element, aHTMLProperty, aAttribute)) {
-    return NS_OK;
+  nsCOMPtr<nsIAtom> attribute = aAttribute ? NS_Atomize(*aAttribute) : nullptr;
+  return SetCSSEquivalentToHTMLStyle(aElement, aHTMLProperty, attribute,
+                                     aValue, aSuppressTransaction);
+}
+
+int32_t
+CSSEditUtils::SetCSSEquivalentToHTMLStyle(Element* aElement,
+                                          nsIAtom* aHTMLProperty,
+                                          nsIAtom* aAttribute,
+                                          const nsAString* aValue,
+                                          bool aSuppressTransaction)
+{
+  MOZ_ASSERT(aElement);
+
+  if (!IsCSSEditableProperty(aElement, aHTMLProperty, aAttribute)) {
+    return 0;
   }
 
   // we can apply the styles only if the node is an element and if we have
@@ -932,18 +938,20 @@ CSSEditUtils::SetCSSEquivalentToHTMLStyle(nsIDOMNode* aNode,
   // Find the CSS equivalence to the HTML style
   nsTArray<nsIAtom*> cssPropertyArray;
   nsTArray<nsString> cssValueArray;
-  GenerateCSSDeclarationsFromHTMLStyle(element, aHTMLProperty, aAttribute,
+  GenerateCSSDeclarationsFromHTMLStyle(aElement, aHTMLProperty, aAttribute,
                                        aValue, cssPropertyArray, cssValueArray,
                                        false);
 
   // set the individual CSS inline styles
-  *aCount = cssPropertyArray.Length();
-  for (int32_t index = 0; index < *aCount; index++) {
-    nsresult rv = SetCSSProperty(*element, *cssPropertyArray[index],
+  size_t count = cssPropertyArray.Length();
+  for (size_t index = 0; index < count; index++) {
+    nsresult rv = SetCSSProperty(*aElement, *cssPropertyArray[index],
                                  cssValueArray[index], aSuppressTransaction);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return 0;
+    }
   }
-  return NS_OK;
+  return count;
 }
 
 // Remove from aNode the CSS inline style equivalent to HTMLProperty/aAttribute/aValue for the node
@@ -955,20 +963,22 @@ CSSEditUtils::RemoveCSSEquivalentToHTMLStyle(nsIDOMNode* aNode,
                                              bool aSuppressTransaction)
 {
   nsCOMPtr<Element> element = do_QueryInterface(aNode);
-  NS_ENSURE_TRUE(element, NS_OK);
+  nsCOMPtr<nsIAtom> attribute = aAttribute ? NS_Atomize(*aAttribute) : nullptr;
 
-  return RemoveCSSEquivalentToHTMLStyle(element, aHTMLProperty, aAttribute,
+  return RemoveCSSEquivalentToHTMLStyle(element, aHTMLProperty, attribute,
                                         aValue, aSuppressTransaction);
 }
 
 nsresult
 CSSEditUtils::RemoveCSSEquivalentToHTMLStyle(Element* aElement,
                                              nsIAtom* aHTMLProperty,
-                                             const nsAString* aAttribute,
+                                             nsIAtom* aAttribute,
                                              const nsAString* aValue,
                                              bool aSuppressTransaction)
 {
-  MOZ_ASSERT(aElement);
+  if (NS_WARN_IF(!aElement)) {
+    return NS_OK;
+  }
 
   if (!IsCSSEditableProperty(aElement, aHTMLProperty, aAttribute)) {
     return NS_OK;
@@ -1003,7 +1013,7 @@ CSSEditUtils::RemoveCSSEquivalentToHTMLStyle(Element* aElement,
 nsresult
 CSSEditUtils::GetCSSEquivalentToHTMLInlineStyleSet(nsINode* aNode,
                                                    nsIAtom* aHTMLProperty,
-                                                   const nsAString* aAttribute,
+                                                   nsIAtom* aAttribute,
                                                    nsAString& aValueString,
                                                    StyleType aStyleType)
 {
@@ -1020,7 +1030,8 @@ CSSEditUtils::GetCSSEquivalentToHTMLInlineStyleSet(nsINode* aNode,
   nsTArray<nsString> cssValueArray;
   // get the CSS equivalence with last param true indicating we want only the
   // "gettable" properties
-  GenerateCSSDeclarationsFromHTMLStyle(theElement, aHTMLProperty, aAttribute, nullptr,
+  GenerateCSSDeclarationsFromHTMLStyle(theElement, aHTMLProperty, aAttribute,
+                                       nullptr,
                                        cssPropertyArray, cssValueArray, true);
   int32_t count = cssPropertyArray.Length();
   for (int32_t index = 0; index < count; index++) {
@@ -1066,48 +1077,58 @@ CSSEditUtils::IsCSSEquivalentToHTMLInlineStyleSet(nsINode* aNode,
                                                   StyleType aStyleType)
 {
   MOZ_ASSERT(aNode && aProperty);
-  bool isSet;
-  nsresult rv = IsCSSEquivalentToHTMLInlineStyleSet(aNode->AsDOMNode(),
-                                                    aProperty, aAttribute,
-                                                    isSet, aValue, aStyleType);
-  NS_ENSURE_SUCCESS(rv, false);
-  return isSet;
+  nsCOMPtr<nsIAtom> attribute = aAttribute ? NS_Atomize(*aAttribute) : nullptr;
+  return IsCSSEquivalentToHTMLInlineStyleSet(aNode,
+                                             aProperty, attribute,
+                                             aValue, aStyleType);
 }
 
-nsresult
+bool
+CSSEditUtils::IsCSSEquivalentToHTMLInlineStyleSet(nsIDOMNode* aNode,
+                                                  nsIAtom* aProperty,
+                                                  const nsAString* aAttribute,
+                                                  nsAString& aValue,
+                                                  StyleType aStyleType)
+{
+  MOZ_ASSERT(aNode && aProperty);
+  nsCOMPtr<nsINode> node = do_QueryInterface(aNode);
+  nsCOMPtr<nsIAtom> attribute = aAttribute ? NS_Atomize(*aAttribute) : nullptr;
+  return IsCSSEquivalentToHTMLInlineStyleSet(node, aProperty, attribute,
+                                             aValue, aStyleType);
+}
+
+bool
 CSSEditUtils::IsCSSEquivalentToHTMLInlineStyleSet(
-                nsIDOMNode* aNode,
+                nsINode* aNode,
                 nsIAtom* aHTMLProperty,
-                const nsAString* aHTMLAttribute,
-                bool& aIsSet,
+                nsIAtom* aHTMLAttribute,
                 nsAString& valueString,
                 StyleType aStyleType)
 {
-  NS_ENSURE_TRUE(aNode, NS_ERROR_NULL_POINTER);
+  NS_ENSURE_TRUE(aNode, false);
 
   nsAutoString htmlValueString(valueString);
-  aIsSet = false;
-  nsCOMPtr<nsINode> node = do_QueryInterface(aNode);
+  bool isSet = false;
   do {
     valueString.Assign(htmlValueString);
     // get the value of the CSS equivalent styles
     nsresult rv =
-      GetCSSEquivalentToHTMLInlineStyleSet(node, aHTMLProperty, aHTMLAttribute,
+      GetCSSEquivalentToHTMLInlineStyleSet(aNode, aHTMLProperty, aHTMLAttribute,
                                            valueString, aStyleType);
-    NS_ENSURE_SUCCESS(rv, rv);
+    NS_ENSURE_SUCCESS(rv, false);
 
     // early way out if we can
     if (valueString.IsEmpty()) {
-      return NS_OK;
+      return isSet;
     }
 
     if (nsGkAtoms::b == aHTMLProperty) {
       if (valueString.EqualsLiteral("bold")) {
-        aIsSet = true;
+        isSet = true;
       } else if (valueString.EqualsLiteral("normal")) {
-        aIsSet = false;
+        isSet = false;
       } else if (valueString.EqualsLiteral("bolder")) {
-        aIsSet = true;
+        isSet = true;
         valueString.AssignLiteral("bold");
       } else {
         int32_t weight = 0;
@@ -1115,32 +1136,31 @@ CSSEditUtils::IsCSSEquivalentToHTMLInlineStyleSet(
         nsAutoString value(valueString);
         weight = value.ToInteger(&errorCode);
         if (400 < weight) {
-          aIsSet = true;
+          isSet = true;
           valueString.AssignLiteral("bold");
         } else {
-          aIsSet = false;
+          isSet = false;
           valueString.AssignLiteral("normal");
         }
       }
     } else if (nsGkAtoms::i == aHTMLProperty) {
       if (valueString.EqualsLiteral("italic") ||
           valueString.EqualsLiteral("oblique")) {
-        aIsSet = true;
+        isSet = true;
       }
     } else if (nsGkAtoms::u == aHTMLProperty) {
       nsAutoString val;
       val.AssignLiteral("underline");
-      aIsSet = ChangeStyleTransaction::ValueIncludes(valueString, val);
+      isSet = ChangeStyleTransaction::ValueIncludes(valueString, val);
     } else if (nsGkAtoms::strike == aHTMLProperty) {
       nsAutoString val;
       val.AssignLiteral("line-through");
-      aIsSet = ChangeStyleTransaction::ValueIncludes(valueString, val);
-    } else if (aHTMLAttribute &&
-               ((nsGkAtoms::font == aHTMLProperty &&
-                 aHTMLAttribute->EqualsLiteral("color")) ||
-                aHTMLAttribute->EqualsLiteral("bgcolor"))) {
+      isSet = ChangeStyleTransaction::ValueIncludes(valueString, val);
+    } else if ((nsGkAtoms::font == aHTMLProperty &&
+                aHTMLAttribute == nsGkAtoms::color) ||
+               aHTMLAttribute == nsGkAtoms::bgcolor) {
       if (htmlValueString.IsEmpty()) {
-        aIsSet = true;
+        isSet = true;
       } else {
         nscolor rgba;
         nsAutoString subStr;
@@ -1174,54 +1194,53 @@ CSSEditUtils::IsCSSEquivalentToHTMLInlineStyleSet(
             htmlColor.Append(char16_t(')'));
           }
 
-          aIsSet = htmlColor.Equals(valueString,
-                                    nsCaseInsensitiveStringComparator());
+          isSet = htmlColor.Equals(valueString,
+                                   nsCaseInsensitiveStringComparator());
         } else {
-          aIsSet = htmlValueString.Equals(valueString,
-                                    nsCaseInsensitiveStringComparator());
+          isSet = htmlValueString.Equals(valueString,
+                                         nsCaseInsensitiveStringComparator());
         }
       }
     } else if (nsGkAtoms::tt == aHTMLProperty) {
-      aIsSet = StringBeginsWith(valueString, NS_LITERAL_STRING("monospace"));
+      isSet = StringBeginsWith(valueString, NS_LITERAL_STRING("monospace"));
     } else if (nsGkAtoms::font == aHTMLProperty && aHTMLAttribute &&
-               aHTMLAttribute->EqualsLiteral("face")) {
+               aHTMLAttribute == nsGkAtoms::face) {
       if (!htmlValueString.IsEmpty()) {
         const char16_t commaSpace[] = { char16_t(','), char16_t(' '), 0 };
         const char16_t comma[] = { char16_t(','), 0 };
         htmlValueString.ReplaceSubstring(commaSpace, comma);
         nsAutoString valueStringNorm(valueString);
         valueStringNorm.ReplaceSubstring(commaSpace, comma);
-        aIsSet = htmlValueString.Equals(valueStringNorm,
-                                        nsCaseInsensitiveStringComparator());
+        isSet = htmlValueString.Equals(valueStringNorm,
+                                       nsCaseInsensitiveStringComparator());
       } else {
-        aIsSet = true;
+        isSet = true;
       }
-      return NS_OK;
-    } else if (aHTMLAttribute && aHTMLAttribute->EqualsLiteral("align")) {
-      aIsSet = true;
+      return isSet;
+    } else if (aHTMLAttribute == nsGkAtoms::align) {
+      isSet = true;
     } else {
-      aIsSet = false;
-      return NS_OK;
+      return false;
     }
 
     if (!htmlValueString.IsEmpty() &&
         htmlValueString.Equals(valueString,
                                nsCaseInsensitiveStringComparator())) {
-      aIsSet = true;
+      isSet = true;
     }
 
     if (htmlValueString.EqualsLiteral("-moz-editor-invert-value")) {
-      aIsSet = !aIsSet;
+      isSet = !isSet;
     }
 
     if (nsGkAtoms::u == aHTMLProperty || nsGkAtoms::strike == aHTMLProperty) {
       // unfortunately, the value of the text-decoration property is not inherited.
       // that means that we have to look at ancestors of node to see if they are underlined
-      node = node->GetParentElement();  // set to null if it's not a dom element
+      aNode = aNode->GetParentElement(); // set to null if it's not a dom element
     }
   } while ((nsGkAtoms::u == aHTMLProperty ||
-            nsGkAtoms::strike == aHTMLProperty) && !aIsSet && node);
-  return NS_OK;
+            nsGkAtoms::strike == aHTMLProperty) && !isSet && aNode);
+  return isSet;
 }
 
 void
diff --git a/editor/libeditor/CSSEditUtils.h b/editor/libeditor/CSSEditUtils.h
index 0b9a12952..5129ab88d 100644
--- a/editor/libeditor/CSSEditUtils.h
+++ b/editor/libeditor/CSSEditUtils.h
@@ -90,6 +90,8 @@ public:
    */
   bool IsCSSEditableProperty(nsINode* aNode, nsIAtom* aProperty,
                              const nsAString* aAttribute);
+  bool IsCSSEditableProperty(nsINode* aNode, nsIAtom* aProperty,
+                             nsIAtom* aAttribute);
 
   /**
    * Adds/remove a CSS declaration to the STYLE atrribute carried by a given
@@ -188,14 +190,14 @@ public:
    *
    * @param aNode          [IN] A DOM node.
    * @param aHTMLProperty  [IN] An atom containing an HTML property.
-   * @param aAttribute     [IN] A pointer to an attribute name or nullptr if
+   * @param aAttribute     [IN] An atom of attribute name or nullptr if
    *                            irrelevant.
    * @param aValueString   [OUT] The list of CSS values.
    * @param aStyleType     [IN] eSpecified or eComputed.
    */
   nsresult GetCSSEquivalentToHTMLInlineStyleSet(nsINode* aNode,
                                                 nsIAtom* aHTMLProperty,
-                                                const nsAString* aAttribute,
+                                                nsIAtom* aAttribute,
                                                 nsAString& aValueString,
                                                 StyleType aStyleType);
 
@@ -205,18 +207,22 @@ public:
    *
    * @param aNode          [IN] A DOM node.
    * @param aHTMLProperty  [IN] An atom containing an HTML property.
-   * @param aAttribute     [IN] A pointer to an attribute name or nullptr if
-   *                            irrelevant.
-   * @param aIsSet         [OUT] A boolean being true if the css properties are
-   *                             set.
+   * @param aAttribute     [IN] A pointer/atom to an attribute name or nullptr
+   *                            if irrelevant.
    * @param aValueString   [IN/OUT] The attribute value (in) the list of CSS
    *                                values (out).
    * @param aStyleType     [IN] eSpecified or eComputed.
-   *
-   * The nsIContent variant returns aIsSet instead of using an out parameter.
+   * @return               A boolean being true if the css properties are
+   *                       set.
    */
   bool IsCSSEquivalentToHTMLInlineStyleSet(nsINode* aContent,
                                            nsIAtom* aProperty,
+                                           nsIAtom* aAttribute,
+                                           nsAString& aValue,
+                                           StyleType aStyleType);
+
+  bool IsCSSEquivalentToHTMLInlineStyleSet(nsINode* aContent,
+                                           nsIAtom* aProperty,
                                            const nsAString* aAttribute,
                                            const nsAString& aValue,
                                            StyleType aStyleType);
@@ -227,12 +233,11 @@ public:
                                            nsAString& aValue,
                                            StyleType aStyleType);
 
-  nsresult IsCSSEquivalentToHTMLInlineStyleSet(nsIDOMNode* aNode,
-                                               nsIAtom* aHTMLProperty,
-                                               const nsAString* aAttribute,
-                                               bool& aIsSet,
-                                               nsAString& aValueString,
-                                               StyleType aStyleType);
+  bool IsCSSEquivalentToHTMLInlineStyleSet(nsIDOMNode* aNode,
+                                           nsIAtom* aProperty,
+                                           const nsAString* aAttribute,
+                                           nsAString& aValue,
+                                           StyleType aStyleType);
 
   /**
    * Adds to the node the CSS inline styles equivalent to the HTML style
@@ -240,27 +245,29 @@ public:
    *
    * @param aNode          [IN] A DOM node.
    * @param aHTMLProperty  [IN] An atom containing an HTML property.
-   * @param aAttribute     [IN] A pointer to an attribute name or nullptr if
-   *                            irrelevant.
+   * @param aAttribute     [IN] A pointer/atom to an attribute name or nullptr
+   *                            if irrelevant.
    * @param aValue         [IN] The attribute value.
-   * @param aCount         [OUT] The number of CSS properties set by the call.
    * @param aSuppressTransaction [IN] A boolean indicating, when true,
    *                                  that no transaction should be recorded.
    *
-   * aCount is returned by the dom::Element variant instead of being an out
-   * parameter.
+   * @return               The number of CSS properties set by the call.
    */
   int32_t SetCSSEquivalentToHTMLStyle(dom::Element* aElement,
                                       nsIAtom* aProperty,
+                                      nsIAtom* aAttribute,
+                                      const nsAString* aValue,
+                                      bool aSuppressTransaction);
+  int32_t SetCSSEquivalentToHTMLStyle(dom::Element* aElement,
+                                      nsIAtom* aProperty,
+                                      const nsAString* aAttribute,
+                                      const nsAString* aValue,
+                                      bool aSuppressTransaction);
+  int32_t SetCSSEquivalentToHTMLStyle(nsIDOMNode* aNode,
+                                      nsIAtom* aHTMLProperty,
                                       const nsAString* aAttribute,
                                       const nsAString* aValue,
                                       bool aSuppressTransaction);
-  nsresult SetCSSEquivalentToHTMLStyle(nsIDOMNode* aNode,
-                                       nsIAtom* aHTMLProperty,
-                                       const nsAString* aAttribute,
-                                       const nsAString* aValue,
-                                       int32_t* aCount,
-                                       bool aSuppressTransaction);
 
   /**
    * Removes from the node the CSS inline styles equivalent to the HTML style.
@@ -284,7 +291,7 @@ public:
    *
    * @param aElement       [IN] A DOM Element (must not be null).
    * @param aHTMLProperty  [IN] An atom containing an HTML property.
-   * @param aAttribute     [IN] A pointer to an attribute name or nullptr if
+   * @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,
@@ -292,7 +299,7 @@ public:
    */
   nsresult RemoveCSSEquivalentToHTMLStyle(dom::Element* aElement,
                                           nsIAtom* aHTMLProperty,
-                                          const nsAString* aAttribute,
+                                          nsIAtom* aAttribute,
                                           const nsAString* aValue,
                                           bool aSuppressTransaction);
 
@@ -409,7 +416,7 @@ private:
    *
    * @param aNode              [IN] The DOM node.
    * @param aHTMLProperty      [IN] An atom containing an HTML property.
-   * @param aAttribute         [IN] A pointer to an attribute name or nullptr
+   * @param aAttribute         [IN] An atom to an attribute name or nullptr
    *                                if irrelevant
    * @param aValue             [IN] The attribute value.
    * @param aPropertyArray     [OUT] The array of CSS properties.
@@ -422,7 +429,7 @@ private:
    */
   void GenerateCSSDeclarationsFromHTMLStyle(dom::Element* aNode,
                                             nsIAtom* aHTMLProperty,
-                                            const nsAString* aAttribute,
+                                            nsIAtom* aAttribute,
                                             const nsAString* aValue,
                                             nsTArray<nsIAtom*>& aPropertyArray,
                                             nsTArray<nsString>& aValueArray,
diff --git a/editor/libeditor/HTMLEditRules.cpp b/editor/libeditor/HTMLEditRules.cpp
index af4a43ab9..14f6cfe11 100644
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -840,19 +840,18 @@ HTMLEditRules::GetAlignment(bool* aMixed,
 
   NS_ENSURE_TRUE(nodeToExamine, NS_ERROR_NULL_POINTER);
 
-  NS_NAMED_LITERAL_STRING(typeAttrName, "align");
   nsCOMPtr<Element> blockParent = htmlEditor->GetBlock(*nodeToExamine);
 
   NS_ENSURE_TRUE(blockParent, NS_ERROR_FAILURE);
 
   if (htmlEditor->IsCSSEnabled() &&
       htmlEditor->mCSSEditUtils->IsCSSEditableProperty(blockParent, nullptr,
-                                                        &typeAttrName)) {
+                                                       nsGkAtoms::align)) {
     // We are in CSS mode and we know how to align this element with CSS
     nsAutoString value;
     // Let's get the value(s) of text-align or margin-left/margin-right
     htmlEditor->mCSSEditUtils->GetCSSEquivalentToHTMLInlineStyleSet(
-        blockParent, nullptr, &typeAttrName, value, CSSEditUtils::eComputed);
+        blockParent, nullptr, nsGkAtoms::align, value, CSSEditUtils::eComputed);
     if (value.EqualsLiteral("center") ||
         value.EqualsLiteral("-moz-center") ||
         value.EqualsLiteral("auto auto")) {
@@ -4756,7 +4755,7 @@ HTMLEditRules::WillAlign(Selection& aSelection,
       NS_ENSURE_SUCCESS(rv, rv);
       if (useCSS) {
         htmlEditor->mCSSEditUtils->SetCSSEquivalentToHTMLStyle(
-            curNode->AsElement(), nullptr, &NS_LITERAL_STRING("align"),
+            curNode->AsElement(), nullptr, nsGkAtoms::align,
             &aAlignType, false);
         curDiv = nullptr;
         continue;
@@ -7143,9 +7142,9 @@ HTMLEditRules::CacheInlineStyles(nsIDOMNode* aNode)
                                               isSet, &outValue);
     } else {
       NS_ENSURE_STATE(mHTMLEditor);
-      mHTMLEditor->mCSSEditUtils->IsCSSEquivalentToHTMLInlineStyleSet(aNode,
-        mCachedStyles[j].tag, &(mCachedStyles[j].attr), isSet, outValue,
-        CSSEditUtils::eComputed);
+      isSet = mHTMLEditor->mCSSEditUtils->IsCSSEquivalentToHTMLInlineStyleSet(
+                aNode, mCachedStyles[j].tag, &(mCachedStyles[j].attr), outValue,
+                CSSEditUtils::eComputed);
     }
     if (isSet) {
       mCachedStyles[j].mPresent = true;
diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp
index c2f0bdc6d..82ef92042 100644
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -4398,30 +4398,35 @@ HTMLEditor::SetAttributeOrEquivalent(nsIDOMElement* aElement,
   nsAutoScriptBlocker scriptBlocker;
 
   if (IsCSSEnabled() && mCSSEditUtils) {
-    int32_t count;
-    nsresult rv =
-      mCSSEditUtils->SetCSSEquivalentToHTMLStyle(aElement, nullptr,
-                                                 &aAttribute, &aValue,
-                                                 &count,
+    nsCOMPtr<dom::Element> element = do_QueryInterface(aElement);
+    MOZ_ASSERT(element);
+
+    nsCOMPtr<nsIAtom> attribute = NS_Atomize(aAttribute);
+    MOZ_ASSERT(attribute);
+
+    int32_t count =
+      mCSSEditUtils->SetCSSEquivalentToHTMLStyle(element, nullptr,
+                                                 attribute, &aValue,
                                                  aSuppressTransaction);
-    NS_ENSURE_SUCCESS(rv, rv);
     if (count) {
       // we found an equivalence ; let's remove the HTML attribute itself if it is set
       nsAutoString existingValue;
       bool wasSet = false;
-      rv = GetAttributeValue(aElement, aAttribute, existingValue, &wasSet);
+      nsresult rv =
+        GetAttributeValue(aElement, aAttribute, existingValue, &wasSet);
       NS_ENSURE_SUCCESS(rv, rv);
       if (!wasSet) {
         return NS_OK;
       }
-      return aSuppressTransaction ? aElement->RemoveAttribute(aAttribute) :
-                                    RemoveAttribute(aElement, aAttribute);
+      return aSuppressTransaction ?
+               element->UnsetAttr(kNameSpaceID_None, attribute, true) :
+               RemoveAttribute(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.EqualsLiteral("style")) {
+    if (attribute == nsGkAtoms::style) {
       // if it is the style attribute, just add the new value to the existing style
       // attribute's value
       nsAutoString existingValue;
@@ -4432,14 +4437,15 @@ HTMLEditor::SetAttributeOrEquivalent(nsIDOMElement* aElement,
       existingValue.Append(' ');
       existingValue.Append(aValue);
       return aSuppressTransaction ?
-        aElement->SetAttribute(aAttribute, existingValue) :
+        element->SetAttr(kNameSpaceID_None, attribute, existingValue, true) :
         SetAttribute(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->SetAttribute(aAttribute, aValue) :
-                                  SetAttribute(aElement, aAttribute, aValue);
+    return aSuppressTransaction ?
+             element->SetAttr(kNameSpaceID_None, attribute, aValue, true) :
+             SetAttribute(aElement, aAttribute, aValue);
   }
 
   // we are not in an HTML+CSS editor; let's set the attribute the HTML way
@@ -4461,7 +4467,7 @@ HTMLEditor::RemoveAttributeOrEquivalent(nsIDOMElement* aElement,
   if (IsCSSEnabled() && mCSSEditUtils) {
     nsresult rv =
       mCSSEditUtils->RemoveCSSEquivalentToHTMLStyle(
-        element, nullptr, &aAttribute, nullptr, aSuppressTransaction);
+        element, nullptr, attribute, nullptr, aSuppressTransaction);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
@@ -4523,7 +4529,6 @@ HTMLEditor::SetCSSBackgroundColor(const nsAString& aColor)
   NS_ENSURE_SUCCESS(rv, rv);
   if (!cancel && !handled) {
     // Loop through the ranges in the selection
-    NS_NAMED_LITERAL_STRING(bgcolor, "bgcolor");
     for (uint32_t i = 0; i < selection->RangeCount(); i++) {
       RefPtr<nsRange> range = selection->GetRangeAt(i);
       NS_ENSURE_TRUE(range, NS_ERROR_FAILURE);
@@ -4542,13 +4547,15 @@ HTMLEditor::SetCSSBackgroundColor(const nsAString& aColor)
         if (blockParent && cachedBlockParent != blockParent) {
           cachedBlockParent = blockParent;
           mCSSEditUtils->SetCSSEquivalentToHTMLStyle(blockParent, nullptr,
-                                                     &bgcolor, &aColor, false);
+                                                     nsGkAtoms::bgcolor,
+                                                     &aColor, false);
         }
       } else if (startNode == endNode &&
                  startNode->IsHTMLElement(nsGkAtoms::body) && isCollapsed) {
         // No block in the document, let's apply the background to the body
         mCSSEditUtils->SetCSSEquivalentToHTMLStyle(startNode->AsElement(),
-                                                   nullptr, &bgcolor, &aColor,
+                                                   nullptr, nsGkAtoms::bgcolor,
+                                                   &aColor,
                                                    false);
       } else if (startNode == endNode && (endOffset - startOffset == 1 ||
                                           (!startOffset && !endOffset))) {
@@ -4559,7 +4566,8 @@ HTMLEditor::SetCSSBackgroundColor(const nsAString& aColor)
         if (blockParent && cachedBlockParent != blockParent) {
           cachedBlockParent = blockParent;
           mCSSEditUtils->SetCSSEquivalentToHTMLStyle(blockParent, nullptr,
-                                                     &bgcolor, &aColor, false);
+                                                     nsGkAtoms::bgcolor,
+                                                     &aColor, false);
         }
       } else {
         // Not the easy case.  Range not contained in single text node.  There
@@ -4602,7 +4610,8 @@ HTMLEditor::SetCSSBackgroundColor(const nsAString& aColor)
           if (blockParent && cachedBlockParent != blockParent) {
             cachedBlockParent = blockParent;
             mCSSEditUtils->SetCSSEquivalentToHTMLStyle(blockParent, nullptr,
-                                                       &bgcolor, &aColor,
+                                                       nsGkAtoms::bgcolor,
+                                                       &aColor,
                                                        false);
           }
         }
@@ -4613,7 +4622,8 @@ HTMLEditor::SetCSSBackgroundColor(const nsAString& aColor)
           if (blockParent && cachedBlockParent != blockParent) {
             cachedBlockParent = blockParent;
             mCSSEditUtils->SetCSSEquivalentToHTMLStyle(blockParent, nullptr,
-                                                       &bgcolor, &aColor,
+                                                       nsGkAtoms::bgcolor,
+                                                       &aColor,
                                                        false);
           }
         }
@@ -4627,7 +4637,8 @@ HTMLEditor::SetCSSBackgroundColor(const nsAString& aColor)
           if (blockParent && cachedBlockParent != blockParent) {
             cachedBlockParent = blockParent;
             mCSSEditUtils->SetCSSEquivalentToHTMLStyle(blockParent, nullptr,
-                                                       &bgcolor, &aColor,
+                                                       nsGkAtoms::bgcolor,
+                                                       &aColor,
                                                        false);
           }
         }
diff --git a/editor/libeditor/HTMLStyleEditor.cpp b/editor/libeditor/HTMLStyleEditor.cpp
index 72eb1d690..d3eabba1d 100644
--- a/editor/libeditor/HTMLStyleEditor.cpp
+++ b/editor/libeditor/HTMLStyleEditor.cpp
@@ -429,7 +429,7 @@ HTMLEditor::SetInlinePropertyOnNodeImpl(nsIContent& aNode,
                  mCSSEditUtils->IsCSSEditableProperty(&aNode, &aProperty,
                                                       aAttribute)) ||
                 // bgcolor is always done using CSS
-                aAttribute->EqualsLiteral("bgcolor");
+                attrAtom == nsGkAtoms::bgcolor;
 
   if (useCSS) {
     nsCOMPtr<dom::Element> tmp;
@@ -444,12 +444,9 @@ HTMLEditor::SetInlinePropertyOnNodeImpl(nsIContent& aNode,
     }
 
     // Add the CSS styles corresponding to the HTML style request
-    int32_t count;
-    nsresult rv =
-      mCSSEditUtils->SetCSSEquivalentToHTMLStyle(tmp->AsDOMNode(),
-                                                 &aProperty, aAttribute,
-                                                 &aValue, &count, false);
-    NS_ENSURE_SUCCESS(rv, rv);
+    mCSSEditUtils->SetCSSEquivalentToHTMLStyle(tmp,
+                                               &aProperty, attrAtom,
+                                               &aValue, false);
     return NS_OK;
   }
 
@@ -576,8 +573,9 @@ HTMLEditor::SplitStyleAbovePoint(nsCOMPtr<nsINode>* aNode,
       // in this implementation for the node; let's check if it carries those
       // CSS styles
       nsAutoString firstValue;
-      mCSSEditUtils->IsCSSEquivalentToHTMLInlineStyleSet(GetAsDOMNode(node),
-        aProperty, aAttribute, isSet, firstValue, CSSEditUtils::eSpecified);
+      isSet = mCSSEditUtils->IsCSSEquivalentToHTMLInlineStyleSet(
+                node, aProperty, aAttribute, firstValue,
+                CSSEditUtils::eSpecified);
     }
     if (// node is the correct inline prop
         (aProperty && node->IsHTMLElement(aProperty)) ||
@@ -796,15 +794,17 @@ HTMLEditor::RemoveStyleInside(nsIContent& aNode,
     // the HTML style defined by aProperty/aAttribute has a CSS equivalence in
     // this implementation for the node aNode; let's check if it carries those
     // css styles
+    nsCOMPtr<nsIAtom> attribute =
+      aAttribute ? NS_Atomize(*aAttribute) : nullptr;
     nsAutoString propertyValue;
     bool isSet = mCSSEditUtils->IsCSSEquivalentToHTMLInlineStyleSet(&aNode,
-      aProperty, aAttribute, propertyValue, CSSEditUtils::eSpecified);
+      aProperty, attribute, propertyValue, CSSEditUtils::eSpecified);
     if (isSet && aNode.IsElement()) {
       // yes, tmp has the corresponding css declarations in its style attribute
       // let's remove them
       mCSSEditUtils->RemoveCSSEquivalentToHTMLStyle(aNode.AsElement(),
                                                     aProperty,
-                                                    aAttribute,
+                                                    attribute,
                                                     &propertyValue,
                                                     false);
       // remove the node if it is a span or font, if its style attribute is