Bug 1539110 - Make HTMLEditor::RemoveStyleInside() and HTMLEditor::Sp…

…litStyleAbovePoint() check tag names with whitelist r=m_kato

`document.execCommand("removeformat")` removes any elements in the range which
are editable, not `<a>`, not block and a container.
https://searchfox.org/mozilla-central/rev/dd7e27f4a805e4115d0dbee70e1220b23b23c567/editor/libeditor/HTMLStyleEditor.cpp#760-763

This means that it removes hidden elements like `<script>` and `<style>`,
or non-HTML elements like SVG elements.  However, the unofficial document
of `execCommand()` lists up elements which should be handled by the command.
https://w3c.github.io/editing/execCommand.html#removeformat-candidate

Additionally, Chrome respects this list since not including `<del>` element
into the list does not make sense but Chrome ignores it.  So, we should
respect the list.

Differential Revision: https://phabricator.services.mozilla.com/D27018

--HG--
extra : moz-landing-system : lando

editor/libeditor/HTMLEditUtils.cpp

@@ -37,6 +37,34 @@ bool HTMLEditUtils::IsInlineStyle(nsINode* aNode) {
       nsGkAtoms::sup, nsGkAtoms::font);
 }
 
+bool HTMLEditUtils::IsRemovableInlineStyleElement(Element& aElement) {
+  if (!aElement.IsHTMLElement()) {
+    return false;
+  }
+  // https://w3c.github.io/editing/execCommand.html#removeformat-candidate
+  if (aElement.IsAnyOfHTMLElements(
+          nsGkAtoms::abbr,  // Chrome ignores, but does not make sense.
+          nsGkAtoms::acronym, nsGkAtoms::b,
+          nsGkAtoms::bdi,  // Chrome ignores, but does not make sense.
+          nsGkAtoms::bdo, nsGkAtoms::big, nsGkAtoms::cite, nsGkAtoms::code,
+          // nsGkAtoms::del, Chrome ignores, but does not make sense but
+          // execCommand unofficial draft excludes this.  Spec issue:
+          // https://github.com/w3c/editing/issues/192
+          nsGkAtoms::dfn, nsGkAtoms::em, nsGkAtoms::font, nsGkAtoms::i,
+          nsGkAtoms::ins, nsGkAtoms::kbd,
+          nsGkAtoms::mark,  // Chrome ignores, but does not make sense.
+          nsGkAtoms::nobr, nsGkAtoms::q, nsGkAtoms::s, nsGkAtoms::samp,
+          nsGkAtoms::small, nsGkAtoms::span, nsGkAtoms::strike,
+          nsGkAtoms::strong, nsGkAtoms::sub, nsGkAtoms::sup, nsGkAtoms::tt,
+          nsGkAtoms::u, nsGkAtoms::var)) {
+    return true;
+  }
+  // If it's a <blink> element, we can remove it.
+  nsAutoString tagName;
+  aElement.GetTagName(tagName);
+  return tagName.LowerCaseEqualsASCII("blink");
+}
+
 /**
  * IsFormatNode() returns true if aNode is a format node.
  */

editor/libeditor/HTMLEditUtils.h

@@ -12,10 +12,19 @@ class nsAtom;
 class nsINode;
 
 namespace mozilla {
+namespace dom {
+class Element;
+}  // namespace dom
 
 class HTMLEditUtils final {
  public:
   static bool IsInlineStyle(nsINode* aNode);
+  /**
+   * IsRemovableInlineStyleElement() returns true if aElement is an inline
+   * element and can be removed or split to in order to modifying inline
+   * styles.
+   */
+  static bool IsRemovableInlineStyleElement(dom::Element& aElement);
   static bool IsFormatNode(nsINode* aNode);
   static bool IsNodeThatCanOutdent(nsINode* aNode);
   static bool IsHeader(nsINode& aNode);

editor/libeditor/HTMLEditor.h

@@ -2108,7 +2108,6 @@ class HTMLEditor final : public TextEditor,
                              nsAtom* aAttribute,
                              const bool aChildrenOnly = false);
 
-  bool NodeIsProperty(nsINode& aNode);
   bool IsAtFrontOfNode(nsINode& aNode, int32_t aOffset);
   bool IsAtEndOfNode(nsINode& aNode, int32_t aOffset);
   bool IsOnlyAttribute(const Element* aElement, nsAtom* aAttribute);

editor/libeditor/HTMLStyleEditor.cpp

@@ -634,7 +634,8 @@ nsresult HTMLEditor::SplitStyleAbovePoint(
         // node is href - test if really <a href=...
         (aProperty == nsGkAtoms::href && HTMLEditUtils::IsLink(node)) ||
         // or node is any prop, and we asked to split them all
-        (!aProperty && NodeIsProperty(*node)) ||
+        (!aProperty && node->IsElement() && IsEditable(node) &&
+         HTMLEditUtils::IsRemovableInlineStyleElement(*node->AsElement())) ||
         // or the style is specified in the style attribute
         isSet) {
       // Found a style node we need to split
@@ -757,11 +758,6 @@ nsresult HTMLEditor::ClearStyle(nsCOMPtr<nsINode>* aNode, int32_t* aOffset,
   return NS_OK;
 }
 
-bool HTMLEditor::NodeIsProperty(nsINode& aNode) {
-  return IsContainer(&aNode) && IsEditable(&aNode) && !IsBlockNode(&aNode) &&
-         !aNode.IsHTMLElement(nsGkAtoms::a);
-}
-
 nsresult HTMLEditor::RemoveStyleInside(nsIContent& aNode, nsAtom* aProperty,
                                        nsAtom* aAttribute,
                                        const bool aChildrenOnly /* = false */) {
@@ -788,7 +784,8 @@ nsresult HTMLEditor::RemoveStyleInside(nsIContent& aNode, nsAtom* aProperty,
        // and for named anchors
        (aProperty == nsGkAtoms::name && HTMLEditUtils::IsNamedAnchor(&aNode)) ||
        // or node is any prop and we asked for that
-       (!aProperty && NodeIsProperty(aNode)))) {
+       (!aProperty && IsEditable(&aNode) &&
+        HTMLEditUtils::IsRemovableInlineStyleElement(*aNode.AsElement())))) {
     // if we weren't passed an attribute, then we want to
     // remove any matching inlinestyles entirely
     if (!aAttribute) {

testing/web-platform/meta/editing/run/removeformat.html.ini

@@ -5,33 +5,6 @@
   [[["stylewithcss","false"\],["removeformat",""\]\] "foo<b id=foo>b[a\]r</b>baz" compare innerHTML]
     expected: FAIL
 
-  [[["removeformat",""\]\] "[foo<del>bar</del>baz\]" compare innerHTML]
-    expected: FAIL
-
-  [[["removeformat",""\]\] "foo<del>b[a\]r</del>baz" compare innerHTML]
-    expected: FAIL
-
-  [[["removeformat",""\]\] "[foo<nobr>bar</nobr>baz\]" compare innerHTML]
-    expected: FAIL
-
-  [[["removeformat",""\]\] "foo<nobr>b[a\]r</nobr>baz" compare innerHTML]
-    expected: FAIL
-
-  [[["removeformat",""\]\] "[foo<svg><circle fill=blue r=20 cx=20 cy=20 /></svg>bar\]" compare innerHTML]
-    expected: FAIL
-
-  [[["removeformat",""\]\] "[foo<nonexistentelement>bar</nonexistentelement>baz\]" compare innerHTML]
-    expected: FAIL
-
-  [[["removeformat",""\]\] "foo<nonexistentelement>b[a\]r</nonexistentelement>baz" compare innerHTML]
-    expected: FAIL
-
-  [[["removeformat",""\]\] "[foo<nonexistentelement style=\\"display: block\\">bar</nonexistentelement>baz\]" compare innerHTML]
-    expected: FAIL
-
-  [[["removeformat",""\]\] "foo<nonexistentelement style=\\"display: block\\">b[a\]r</nonexistentelement>baz" compare innerHTML]
-    expected: FAIL
-
   [[["removeformat",""\]\] "foo<span id=foo>b[a\]r</span>baz" compare innerHTML]
     expected: FAIL
 

testing/web-platform/tests/editing/data/removeformat.js

@@ -75,6 +75,26 @@ var browserTests = [
     "[foobarbaz]",
     [true,true],
     {"stylewithcss":[false,true,"",false,false,""],"removeformat":[false,false,"",false,false,""]}],
+["[foo<span style=\"display: none\">bar</span>baz]",
+    [["stylewithcss","true"],["removeformat",""]],
+    "[foobarbaz]",
+    [true,true],
+    {"stylewithcss":[false,false,"",false,true,""],"removeformat":[false,false,"",false,false,""]}],
+["[foo<span style=\"display: none\">bar</span>baz]",
+    [["stylewithcss","false"],["removeformat",""]],
+    "[foobarbaz]",
+    [true,true],
+    {"stylewithcss":[false,true,"",false,false,""],"removeformat":[false,false,"",false,false,""]}],
+["[foo<span style=\"display: none; font-weight: bold\">bar</span>baz]",
+    [["stylewithcss","true"],["removeformat",""]],
+    "[foobarbaz]",
+    [true,true],
+    {"stylewithcss":[false,false,"",false,true,""],"removeformat":[false,false,"",false,false,""]}],
+["[foo<span style=\"display: none; font-weight: bold\">bar</span>baz]",
+    [["stylewithcss","false"],["removeformat",""]],
+    "[foobarbaz]",
+    [true,true],
+    {"stylewithcss":[false,true,"",false,false,""],"removeformat":[false,false,"",false,false,""]}],
 ["foo<span style=\"font-weight: bold\">b[a]r</span>baz",
     [["stylewithcss","true"],["removeformat",""]],
     "foo<span style=\"font-weight:bold\">b</span>[a]<span style=\"font-weight:bold\">r</span>baz",
@@ -85,6 +105,26 @@ var browserTests = [
     "foo<span style=\"font-weight:bold\">b</span>[a]<span style=\"font-weight:bold\">r</span>baz",
     [true,true],
     {"stylewithcss":[false,true,"",false,false,""],"removeformat":[false,false,"",false,false,""]}],
+["foo<span style=\"display: none\">b[a]r</span>baz",
+    [["stylewithcss","true"],["removeformat",""]],
+    "foo<span style=\"display:none\">b</span>[a]<span style=\"display:none\">r</span>baz",
+    [true,true],
+    {"stylewithcss":[false,false,"",false,true,""],"removeformat":[false,false,"",false,false,""]}],
+["foo<span style=\"display: none\">b[a]r</span>baz",
+    [["stylewithcss","false"],["removeformat",""]],
+    "foo<span style=\"display:none\">b</span>[a]<span style=\"display:none\">r</span>baz",
+    [true,true],
+    {"stylewithcss":[false,true,"",false,false,""],"removeformat":[false,false,"",false,false,""]}],
+["foo<span style=\"display: none; font-weight: bold\">b[a]r</span>baz",
+    [["stylewithcss","true"],["removeformat",""]],
+    "foo<span style=\"display:none; font-weight:bold\">b</span>[a]<span style=\"display:none; font-weight:bold\">r</span>baz",
+    [true,true],
+    {"stylewithcss":[false,false,"",false,true,""],"removeformat":[false,false,"",false,false,""]}],
+["foo<span style=\"display: none; font-weight: bold\">b[a]r</span>baz",
+    [["stylewithcss","false"],["removeformat",""]],
+    "foo<span style=\"display:none; font-weight:bold\">b</span>[a]<span style=\"display:none; font-weight:bold\">r</span>baz",
+    [true,true],
+    {"stylewithcss":[false,true,"",false,false,""],"removeformat":[false,false,"",false,false,""]}],
 ["[foo<span style=\"font-variant: small-caps\">bar</span>baz]",
     [["stylewithcss","true"],["removeformat",""]],
     "[foobarbaz]",