Bug 1469108: Propagate directionality to the shadow tree. r?smaug draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 15 Jun 2018 19:16:16 -0700
changeset 807918 d71d0e5f7b5b99049cba43b0462a356f9de80e3e
parent 807917 1c5e0a39b2246110e047d0771cc0820fdaa0882d
push id113242
push userbmo:emilio@crisal.io
push dateSat, 16 Jun 2018 02:39:16 +0000
reviewerssmaug
bugs1469108
milestone62.0a1
Bug 1469108: Propagate directionality to the shadow tree. r?smaug Make it so that directionality of the ShadowRoot descendants is computed based on the host and it's ancestors, but don't touch the dir=auto code, since I don't think anybody agrees with what needs to happen there, and I think in general it shouldn't be accounted for. MozReview-Commit-ID: AZMBZ5m1SQf
dom/base/DirectionalityUtils.cpp
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/css/css-scoping/shadow-directionality-001.tentative.html
testing/web-platform/tests/shadow-dom/directionality-001-ref.html
testing/web-platform/tests/shadow-dom/directionality-001.tentative.html
--- a/dom/base/DirectionalityUtils.cpp
+++ b/dom/base/DirectionalityUtils.cpp
@@ -208,25 +208,27 @@
 
 #include "nsINode.h"
 #include "nsIContent.h"
 #include "nsIContentInlines.h"
 #include "nsIDocument.h"
 #include "mozilla/AutoRestore.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/dom/Element.h"
+#include "mozilla/dom/ShadowRoot.h"
 #include "nsUnicodeProperties.h"
 #include "nsTextFragment.h"
 #include "nsAttrValue.h"
 #include "nsTextNode.h"
 #include "nsCheapSets.h"
 
 namespace mozilla {
 
 using mozilla::dom::Element;
+using mozilla::dom::ShadowRoot;
 
 /**
  * Returns true if aElement is one of the elements whose text content should not
  * affect its own direction, nor the direction of ancestors with dir=auto.
  *
  * Note that this does not include <bdi>, whose content does affect its own
  * direction when it has dir=auto (which it has by default), so one needs to
  * test for it separately, e.g. with DoesNotAffectDirectionOfAncestors.
@@ -271,16 +273,18 @@ GetDirectionFromChar(uint32_t ch)
     case eCharType_LeftToRight:
       return eDir_LTR;
 
     default:
       return eDir_NotSet;
   }
 }
 
+// FIXME(bug 1100912): Should ShadowRoot children affect the host if it's
+// dir=auto? Probably not at least in closed mode.
 inline static bool
 NodeAffectsDirAutoAncestor(nsINode* aTextNode)
 {
   Element* parent = aTextNode->GetParentElement();
   return (parent &&
           !DoesNotParticipateInAutoDirection(parent) &&
           parent->NodeOrAncestorHasDirAuto() &&
           !aTextNode->IsInAnonymousSubtree());
@@ -620,61 +624,79 @@ public:
 };
 
 Directionality
 RecomputeDirectionality(Element* aElement, bool aNotify)
 {
   MOZ_ASSERT(!aElement->HasDirAuto(),
              "RecomputeDirectionality called with dir=auto");
 
-  Directionality dir = eDir_LTR;
+  if (aElement->HasValidDir()) {
+    return aElement->GetDirectionality();
+  }
 
-  if (aElement->HasValidDir()) {
-    dir = aElement->GetDirectionality();
-  } else {
-    Element* parent = aElement->GetParentElement();
-    if (parent) {
-      // If the element doesn't have an explicit dir attribute with a valid
-      // value, the directionality is the same as the parent element (but
-      // don't propagate the parent directionality if it isn't set yet).
-      Directionality parentDir = parent->GetDirectionality();
+  Directionality dir = eDir_LTR;
+  if (nsINode* parent = aElement->GetParentNode()) {
+    if (ShadowRoot* shadow = ShadowRoot::FromNode(parent)) {
+      parent = shadow->GetHost();
+    }
+
+    if (parent && parent->IsElement()) {
+      // If the node doesn't have an explicit dir attribute with a valid value,
+      // the directionality is the same as the parent element (but don't propagate
+      // the parent directionality if it isn't set yet).
+      Directionality parentDir = parent->AsElement()->GetDirectionality();
       if (parentDir != eDir_NotSet) {
         dir = parentDir;
       }
-    } else {
-      // If there is no parent element and no dir attribute, the directionality
-      // is LTR.
-      dir = eDir_LTR;
     }
+  }
 
-    aElement->SetDirectionality(dir, aNotify);
-  }
+  aElement->SetDirectionality(dir, aNotify);
   return dir;
 }
 
-void
-SetDirectionalityOnDescendants(Element* aElement, Directionality aDir,
-                               bool aNotify)
+static void
+SetDirectionalityOnDescendantsInternal(nsINode* aNode,
+                                       Directionality aDir,
+                                       bool aNotify)
 {
-  for (nsIContent* child = aElement->GetFirstChild(); child; ) {
+  if (Element* element = Element::FromNode(aNode)) {
+    if (ShadowRoot* shadow = element->GetShadowRoot()) {
+      SetDirectionalityOnDescendantsInternal(shadow, aDir, aNotify);
+    }
+  }
+
+  for (nsIContent* child = aNode->GetFirstChild(); child; ) {
     if (!child->IsElement()) {
-      child = child->GetNextNode(aElement);
+      child = child->GetNextNode(aNode);
       continue;
     }
 
     Element* element = child->AsElement();
     if (element->HasValidDir() || element->HasDirAuto()) {
-      child = child->GetNextNonChildNode(aElement);
+      child = child->GetNextNonChildNode(aNode);
       continue;
     }
+    if (ShadowRoot* shadow = element->GetShadowRoot()) {
+      SetDirectionalityOnDescendantsInternal(shadow, aDir, aNotify);
+    }
     element->SetDirectionality(aDir, aNotify);
-    child = child->GetNextNode(aElement);
+    child = child->GetNextNode(aNode);
   }
 }
 
+// We want the public version of this only to acc
+void
+SetDirectionalityOnDescendants(Element* aElement, Directionality aDir,
+                               bool aNotify)
+{
+  return SetDirectionalityOnDescendantsInternal(aElement, aDir, aNotify);
+}
+
 /**
  * Walk the parent chain of a text node whose dir attribute has been removed and
  * reset the direction of any of its ancestors which have dir=auto and whose
  * directionality is determined by a text node descendant.
  */
 void
 WalkAncestorsResetAutoDirection(Element* aElement, bool aNotify)
 {
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -129326,16 +129326,28 @@
       [
        "/css/css-scoping/reference/green-box.html",
        "=="
       ]
      ],
      {}
     ]
    ],
+   "css/css-scoping/shadow-directionality-001.tentative.html": [
+    [
+     "/css/css-scoping/shadow-directionality-001.tentative.html",
+     [
+      [
+       "/css/css-scoping/reference/green-box.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
    "css/css-scoping/shadow-disabled-sheet-001.html": [
     [
      "/css/css-scoping/shadow-disabled-sheet-001.html",
      [
       [
        "/css/css-scoping/reference/green-box.html",
        "=="
       ]
@@ -182910,16 +182922,28 @@
       [
        "/service-workers/service-worker/resources/svg-target-reftest-001.html",
        "=="
       ]
      ],
      {}
     ]
    ],
+   "shadow-dom/directionality-001.tentative.html": [
+    [
+     "/shadow-dom/directionality-001.tentative.html",
+     [
+      [
+       "/shadow-dom/directionality-001-ref.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
    "shadow-dom/layout-slot-no-longer-assigned.html": [
     [
      "/shadow-dom/layout-slot-no-longer-assigned.html",
      [
       [
        "/shadow-dom/reference/empty.html",
        "=="
       ]
@@ -295381,16 +295405,21 @@
      {}
     ]
    ],
    "shadow-dom/OWNERS": [
     [
      {}
     ]
    ],
+   "shadow-dom/directionality-001-ref.html": [
+    [
+     {}
+    ]
+   ],
    "shadow-dom/reference/empty.html": [
     [
      {}
     ]
    ],
    "shadow-dom/resources/Document-prototype-currentScript-helper.js": [
     [
      {}
@@ -522924,16 +522953,20 @@
   "css/css-scoping/shadow-at-import.html": [
    "67295000ad3c24c2d9ab0ac556d34758f3ce654c",
    "reftest"
   ],
   "css/css-scoping/shadow-cascade-order-001.html": [
    "46913ea7e47811b11be898de5c3bd0a330ea6637",
    "testharness"
   ],
+  "css/css-scoping/shadow-directionality-001.tentative.html": [
+   "dd9bf69af6aa22cb89c0efb0ecbf8ed60433f923",
+   "reftest"
+  ],
   "css/css-scoping/shadow-disabled-sheet-001.html": [
    "3de2d23c1b3339b964ec2c009832a3207a3b9dc4",
    "reftest"
   ],
   "css/css-scoping/shadow-fallback-dynamic-001.html": [
    "741cd9e29067a4634aa5beb6bd06afa540895d22",
    "reftest"
   ],
@@ -522965,17 +522998,17 @@
    "2a90e0623a99cfb46430f0236ceea44f93a25131",
    "reftest"
   ],
   "css/css-scoping/shadow-root-insert-into-document.html": [
    "2cee9fff35c9222074f4ef78dcfcb8a3e02bbc98",
    "reftest"
   ],
   "css/css-scoping/slot-non-html-display-value.html": [
-   "e5d57d25feeccd376116a527c401d570ec1ac1b4",
+   "924373f6b99c9ba4773c6e01d0318ceebff0b35e",
    "testharness"
   ],
   "css/css-scoping/slotted-invalidation.html": [
    "c500e1ceba1b293d45df5f66fd89d4a5d9ceb952",
    "testharness"
   ],
   "css/css-scoping/slotted-link.html": [
    "3e1eb6676da866393e6963f0a44377ea3967c0b9",
@@ -547341,17 +547374,17 @@
    "c9ed57c7ef7a035c25feff4ea60547a57d727f31",
    "testharness"
   ],
   "css/cssom/font-shorthand-serialization.html": [
    "9d0bce1c0d74bf90aca1eb8ee6aa2e2ed2b92b30",
    "testharness"
   ],
   "css/cssom/getComputedStyle-detached-subtree.html": [
-   "01978ca7ea08cbf61b28e9d77753fe5852bcbff9",
+   "886f72b4eaa82d3aeb4de5c5b27f71369dbe0186",
    "testharness"
   ],
   "css/cssom/getComputedStyle-dynamic-subdoc.html": [
    "3f379487727d9730de9e3569b26632c35d602d9d",
    "testharness"
   ],
   "css/cssom/getComputedStyle-pseudo.html": [
    "d3ef09fb6092078562f8923879b9ece97938df47",
@@ -548377,17 +548410,17 @@
    "7cad13aa87e8a0d95dc9e35eb7d1dec7930939fb",
    "reftest"
   ],
   "css/mediaqueries/mq-calc-005.html": [
    "75334bbcf4ef412b9976c59e1efe2177ad62b465",
    "reftest"
   ],
   "css/mediaqueries/mq-case-insensitive-001.html": [
-   "5d94915b19757b3ee5ac49fb2fd119c82317ddf3",
+   "33087ab04eea830e3fcf10820fbfd167709bd18a",
    "reftest"
   ],
   "css/mediaqueries/mq-invalid-media-type-001.html": [
    "4b11afa3270c95b0a2736f114627b6f02346805a",
    "reftest"
   ],
   "css/mediaqueries/mq-invalid-media-type-002.html": [
    "42760d383b11e870f663e11624c5de8d7dfaa1ec",
@@ -610356,16 +610389,24 @@
   "shadow-dom/ShadowRoot-interface.html": [
    "8c39afc1c5648c3e95fe69c4ea5003958f7734b7",
    "testharness"
   ],
   "shadow-dom/Slotable-interface.html": [
    "61f7da763fa4eb6f21077868caf0a07a4a9e44ae",
    "testharness"
   ],
+  "shadow-dom/directionality-001-ref.html": [
+   "cebc0bb99317742ebb6b623c4d777bc60767e26f",
+   "support"
+  ],
+  "shadow-dom/directionality-001.tentative.html": [
+   "763fd90e8ed83fb616379997735a5c283f0bd869",
+   "reftest"
+  ],
   "shadow-dom/event-composed-path-after-dom-mutation.html": [
    "69ea3efc8230a0ed31968f24379289c6691d77d1",
    "testharness"
   ],
   "shadow-dom/event-composed-path-with-related-target.html": [
    "a4c58227f937a943b3845ed3f672419b62a8caad",
    "testharness"
   ],
@@ -618665,17 +618706,17 @@
    "21bae43b3a6e966b8698b7c439b29a68029adc58",
    "wdspec"
   ],
   "webdriver/tests/execute_script/json_serialize_windowproxy.py": [
    "20db10d82ed2b28a22674fcdc37cac0323d33c95",
    "wdspec"
   ],
   "webdriver/tests/execute_script/user_prompts.py": [
-   "d87cc66a9d1e0003a3973b86e541b62999e9975f",
+   "4bd4e9089185505d8add4d5ebe9806498da42999",
    "wdspec"
   ],
   "webdriver/tests/find_element/__init__.py": [
    "da39a3ee5e6b4b0d3255bfef95601890afd80709",
    "support"
   ],
   "webdriver/tests/find_element/find.py": [
    "9af0db4de0d09cbf68fa43bb40145cffc7b95635",
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-scoping/shadow-directionality-001.tentative.html
@@ -0,0 +1,34 @@
+<!doctype html>
+<title>CSS Test: directionality propagation in Shadow DOM.</title>
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="help" href="https://html.spec.whatwg.org/#the-dir-attribute">
+<link rel="help" href="https://github.com/whatwg/html/issues/3699">
+<link rel="match" href="reference/green-box.html">
+<style>
+  div { width: 100px; }
+</style>
+<p>Test passes if you see a single 100px by 100px green box below.</p>
+<div id="host1"></div>
+<div id="host2" dir="rtl"></div>
+<div id="host3"></div>
+<div id="host4"></div>
+<script>
+  host1.attachShadow({ mode: "open" }).innerHTML = `
+    <style>:dir(ltr) { background: green; height: 25px; }</style>
+    <div></div>
+  `;
+  host2.attachShadow({ mode: "open" }).innerHTML = `
+    <style>:dir(rtl) { background: green; height: 25px; }</style>
+    <div></div>
+  `;
+  host3.attachShadow({ mode: "open" }).innerHTML = `
+    <style>:dir(rtl) { background: green; height: 25px; }</style>
+    <div></div>
+  `;
+  host4.attachShadow({ mode: "open" }).innerHTML = `
+    <style>span:dir(ltr) { display: block; background: green; height: 25px; }</style>
+    <div dir="ltr"><span></span></div>
+  `;
+  document.body.offsetTop;
+  host3.setAttribute("dir", "rtl");
+</script>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/shadow-dom/directionality-001-ref.html
@@ -0,0 +1,6 @@
+<!doctype html>
+<title>CSS Test Reference</title>
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<div dir="rtl"> 123 456 <span><span> 789 101112 </span></span></div>
+<div dir="rtl"> 123 456 <span dir="ltr"><span> 789 101112 </span></span></div>
+<div dir="rtl"> 123 456 <span><span> 789 101112 </span></span></div>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/shadow-dom/directionality-001.tentative.html
@@ -0,0 +1,19 @@
+<!doctype html>
+<title>Test: directionality propagation in Shadow DOM.</title>
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="help" href="https://html.spec.whatwg.org/#the-dir-attribute">
+<link rel="help" href="https://github.com/whatwg/html/issues/3699">
+<link rel="match" href="directionality-001-ref.html">
+<div id="host0" dir="rtl"><span> 789 101112 </span></div>
+<div id="host1" dir="rtl"><span> 789 101112 </span></div>
+<div id="host2" dir="rtl"><span> 789 101112 </span></div>
+<script>
+  host0.attachShadow({mode: 'closed'}).innerHTML =
+    '<div> 123 456 <span><slot></slot></span></div>';
+
+  host1.attachShadow({mode: 'closed'}).innerHTML =
+    '<div> 123 456 <span dir="ltr"><slot></slot></span></div>';
+
+  host2.attachShadow({mode: 'closed'}).innerHTML =
+    '<div> 123 456 <span><slot dir="ltr"></slot></span></div>';
+</script>