Move WalkDescendantsClearAncestorDirAuto from BeforeSetAttr to OnSetDirAttr to simplify the test for elements that had dir=auto but no longer do, especially <bdi>. Bug 845093, r=ehsan, a=bajaj
authorSimon Montagu <smontagu@smontagu.org>
Thu, 07 Mar 2013 22:53:08 +0200
changeset 132381 9a2cea1ca6781cac93dcd92192f6f1b172297a90
parent 132380 3c95ca69e1d7a24a7dba7dfff4f3b79b7f386117
child 132382 5d46a43cf183184a89148d5e4c7b695c6e1ebaf4
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan, bajaj
bugs845093
milestone21.0a2
Move WalkDescendantsClearAncestorDirAuto from BeforeSetAttr to OnSetDirAttr to simplify the test for elements that had dir=auto but no longer do, especially <bdi>. Bug 845093, r=ehsan, a=bajaj
content/base/public/DirectionalityUtils.h
content/base/src/DirectionalityUtils.cpp
content/base/src/Element.cpp
content/html/content/src/nsGenericHTMLElement.h
--- a/content/base/public/DirectionalityUtils.h
+++ b/content/base/public/DirectionalityUtils.h
@@ -112,16 +112,17 @@ void SetDirectionalityFromValue(mozilla:
  * in AfterSetAttr we don't know the old value, so we can't identify all cases
  * where we need to walk up or down the document tree and reset the direction;
  * and in BeforeSetAttr we can't do the walk because this element hasn't had the
  * value set yet so the results will be wrong.
  */
 void OnSetDirAttr(mozilla::dom::Element* aElement,
                   const nsAttrValue* aNewValue,
                   bool hadValidDir,
+                  bool hadDirAuto,
                   bool aNotify);
 
 /**
  * Called when binding a new element to the tree, to set the
  * NodeAncestorHasDirAuto flag and set the direction of the element and its
  * ancestors if necessary
  */
 void SetDirOnBind(mozilla::dom::Element* aElement, nsIContent* aParent);
--- a/content/base/src/DirectionalityUtils.cpp
+++ b/content/base/src/DirectionalityUtils.cpp
@@ -853,17 +853,17 @@ SetDirectionalityFromValue(Element* aEle
     dir = eDir_LTR;
   }
 
   aElement->SetDirectionality(dir, aNotify);
 }
 
 void
 OnSetDirAttr(Element* aElement, const nsAttrValue* aNewValue,
-             bool hadValidDir, bool aNotify)
+             bool hadValidDir, bool hadDirAuto, bool aNotify)
 {
   if (aElement->IsHTML(nsGkAtoms::input)) {
     return;
   }
 
   if (aElement->AncestorHasDirAuto()) {
     if (!hadValidDir) {
       // The element is a descendant of an element with dir = auto, is
@@ -874,16 +874,30 @@ OnSetDirAttr(Element* aElement, const ns
       WalkDescendantsResetAutoDirection(aElement);
     } else if (!aElement->HasValidDir()) {
       // The element is a descendant of an element with dir = auto and is
       // having its dir attribute removed or set to an invalid value.
       // Reset the direction of any of its ancestors whose direction is
       // determined by a text node descendant
       WalkAncestorsResetAutoDirection(aElement, aNotify);
     }
+  } else if (hadDirAuto && !aElement->HasDirAuto()) {
+    // The element isn't a descendant of an element with dir = auto, and is
+    // having its dir attribute set to something other than auto.
+    // Walk the descendant tree and clear the AncestorHasDirAuto flag.
+    //
+    // N.B: For elements other than <bdi> it would be enough to test that the
+    //      current value of dir was "auto" in BeforeSetAttr to know that we
+    //      were unsetting dir="auto". For <bdi> things are more complicated,
+    //      since it behaves like dir="auto" whenever the dir attribute is
+    //      empty or invalid, so we would have to check whether the old value
+    //      was not either "ltr" or "rtl", and the new value was either "ltr"
+    //      or "rtl". Element::HasDirAuto() encapsulates all that, so doing it
+    //      here is simpler.
+    WalkDescendantsClearAncestorDirAuto(aElement);
   }
 
   if (aElement->HasDirAuto()) {
     WalkDescendantsSetDirAuto(aElement, aNotify);
   } else {
     SetDirectionalityOnDescendants(aElement,
                                    RecomputeDirectionality(aElement, aNotify),
                                    aNotify);
--- a/content/base/src/Element.cpp
+++ b/content/base/src/Element.cpp
@@ -1849,20 +1849,22 @@ Element::SetAttrAndNotify(int32_t aNames
   // Copy aParsedValue for later use since it will be lost when we call
   // SetAndTakeMappedAttr below
   nsAttrValue aValueForAfterSetAttr;
   if (aCallAfterSetAttr) {
     aValueForAfterSetAttr.SetTo(aParsedValue);
   }
 
   bool hadValidDir = false;
+  bool hadDirAuto = false;
 
   if (aNamespaceID == kNameSpaceID_None) {
     if (aName == nsGkAtoms::dir) {
       hadValidDir = HasValidDir() || IsHTML(nsGkAtoms::bdi);
+      hadDirAuto = HasDirAuto(); // already takes bdi into account
     }
 
     // XXXbz Perhaps we should push up the attribute mapping function
     // stuff to Element?
     if (!IsAttributeMapped(aName) ||
         !SetMappedAttribute(document, aName, aParsedValue, &rv)) {
       rv = mAttrsAndChildren.SetAndTakeAttr(aName, aParsedValue);
     }
@@ -1892,17 +1894,18 @@ Element::SetAttrAndNotify(int32_t aNames
     nsNodeUtils::AttributeChanged(this, aNamespaceID, aName, aModType);
   }
 
   if (aCallAfterSetAttr) {
     rv = AfterSetAttr(aNamespaceID, aName, &aValueForAfterSetAttr, aNotify);
     NS_ENSURE_SUCCESS(rv, rv);
 
     if (aNamespaceID == kNameSpaceID_None && aName == nsGkAtoms::dir) {
-      OnSetDirAttr(this, &aValueForAfterSetAttr, hadValidDir, aNotify);
+      OnSetDirAttr(this, &aValueForAfterSetAttr,
+                   hadValidDir, hadDirAuto, aNotify);
     }
   }
 
   if (aFireMutation) {
     nsMutationEvent mutation(true, NS_MUTATION_ATTRMODIFIED);
 
     nsAutoString ns;
     nsContentUtils::NameSpaceManager()->GetNameSpaceURI(aNamespaceID, ns);
@@ -2047,19 +2050,21 @@ Element::UnsetAttr(int32_t aNameSpaceID,
     slots->mAttributeMap->DropAttribute(aNameSpaceID, aName);
   }
 
   // The id-handling code, and in the future possibly other code, need to
   // react to unexpected attribute changes.
   nsMutationGuard::DidMutate();
 
   bool hadValidDir = false;
+  bool hadDirAuto = false;
 
   if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::dir) {
     hadValidDir = HasValidDir() || IsHTML(nsGkAtoms::bdi);
+    hadDirAuto = HasDirAuto(); // already takes bdi into account
   }
 
   nsAttrValue oldValue;
   rv = mAttrsAndChildren.RemoveAttrAt(index, oldValue);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (document || HasFlag(NODE_FORCE_XBL_BINDINGS)) {
     nsRefPtr<nsXBLBinding> binding =
@@ -2075,17 +2080,17 @@ Element::UnsetAttr(int32_t aNameSpaceID,
     nsNodeUtils::AttributeChanged(this, aNameSpaceID, aName,
                                   nsIDOMMutationEvent::REMOVAL);
   }
 
   rv = AfterSetAttr(aNameSpaceID, aName, nullptr, aNotify);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::dir) {
-    OnSetDirAttr(this, nullptr, hadValidDir, aNotify);
+    OnSetDirAttr(this, nullptr, hadValidDir, hadDirAuto, aNotify);
   }
 
   if (hasMutationListeners) {
     nsCOMPtr<nsIDOMEventTarget> node = do_QueryObject(this);
     nsMutationEvent mutation(true, NS_MUTATION_ATTRMODIFIED);
 
     mutation.mRelatedNode = attrNode;
     mutation.mAttrName = aName;
--- a/content/html/content/src/nsGenericHTMLElement.h
+++ b/content/html/content/src/nsGenericHTMLElement.h
@@ -763,20 +763,16 @@ protected:
       RegUnRegAccessKey(false);
     }
   }
 
 private:
   void RegUnRegAccessKey(bool aDoReg);
 
 protected:
-  virtual nsresult BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName,
-                                 const nsAttrValueOrString* aValue,
-                                 bool aNotify);
-
   virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName,
                                 const nsAttrValue* aValue, bool aNotify);
 
   virtual nsEventListenerManager*
     GetEventListenerManagerForAttr(nsIAtom* aAttrName, bool* aDefer);
 
   virtual const nsAttrName* InternalGetExistingAttrNameFromQName(const nsAString& aStr) const;