Bug 1451576 - Allow dir=auto to work in native anonymous content draft
authorTimothy Guan-tin Chien <timdream@gmail.com>
Tue, 17 Apr 2018 00:17:56 +0800
changeset 788714 0c976fba4a512b9bdd449fda6ab04e9fd7772b78
parent 783966 789e30ff2e3d6e1fcfce1a373c1e5635488d24da
child 788715 0b8b0ce48f01c3ae9441b916f37f318cf1cba068
push id108070
push usertimdream@gmail.com
push dateThu, 26 Apr 2018 21:39:41 +0000
bugs1451576, 839886, 1377826, 1446830
milestone61.0a1
Bug 1451576 - Allow dir=auto to work in native anonymous content The rules set in place in bug 839886 and bug 1377826 prevents any native anonymous content to participate in auto-direction, both on having dir=auto to work on it and have it to affect the direction of non-anonymous dir=auto parent. This patch relax the rules a little bit by allowing the anonymous element with dir=auto to particiate in auto-direction. For simplicity, it would allow the text node in to affact auto-direction only if the text node is the direct children of the dir=auto parent. This patch is needed for HTML-based in-content widget to display RTL labels correctly. It shouldn't change any UI on its own; the purpose of the fix here is to display RTL-text filenames correctly when bug 1446830 is fixed. The change is needed in ResetDirectionSetByTextNode() because when CC clean-up the document, the function is called from nsTextNode::UnbindFromTree(). At that point, IsInAnonymousSubtree() will cause an assertion error on when calling SubtreeRoot(), since subtree hierarchy is already broken by earlier UnbindFormTree() calls to our parent nodes. Substitute that check with HasTextNodeDirectionalityMap() in and only in ResetDirectionSetByTextNode() should be ok -- given the function doesn't really do anything if HasTextNodeDirectionalityMap() is false. There is no need to call EnsureMapIsClearFor() when the condition is false either; EnsureMapIsClearFor() itself is a no-op if the condition is false. MozReview-Commit-ID: GqF5ypDZcbH
dom/base/DirectionalityUtils.cpp
--- a/dom/base/DirectionalityUtils.cpp
+++ b/dom/base/DirectionalityUtils.cpp
@@ -236,30 +236,31 @@ using mozilla::dom::Element;
 static bool
 DoesNotParticipateInAutoDirection(const Element* aElement)
 {
   mozilla::dom::NodeInfo* nodeInfo = aElement->NodeInfo();
   return (!aElement->IsHTMLElement() ||
           nodeInfo->Equals(nsGkAtoms::script) ||
           nodeInfo->Equals(nsGkAtoms::style) ||
           nodeInfo->Equals(nsGkAtoms::textarea) ||
-          aElement->IsInAnonymousSubtree());
+          (aElement->IsInAnonymousSubtree() && !aElement->HasDirAuto()));
 }
 
 /**
  * Returns true if aElement is one of the element whose text content should not
  * affect the direction of ancestors with dir=auto (though it may affect its own
  * direction, e.g. <bdi>)
  */
 static bool
 DoesNotAffectDirectionOfAncestors(const Element* aElement)
 {
   return (DoesNotParticipateInAutoDirection(aElement) ||
           aElement->IsHTMLElement(nsGkAtoms::bdi) ||
-          aElement->HasFixedDir());
+          aElement->HasFixedDir() ||
+          aElement->IsInAnonymousSubtree());
 }
 
 /**
  * Returns the directionality of a Unicode character
  */
 static Directionality
 GetDirectionFromChar(uint32_t ch)
 {
@@ -275,20 +276,24 @@ GetDirectionFromChar(uint32_t ch)
       return eDir_NotSet;
   }
 }
 
 inline static bool
 NodeAffectsDirAutoAncestor(nsINode* aTextNode)
 {
   Element* parent = aTextNode->GetParentElement();
+  // In the anonymous content, we limit our implementation to only
+  // allow the children text node of the direct dir=auto parent in
+  // the same anonymous subtree to affact the direction.
   return (parent &&
           !DoesNotParticipateInAutoDirection(parent) &&
           parent->NodeOrAncestorHasDirAuto() &&
-          !aTextNode->IsInAnonymousSubtree());
+          (!aTextNode->IsInAnonymousSubtree() ||
+            parent->HasDirAuto()));
 }
 
 Directionality
 GetDirectionFromText(const char16_t* aText, const uint32_t aLength,
                      uint32_t* aFirstStrong)
 {
   const char16_t* start = aText;
   const char16_t* end = aText + aLength;
@@ -915,24 +920,29 @@ SetDirectionFromNewTextNode(nsTextNode* 
   if (dir != eDir_NotSet) {
     SetAncestorDirectionIfAuto(aTextNode, dir);
   }
 }
 
 void
 ResetDirectionSetByTextNode(nsTextNode* aTextNode)
 {
-  if (!NodeAffectsDirAutoAncestor(aTextNode)) {
-    nsTextNodeDirectionalityMap::EnsureMapIsClearFor(aTextNode);
+  // We used to check NodeAffectsDirAutoAncestor() in this function, but
+  // stopped doing that since calling IsInAnonymousSubtree()
+  // too late (during nsTextNode::UnbindFromTree) is impossible and this
+  // function was no-op when there's no directionality map.
+  if (!aTextNode->HasTextNodeDirectionalityMap()) {
     return;
   }
 
   Directionality dir = GetDirectionFromText(aTextNode->GetText());
-  if (dir != eDir_NotSet && aTextNode->HasTextNodeDirectionalityMap()) {
+  if (dir != eDir_NotSet) {
     nsTextNodeDirectionalityMap::ResetTextNodeDirection(aTextNode, aTextNode);
+  } else {
+    nsTextNodeDirectionalityMap::EnsureMapIsClearFor(aTextNode);
   }
 }
 
 void
 SetDirectionalityFromValue(Element* aElement, const nsAString& value,
                            bool aNotify)
 {
   Directionality dir = GetDirectionFromText(PromiseFlatString(value).get(),