Bug 1881225 - Consider CDATASections for dir=auto. r=emilio

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

dom/base/CharacterData.cpp

@@ -250,9 +250,7 @@ nsresult CharacterData::SetTextInternal(
 
   auto oldDir = Directionality::Unset;
   const bool dirAffectsAncestor =
-      NodeType() == TEXT_NODE &&
+      IsText() && TextNodeWillChangeDirection(AsText(), &oldDir, aOffset);
-      TextNodeWillChangeDirection(static_cast<nsTextNode*>(this), &oldDir,
-                                  aOffset);
 
   if (aOffset == 0 && endOffset == textLength) {
     // Replacing whole text or old text was empty.
@@ -313,8 +311,8 @@ nsresult CharacterData::SetTextInternal(
   if (dirAffectsAncestor) {
     // dirAffectsAncestor being true implies that we have a text node, see
     // above.
-    MOZ_ASSERT(NodeType() == TEXT_NODE);
+    MOZ_ASSERT(IsText());
-    TextNodeChangedDirection(static_cast<nsTextNode*>(this), oldDir, aNotify);
+    TextNodeChangedDirection(AsText(), oldDir, aNotify);
   }
 
   // Notify observers

dom/base/DirectionalityUtils.cpp

@@ -145,12 +145,12 @@
 #include "mozilla/dom/HTMLInputElement.h"
 #include "mozilla/dom/HTMLSlotElement.h"
 #include "mozilla/dom/ShadowRoot.h"
+#include "mozilla/dom/Text.h"
 #include "mozilla/dom/UnbindContext.h"
 #include "mozilla/intl/UnicodeProperties.h"
 #include "nsUnicodeProperties.h"
 #include "nsTextFragment.h"
 #include "nsAttrValue.h"
-#include "nsTextNode.h"
 
 namespace mozilla {
 
@@ -158,6 +158,7 @@ using mozilla::dom::Element;
 using mozilla::dom::HTMLInputElement;
 using mozilla::dom::HTMLSlotElement;
 using mozilla::dom::ShadowRoot;
+using mozilla::dom::Text;
 
 static nsIContent* GetParentOrHostOrSlot(const nsIContent* aContent) {
   if (HTMLSlotElement* slot = aContent->GetAssignedSlot()) {
@@ -228,7 +229,7 @@ inline static bool TextChildrenAffectDirAutoAncestor(nsIContent* aContent) {
          aContent->NodeOrAncestorHasDirAuto();
 }
 
-inline static bool NodeAffectsDirAutoAncestor(nsTextNode* aTextNode) {
+inline static bool NodeAffectsDirAutoAncestor(Text* aTextNode) {
   nsIContent* parent = GetParentOrHostOrSlot(aTextNode);
   return parent && TextChildrenAffectDirAutoAncestor(parent) &&
          !aTextNode->IsInNativeAnonymousSubtree();
@@ -292,7 +293,7 @@ static Directionality GetDirectionFromText(const char* aText,
   return Directionality::Unset;
 }
 
-static Directionality GetDirectionFromText(const mozilla::dom::Text* aTextNode,
+static Directionality GetDirectionFromText(const Text* aTextNode,
                                            uint32_t* aFirstStrong = nullptr) {
   const nsTextFragment* frag = &aTextNode->TextFragment();
   if (frag->Is2b()) {
@@ -302,7 +303,7 @@ static Directionality GetDirectionFromText(const mozilla::dom::Text* aTextNode,
   return GetDirectionFromText(frag->Get1b(), frag->GetLength(), aFirstStrong);
 }
 
-static nsTextNode* WalkDescendantsAndGetDirectionFromText(
+static Text* WalkDescendantsAndGetDirectionFromText(
     nsINode* aRoot, Directionality* aDirectionality) {
   nsIContent* child = aRoot->GetFirstChild();
   while (child) {
@@ -317,8 +318,7 @@ static nsTextNode* WalkDescendantsAndGetDirectionFromText(
       const nsTArray<RefPtr<nsINode>>& assignedNodes = slot->AssignedNodes();
       for (uint32_t i = 0; i < assignedNodes.Length(); ++i) {
         nsIContent* assignedNode = assignedNodes[i]->AsContent();
-        if (assignedNode->NodeType() == nsINode::TEXT_NODE) {
+        if (auto* text = Text::FromNode(assignedNode)) {
-          auto* text = static_cast<nsTextNode*>(assignedNode);
           Directionality textNodeDir = GetDirectionFromText(text);
           if (textNodeDir != Directionality::Unset) {
             *aDirectionality = textNodeDir;
@@ -326,8 +326,8 @@ static nsTextNode* WalkDescendantsAndGetDirectionFromText(
           }
         } else if (assignedNode->IsElement() &&
                    AffectsDirectionOfAncestors(assignedNode->AsElement())) {
-          nsTextNode* text = WalkDescendantsAndGetDirectionFromText(
+          Text* text = WalkDescendantsAndGetDirectionFromText(assignedNode,
-              assignedNode, aDirectionality);
+                                                              aDirectionality);
           if (text) {
             return text;
           }
@@ -335,8 +335,7 @@ static nsTextNode* WalkDescendantsAndGetDirectionFromText(
       }
     }
 
-    if (child->NodeType() == nsINode::TEXT_NODE) {
+    if (auto* text = Text::FromNode(child)) {
-      auto* text = static_cast<nsTextNode*>(child);
       Directionality textNodeDir = GetDirectionFromText(text);
       if (textNodeDir != Directionality::Unset) {
         *aDirectionality = textNodeDir;
@@ -355,7 +354,7 @@ static nsTextNode* WalkDescendantsAndGetDirectionFromText(
  *
  * @return the text node containing the character that determined the direction
  */
-static nsTextNode* WalkDescendantsSetDirectionFromText(Element* aElement,
+static Text* WalkDescendantsSetDirectionFromText(Element* aElement,
                                                  bool aNotify) {
   MOZ_ASSERT(aElement, "Must have an element");
   MOZ_ASSERT(aElement->HasDirAuto(), "Element must have dir=auto");
@@ -368,7 +367,7 @@ static nsTextNode* WalkDescendantsSetDirectionFromText(Element* aElement,
 
   // Check the text in Shadow DOM.
   if (ShadowRoot* shadowRoot = aElement->GetShadowRoot()) {
-    nsTextNode* text =
+    Text* text =
         WalkDescendantsAndGetDirectionFromText(shadowRoot, &textNodeDir);
     if (text) {
       aElement->SetDirectionality(textNodeDir, aNotify);
@@ -377,8 +376,7 @@ static nsTextNode* WalkDescendantsSetDirectionFromText(Element* aElement,
   }
 
   // Check the text in light DOM.
-  nsTextNode* text =
+  Text* text = WalkDescendantsAndGetDirectionFromText(aElement, &textNodeDir);
-      WalkDescendantsAndGetDirectionFromText(aElement, &textNodeDir);
   if (text) {
     aElement->SetDirectionality(textNodeDir, aNotify);
     return text;
@@ -494,8 +492,7 @@ void SetDirectionalityOnDescendants(Element* aElement, Directionality aDir,
 
 static void ResetAutoDirection(Element* aElement, bool aNotify) {
   MOZ_ASSERT(aElement->HasDirAuto());
-  nsTextNode* setByNode =
+  Text* setByNode = WalkDescendantsSetDirectionFromText(aElement, aNotify);
-      WalkDescendantsSetDirectionFromText(aElement, aNotify);
   if (setByNode) {
     setByNode->SetMaySetDirAuto();
   }
@@ -516,7 +513,7 @@ void WalkAncestorsResetAutoDirection(Element* aElement, bool aNotify) {
     if (!parentElement || !parentElement->HasDirAuto()) {
       continue;
     }
-    nsTextNode* setByNode =
+    Text* setByNode =
         WalkDescendantsSetDirectionFromText(parentElement, aNotify);
     if (setByNode) {
       setByNode->SetMaySetDirAuto();
@@ -638,7 +635,7 @@ void WalkDescendantsSetDirAuto(Element* aElement, bool aNotify) {
     SetAncestorHasDirAutoOnDescendants(aElement);
   }
 
-  nsTextNode* textNode = WalkDescendantsSetDirectionFromText(aElement, aNotify);
+  Text* textNode = WalkDescendantsSetDirectionFromText(aElement, aNotify);
   if (textNode) {
     textNode->SetMaySetDirAuto();
   }
@@ -716,13 +713,12 @@ static DirAutoElementResult FindDirAutoElementFrom(nsIContent* aContent) {
   return {nullptr, false};
 }
 
-static DirAutoElementResult FindDirAutoElementForText(nsTextNode* aTextNode) {
+static DirAutoElementResult FindDirAutoElementForText(Text* aTextNode) {
-  MOZ_ASSERT(aTextNode->NodeType() == nsINode::TEXT_NODE,
+  MOZ_ASSERT(aTextNode->IsText(), "Must be a text node");
-             "Must be a text node");
   return FindDirAutoElementFrom(GetParentOrHostOrSlot(aTextNode));
 }
 
-static DirAutoElementResult SetAncestorDirectionIfAuto(nsTextNode* aTextNode,
+static DirAutoElementResult SetAncestorDirectionIfAuto(Text* aTextNode,
                                                        Directionality aDir,
                                                        bool aNotify = true) {
   auto result = FindDirAutoElementForText(aTextNode);
@@ -741,7 +737,7 @@ static DirAutoElementResult SetAncestorDirectionIfAuto(nsTextNode* aTextNode,
   return result;
 }
 
-bool TextNodeWillChangeDirection(nsTextNode* aTextNode, Directionality* aOldDir,
+bool TextNodeWillChangeDirection(Text* aTextNode, Directionality* aOldDir,
                                  uint32_t aOffset) {
   if (!NodeAffectsDirAutoAncestor(aTextNode)) {
     return false;
@@ -754,7 +750,7 @@ bool TextNodeWillChangeDirection(nsTextNode* aTextNode, Directionality* aOldDir,
   return (aOffset <= firstStrong);
 }
 
-void TextNodeChangedDirection(nsTextNode* aTextNode, Directionality aOldDir,
+void TextNodeChangedDirection(Text* aTextNode, Directionality aOldDir,
                               bool aNotify) {
   MOZ_ASSERT(NodeAffectsDirAutoAncestor(aTextNode), "Caller should check");
   Directionality newDir = GetDirectionFromText(aTextNode);
@@ -771,7 +767,7 @@ void TextNodeChangedDirection(nsTextNode* aTextNode, Directionality aOldDir,
   }
 }
 
-void SetDirectionFromNewTextNode(nsTextNode* aTextNode) {
+void SetDirectionFromNewTextNode(Text* aTextNode) {
   if (!NodeAffectsDirAutoAncestor(aTextNode)) {
     return;
   }
@@ -787,7 +783,7 @@ void SetDirectionFromNewTextNode(nsTextNode* aTextNode) {
   }
 }
 
-void ResetDirectionSetByTextNode(nsTextNode* aTextNode,
+void ResetDirectionSetByTextNode(Text* aTextNode,
                                  dom::UnbindContext& aContext) {
   MOZ_ASSERT(!aTextNode->IsInComposedDoc(), "Should be disconnected already");
   if (!aTextNode->MaySetDirAuto()) {

dom/base/DirectionalityUtils.h

@@ -13,11 +13,11 @@
 class nsIContent;
 class nsINode;
 class nsAttrValue;
-class nsTextNode;
 
 namespace mozilla::dom {
 class Element;
 class HTMLSlotElement;
+class Text;
 struct UnbindContext;
 }  // namespace mozilla::dom
 
@@ -115,27 +115,27 @@ void WalkDescendantsClearAncestorDirAuto(nsIContent* aContent);
  *
  * @return whether the text node affects the directionality of any element
  */
-bool TextNodeWillChangeDirection(nsTextNode* aTextNode, Directionality* aOldDir,
+bool TextNodeWillChangeDirection(dom::Text* aTextNode, Directionality* aOldDir,
                                  uint32_t aOffset);
 
 /**
  * After the contents of a text node have changed, change the directionality
  * of any elements whose directionality is determined by that node
  */
-void TextNodeChangedDirection(nsTextNode* aTextNode, Directionality aOldDir,
+void TextNodeChangedDirection(dom::Text* aTextNode, Directionality aOldDir,
                               bool aNotify);
 
 /**
  * When a text node is appended to an element, find any ancestors with dir=auto
  * whose directionality will be determined by the text node
  */
-void SetDirectionFromNewTextNode(nsTextNode* aTextNode);
+void SetDirectionFromNewTextNode(dom::Text* aTextNode);
 
 /**
  * When a text node is removed from a document, find any ancestors whose
  * directionality it determined and redetermine their directionality
  */
-void ResetDirectionSetByTextNode(nsTextNode*, dom::UnbindContext&);
+void ResetDirectionSetByTextNode(dom::Text*, dom::UnbindContext&);
 
 /**
  * Set the directionality of an element according to the directionality of the

dom/base/Text.cpp

@@ -124,6 +124,20 @@ already_AddRefed<Text> Text::Constructor(const GlobalObject& aGlobal,
   return window->GetDoc()->CreateTextNode(aData);
 }
 
+nsresult Text::BindToTree(BindContext& aContext, nsINode& aParent) {
+  nsresult rv = CharacterData::BindToTree(aContext, aParent);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  SetDirectionFromNewTextNode(this);
+
+  return NS_OK;
+}
+
+void Text::UnbindFromTree(UnbindContext& aContext) {
+  CharacterData::UnbindFromTree(aContext);
+  ResetDirectionSetByTextNode(this, aContext);
+}
+
 bool Text::HasTextForTranslation() {
   if (mText.Is2b()) {
     // The fragment contains non-8bit characters which means there

dom/base/Text.h

@@ -29,6 +29,9 @@ class Text : public CharacterData {
                                             const nsAString& aData,
                                             ErrorResult& aRv);
 
+  nsresult BindToTree(BindContext&, nsINode& aParent) override;
+  void UnbindFromTree(UnbindContext&) override;
+
   /**
    * Method to see if the text node contains data that is useful
    * for a translation: i.e., it consists of more than just whitespace,

dom/base/nsINode.h

@@ -2071,16 +2071,15 @@ class nsINode : public mozilla::dom::EventTarget {
   void ClearHasValidDir() { ClearBoolFlag(NodeHasValidDirAttribute); }
   bool HasValidDir() const { return GetBoolFlag(NodeHasValidDirAttribute); }
   void SetMaySetDirAuto() {
-    // FIXME(bug 1881225): dir=auto should probably work on CDATA too.
+    MOZ_ASSERT(IsText());
-    MOZ_ASSERT(NodeType() == TEXT_NODE);
     SetBoolFlag(NodeMaySetDirAuto);
   }
   bool MaySetDirAuto() const {
-    MOZ_ASSERT(NodeType() == TEXT_NODE);
+    MOZ_ASSERT(IsText());
     return GetBoolFlag(NodeMaySetDirAuto);
   }
   void ClearMaySetDirAuto() {
-    MOZ_ASSERT(NodeType() == TEXT_NODE);
+    MOZ_ASSERT(IsText());
     ClearBoolFlag(NodeMaySetDirAuto);
   }
   void SetAncestorHasDirAuto() { SetBoolFlag(NodeAncestorHasDirAuto); }

dom/base/nsTextNode.cpp

@@ -11,7 +11,6 @@
 #include "nsTextNode.h"
 #include "mozilla/dom/TextBinding.h"
 #include "nsContentUtils.h"
-#include "mozilla/dom/DirectionalityUtils.h"
 #include "mozilla/dom/Document.h"
 #include "nsThreadUtils.h"
 #include "nsStubMutationObserver.h"
@@ -114,20 +113,6 @@ nsresult nsTextNode::AppendTextForNormalize(const char16_t* aBuffer,
                          &details);
 }
 
-nsresult nsTextNode::BindToTree(BindContext& aContext, nsINode& aParent) {
-  nsresult rv = CharacterData::BindToTree(aContext, aParent);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  SetDirectionFromNewTextNode(this);
-
-  return NS_OK;
-}
-
-void nsTextNode::UnbindFromTree(UnbindContext& aContext) {
-  CharacterData::UnbindFromTree(aContext);
-  ResetDirectionSetByTextNode(this, aContext);
-}
-
 #ifdef MOZ_DOM_LIST
 void nsTextNode::List(FILE* out, int32_t aIndent) const {
   int32_t index;

dom/base/nsTextNode.h

@@ -44,9 +44,6 @@ class nsTextNode : public mozilla::dom::Text {
   already_AddRefed<CharacterData> CloneDataNode(
       mozilla::dom::NodeInfo* aNodeInfo, bool aCloneText) const override;
 
-  nsresult BindToTree(BindContext&, nsINode& aParent) override;
-  void UnbindFromTree(UnbindContext&) override;
-
   nsresult AppendTextForNormalize(const char16_t* aBuffer, uint32_t aLength,
                                   bool aNotify, nsIContent* aNextSibling);
 

testing/web-platform/tests/html/dom/elements/global-attributes/cdata-dir_auto.html

@@ -0,0 +1,71 @@
+<!DOCTYPE html>
+<head>
+    <meta charset="utf-8">
+    <link rel="author" title="Vincent Hilla" href="mailto:vhilla@mozilla.com">
+    <link rel="help" href="https://html.spec.whatwg.org/multipage/dom.html#the-directionality">
+    <script src="/resources/testharness.js"></script>
+    <script src="/resources/testharnessreport.js"></script>
+</head>
+<body>
+    <div id="test1" dir="auto">
+        <![CDATA[foo]]>اختبر
+    </div>
+
+    <iframe src="cdata-iframe.xhtml"></iframe>
+
+    <script>
+        function awaitMessage(msg) {
+            return new Promise(res => {
+                function waitAndRemove(e) {
+                    if (e.data != msg)
+                        return;
+                    window.removeEventListener("message", waitAndRemove);
+                    res();
+                }
+                window.addEventListener("message", waitAndRemove);
+            });
+        }
+
+        const subframeLoaded = awaitMessage("subframe-loaded");
+
+        async function createXHTMLCase(id) {
+            await subframeLoaded;
+
+            let iframe = document.querySelector("iframe");
+            iframe.contentWindow.postMessage(id, "*");
+
+            await awaitMessage(id);
+
+            const doc = iframe.contentDocument;
+            const div = doc.getElementById(id);
+            const cdata = div.firstChild;
+
+            return [div, cdata];
+        }
+
+        test(function() {
+            const div = document.getElementById("test1");
+            assert_true(div.matches(":dir(rtl)"));
+        }, "Content of CDATA is ignored for dir=auto in html document");
+
+        promise_test(async function() {
+            let [div, cdata] = await createXHTMLCase(1);
+            assert_true(div.matches(":dir(ltr)"));
+        }, "Text in CDATASection is considered when determining auto directionality");
+
+        promise_test(async function() {
+            let [div, cdata] = await createXHTMLCase(2);
+            assert_true(div.matches(":dir(ltr)"));
+            cdata.remove();
+            assert_true(div.matches(":dir(rtl)"));
+        }, "Directionality is updated when removing CDATASection");
+
+        promise_test(async function() {
+            let [div, cdata] = await createXHTMLCase(3);
+            assert_true(div.matches(":dir(ltr)"));
+            cdata.textContent = "اختبر";
+            assert_true(div.matches(":dir(rtl)"));
+        }, "Directionality is updated when changing text of CDATASection");
+    </script>
+</body>
+</html>

testing/web-platform/tests/html/dom/elements/global-attributes/cdata-iframe.xhtml

@@ -0,0 +1,32 @@
+<?xml version="1.0"?>
+<html xmlns="http://www.w3.org/1999/xhtml">
+    <body>
+        <div id="container"></div>
+    </body>
+
+    <script>
+        function createXHTMLCase(id) {
+            const container = document.getElementById("container");
+
+            const div = document.createElement("div");
+            div.dir = "auto";
+            div.id = id;
+
+            const cdata = document.createCDATASection("foo");
+            const text = document.createTextNode("اختبر");
+            div.appendChild(cdata);
+            div.appendChild(text);
+
+            container.appendChild(div);
+
+            return [div, cdata];
+        }
+
+        window.addEventListener("message", (e) => {
+            createXHTMLCase(e.data);
+            window.top.postMessage(e.data);
+        });
+
+        window.top.postMessage("subframe-loaded");
+    </script>
+</html>