Bug 555627. Make transitions actually work correctly on :before and :after. r=dbaron
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 30 Jun 2010 18:54:29 -0700
changeset 46468 6ce0346e2560f5377dd4ec5432fc666e8c5e5f4a
parent 46467 00dfcf7f4c9eee1b09c632fb45d814e9bea4faa9
child 46469 51b69f8d306f9d8966b04b9338cc8fe0ad3be182
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs555627
milestone2.0b2pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 555627. Make transitions actually work correctly on :before and :after. r=dbaron
content/base/src/nsGenericElement.cpp
content/smil/nsSMILMappedAttribute.cpp
content/svg/content/src/nsSVGElement.cpp
layout/base/nsIPresShell.h
layout/base/nsPresShell.cpp
layout/style/nsHTMLCSSStyleSheet.cpp
layout/style/nsStyleSet.cpp
layout/style/nsTransitionManager.cpp
layout/style/nsTransitionManager.h
layout/style/test/test_transitions.html
--- a/content/base/src/nsGenericElement.cpp
+++ b/content/base/src/nsGenericElement.cpp
@@ -3298,17 +3298,17 @@ nsGenericElement::SetSMILOverrideStyleRu
   if (aNotify) {
     nsIDocument* doc = GetCurrentDoc();
     // Only need to request a restyle if we're in a document.  (We might not
     // be in a document, if we're clearing animation effects on a target node
     // that's been detached since the previous animation sample.)
     if (doc) {
       nsCOMPtr<nsIPresShell> shell = doc->GetShell();
       if (shell) {
-        shell->RestyleForAnimation(this);
+        shell->RestyleForAnimation(this, eRestyle_Self);
       }
     }
   }
 
   return NS_OK;
 }
 #endif // MOZ_SMIL
 
--- a/content/smil/nsSMILMappedAttribute.cpp
+++ b/content/smil/nsSMILMappedAttribute.cpp
@@ -154,17 +154,17 @@ nsSMILMappedAttribute::FlushChangesToTar
   mElement->DeleteProperty(SMIL_MAPPED_ATTR_ANIMVAL,
                            SMIL_MAPPED_ATTR_STYLERULE_ATOM);
   nsIDocument* doc = mElement->GetCurrentDoc();
 
   // Request animation restyle
   if (doc) {
     nsIPresShell* shell = doc->GetShell();
     if (shell) {
-      shell->RestyleForAnimation(mElement);
+      shell->RestyleForAnimation(mElement, eRestyle_Self);
     }
   }
 }
 
 already_AddRefed<nsIAtom>
 nsSMILMappedAttribute::GetAttrNameAtom() const
 {
   return do_GetAtom(nsCSSProps::GetStringValue(mPropID));
--- a/content/svg/content/src/nsSVGElement.cpp
+++ b/content/svg/content/src/nsSVGElement.cpp
@@ -761,17 +761,17 @@ nsSVGElement::WalkContentStyleRules(nsRu
     nsIPresShell* shell = doc->GetShell();
     nsPresContext* context = shell ? shell->GetPresContext() : nsnull;
     if (context && context->IsProcessingRestyles() &&
         !context->IsProcessingAnimationStyleChange()) {
       // Any style changes right now could trigger CSS Transitions. We don't
       // want that to happen from SMIL-animated value of mapped attrs, so
       // ignore animated value for now, and request an animation restyle to
       // get our animated value noticed.
-      shell->RestyleForAnimation(this);
+      shell->RestyleForAnimation(this, eRestyle_Self);
     } else {
       // Ok, this is an animation restyle -- go ahead and update/walk the
       // animated content style rule.
       nsICSSStyleRule* animContentStyleRule = GetAnimatedContentStyleRule();
       if (!animContentStyleRule) {
         UpdateAnimatedContentStyleRule();
         animContentStyleRule = GetAnimatedContentStyleRule();
       }
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -63,16 +63,17 @@
 #include "nsColor.h"
 #include "nsEvent.h"
 #include "nsCompatibility.h"
 #include "nsFrameManagerBase.h"
 #include "mozFlushType.h"
 #include "nsWeakReference.h"
 #include <stdio.h> // for FILE definition
 #include "nsRefreshDriver.h"
+#include "nsChangeHint.h"
 
 class nsIContent;
 class nsIDocument;
 class nsIFrame;
 class nsPresContext;
 class nsStyleSet;
 class nsIViewManager;
 class nsIRenderingContext;
@@ -429,17 +430,18 @@ public:
   virtual NS_HIDDEN_(void) CancelAllPendingReflows() = 0;
 
   /**
    * Recreates the frames for a node
    */
   virtual NS_HIDDEN_(nsresult) RecreateFramesFor(nsIContent* aContent) = 0;
 
   void PostRecreateFramesFor(mozilla::dom::Element* aElement);
-  void RestyleForAnimation(mozilla::dom::Element* aElement);
+  void RestyleForAnimation(mozilla::dom::Element* aElement,
+                           nsRestyleHint aHint);
 
   /**
    * Determine if it is safe to flush all pending notifications
    * @param aIsSafeToFlush PR_TRUE if it is safe, PR_FALSE otherwise.
    * 
    */
   virtual NS_HIDDEN_(PRBool) IsSafeToFlush() const = 0;
 
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@@ -3628,24 +3628,19 @@ PresShell::RecreateFramesFor(nsIContent*
 void
 nsIPresShell::PostRecreateFramesFor(Element* aElement)
 {
   FrameConstructor()->PostRestyleEvent(aElement, nsRestyleHint(0),
                                        nsChangeHint_ReconstructFrame);
 }
 
 void
-nsIPresShell::RestyleForAnimation(Element* aElement)
-{
-  // eRestyle_Self is ok here because animations are always tied to a
-  // particular element and don't directly affect its kids.  The kids
-  // might have animations of their own, or inherit from aElement, but
-  // we handle all that during restyling; we don't need to _force_
-  // animation rule matching on the kids here.
-  FrameConstructor()->PostAnimationRestyleEvent(aElement, eRestyle_Self,
+nsIPresShell::RestyleForAnimation(Element* aElement, nsRestyleHint aHint)
+{
+  FrameConstructor()->PostAnimationRestyleEvent(aElement, aHint,
                                                 NS_STYLE_HINT_NONE);
 }
 
 void
 PresShell::ClearFrameRefs(nsIFrame* aFrame)
 {
   mPresContext->EventStateManager()->ClearFrameRefs(aFrame);
 
--- a/layout/style/nsHTMLCSSStyleSheet.cpp
+++ b/layout/style/nsHTMLCSSStyleSheet.cpp
@@ -83,17 +83,18 @@ nsHTMLCSSStyleSheet::RulesMatching(Eleme
 #ifdef MOZ_SMIL
   rule = element->GetSMILOverrideStyleRule();
   if (rule) {
     if (aData->mPresContext->IsProcessingRestyles() &&
         !aData->mPresContext->IsProcessingAnimationStyleChange()) {
       // Non-animation restyle -- don't process SMIL override style, because we
       // don't want SMIL animation to trigger new CSS transitions. Instead,
       // request an Animation restyle, so we still get noticed.
-      aData->mPresContext->PresShell()->RestyleForAnimation(element);
+      aData->mPresContext->PresShell()->RestyleForAnimation(element,
+                                                            eRestyle_Self);
     } else {
       // Animation restyle (or non-restyle traversal of rules)
       // Now we can walk SMIL overrride style, without triggering transitions.
       rule->RuleMatched();
       aData->mRuleWalker->Forward(rule);
     }
   }
 #endif // MOZ_SMIL
--- a/layout/style/nsStyleSet.cpp
+++ b/layout/style/nsStyleSet.cpp
@@ -1130,28 +1130,30 @@ nsStyleSet::GCRuleTrees()
       mOldRuleTrees.RemoveElementAt(i);
     } else {
       NS_NOTREACHED("old rule tree still referenced");
     }
   }
 }
 
 static inline nsRuleNode*
-SkipTransitionRules(nsRuleNode* aRuleNode, Element* aElement)
+SkipTransitionRules(nsRuleNode* aRuleNode, Element* aElement, PRBool isPseudo)
 {
   nsRuleNode* ruleNode = aRuleNode;
   while (!ruleNode->IsRoot() &&
          ruleNode->GetLevel() == nsStyleSet::eTransitionSheet) {
     ruleNode = ruleNode->GetParent();
   }
   if (ruleNode != aRuleNode) {
     NS_ASSERTION(aElement, "How can we have transition rules but no element?");
     // Need to do an animation restyle, just like
     // nsTransitionManager::WalkTransitionRule would.
-    aRuleNode->GetPresContext()->PresShell()->RestyleForAnimation(aElement);
+    nsRestyleHint hint = isPseudo ? eRestyle_Subtree : eRestyle_Self;
+    aRuleNode->GetPresContext()->PresShell()->RestyleForAnimation(aElement,
+                                                                  hint);
   }
   return ruleNode;
 }
 
 already_AddRefed<nsStyleContext>
 nsStyleSet::ReparentStyleContext(nsStyleContext* aStyleContext,
                                  nsStyleContext* aNewParentContext,
                                  Element* aElement)
@@ -1174,30 +1176,36 @@ nsStyleSet::ReparentStyleContext(nsStyle
 
   // Skip transition rules as needed just like
   // nsTransitionManager::WalkTransitionRule would.
   PRBool skipTransitionRules = PresContext()->IsProcessingRestyles() &&
     !PresContext()->IsProcessingAnimationStyleChange();
   if (skipTransitionRules) {
     // Make sure that we're not using transition rules for our new style
     // context.  If we need them, an animation restyle will provide.
-    ruleNode = SkipTransitionRules(ruleNode, aElement);
+    ruleNode =
+      SkipTransitionRules(ruleNode, aElement,
+                          pseudoType !=
+                            nsCSSPseudoElements::ePseudo_NotPseudoElement);
   }
 
   nsRuleNode* visitedRuleNode = nsnull;
   nsStyleContext* visitedContext = aStyleContext->GetStyleIfVisited();
   // Reparenting a style context just changes where we inherit from,
   // not what rules we match or what our DOM looks like.  In
   // particular, it doesn't change whether this is a style context for
   // a link.
   if (visitedContext) {
      visitedRuleNode = visitedContext->GetRuleNode();
      // Again, skip transition rules as needed
      if (skipTransitionRules) {
-       visitedRuleNode = SkipTransitionRules(visitedRuleNode, aElement);
+       visitedRuleNode =
+         SkipTransitionRules(visitedRuleNode, aElement,
+                             pseudoType !=
+                               nsCSSPseudoElements::ePseudo_NotPseudoElement);
      }
   }
 
   return GetContext(aNewParentContext, ruleNode, visitedRuleNode,
                     aStyleContext->IsLinkContext(),
                     aStyleContext->RelevantLinkVisited(),
                     pseudoTag, pseudoType);
 }
--- a/layout/style/nsTransitionManager.cpp
+++ b/layout/style/nsTransitionManager.cpp
@@ -1,8 +1,9 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set shiftwidth=2 tabstop=8 autoindent cindent expandtab: */
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
  * The contents of this file are subject to the Mozilla Public License Version
  * 1.1 (the "License"); you may not use this file except in compliance with
  * the License. You may obtain a copy of the License at
  * http://www.mozilla.org/MPL/
@@ -404,35 +405,47 @@ nsTransitionManager::StyleContextChanged
   // NOTE: Things in this function (and ConsiderStartingTransition)
   // should never call PeekStyleData because we don't preserve gotten
   // structs across reframes.
 
   // Return sooner (before the startedAny check below) for the most
   // common case: no transitions specified or running.
   const nsStyleDisplay *disp = aNewStyleContext->GetStyleDisplay();
   nsCSSPseudoElements::Type pseudoType = aNewStyleContext->GetPseudoType();
+  if (pseudoType != nsCSSPseudoElements::ePseudo_NotPseudoElement) {
+    if (pseudoType != nsCSSPseudoElements::ePseudo_before &&
+        pseudoType != nsCSSPseudoElements::ePseudo_after) {
+      return nsnull;
+    }
+
+    NS_ASSERTION((pseudoType == nsCSSPseudoElements::ePseudo_before &&
+                  aElement->Tag() == nsGkAtoms::mozgeneratedcontentbefore) ||
+                 (pseudoType == nsCSSPseudoElements::ePseudo_after &&
+                  aElement->Tag() == nsGkAtoms::mozgeneratedcontentafter),
+                 "Unexpected aElement coming through");
+
+    // Else the element we want to use from now on is the element the
+    // :before or :after is attached to.
+    aElement = aElement->GetParent()->AsElement();
+  }
+
   ElementTransitions *et =
       GetElementTransitions(aElement, pseudoType, PR_FALSE);
   if (!et &&
       disp->mTransitionPropertyCount == 1 &&
       disp->mTransitions[0].GetDelay() == 0.0f &&
       disp->mTransitions[0].GetDuration() == 0.0f) {
     return nsnull;
   }      
 
 
   if (aNewStyleContext->PresContext()->IsProcessingAnimationStyleChange()) {
     return nsnull;
   }
   
-  if (pseudoType != nsCSSPseudoElements::ePseudo_NotPseudoElement &&
-      pseudoType != nsCSSPseudoElements::ePseudo_before &&
-      pseudoType != nsCSSPseudoElements::ePseudo_after) {
-    return nsnull;
-  }
   if (aNewStyleContext->GetParent() &&
       aNewStyleContext->GetParent()->HasPseudoElementData()) {
     // Ignore transitions on things that inherit properties from
     // pseudo-elements.
     // FIXME (Bug 522599): Add tests for this.
     return nsnull;
   }
 
@@ -734,17 +747,21 @@ nsTransitionManager::ConsiderStartingTra
     pts[currentIndex] = pt;
   } else {
     if (!pts.AppendElement(pt)) {
       NS_WARNING("out of memory");
       return;
     }
   }
 
-  presContext->PresShell()->RestyleForAnimation(aElement);
+  nsRestyleHint hint =
+    aNewStyleContext->GetPseudoType() ==
+      nsCSSPseudoElements::ePseudo_NotPseudoElement ?
+    eRestyle_Self : eRestyle_Subtree;
+  presContext->PresShell()->RestyleForAnimation(aElement, hint);
 
   *aStartedAny = PR_TRUE;
   aWhichStarted->AddProperty(aProperty);
 }
 
 ElementTransitions*
 nsTransitionManager::GetElementTransitions(dom::Element *aElement,
                                            nsCSSPseudoElements::Type aPseudoType,
@@ -818,26 +835,30 @@ nsTransitionManager::WalkTransitionRule(
                                         nsCSSPseudoElements::Type aPseudoType)
 {
   ElementTransitions *et =
     GetElementTransitions(aData->mElement, aPseudoType, PR_FALSE);
   if (!et) {
     return NS_OK;
   }
 
-  if (!aData->mPresContext->IsProcessingAnimationStyleChange()) {
+  if (aData->mPresContext->IsProcessingRestyles() &&
+      !aData->mPresContext->IsProcessingAnimationStyleChange()) {
     // If we're processing a normal style change rather than one from
     // animation, don't add the transition rule.  This allows us to
     // compute the new style value rather than having the transition
     // override it, so that we can start transitioning differently.
 
     // We need to immediately restyle with animation
     // after doing this.
     if (et) {
-      mPresContext->PresShell()->RestyleForAnimation(aData->mElement);
+      nsRestyleHint hint =
+        aPseudoType == nsCSSPseudoElements::ePseudo_NotPseudoElement ?
+        eRestyle_Self : eRestyle_Subtree;
+      mPresContext->PresShell()->RestyleForAnimation(aData->mElement, hint);
     }
     return NS_OK;
   }
 
   if (!et->EnsureStyleRuleFor(
         aData->mPresContext->RefreshDriver()->MostRecentRefresh())) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
@@ -979,17 +1000,23 @@ nsTransitionManager::WillRefresh(mozilla
           // to know not to start a new transition for the transition
           // from the almost-completed value to the final value.
           pt.SetRemovedSentinel();
         }
       } while (i != 0);
 
       // We need to restyle even if the transition rule no longer
       // applies (in which case we just made it not apply).
-      mPresContext->PresShell()->RestyleForAnimation(et->mElement);
+      NS_ASSERTION(et->mElementProperty == nsGkAtoms::transitionsProperty ||
+                   et->mElementProperty == nsGkAtoms::transitionsOfBeforeProperty ||
+                   et->mElementProperty == nsGkAtoms::transitionsOfAfterProperty,
+                   "Unexpected element property; might restyle too much");
+      nsRestyleHint hint = et->mElementProperty == nsGkAtoms::transitionsProperty ?
+        eRestyle_Self : eRestyle_Subtree;
+      mPresContext->PresShell()->RestyleForAnimation(et->mElement, hint);
 
       if (et->mPropertyTransitions.IsEmpty()) {
         et->Destroy();
         // |et| is now a dangling pointer!
         et = nsnull;
       }
     }
   }
--- a/layout/style/nsTransitionManager.h
+++ b/layout/style/nsTransitionManager.h
@@ -62,18 +62,20 @@ public:
    * Notify the transition manager that the pres context is going away.
    */
   void Disconnect();
 
   /**
    * StyleContextChanged 
    *
    * To be called from nsFrameManager::ReResolveStyleContext when the
-   * style of an element has changed, to initiate transitions from that
-   * style change.
+   * style of an element has changed, to initiate transitions from
+   * that style change.  For style contexts with :before and :after
+   * pseudos, aElement is expected to be the generated before/after
+   * element.
    *
    * It may return a "cover rule" (see CoverTransitionStartStyleRule) to
    * cover up some of the changes for the duration of the restyling of
    * descendants.  If it does, this function will take care of causing
    * the necessary restyle afterwards, but the caller must restyle the
    * element *again* with the original sequence of rules plus the
    * returned cover rule as the most specific rule.
    */
--- a/layout/style/test/test_transitions.html
+++ b/layout/style/test/test_transitions.html
@@ -6,16 +6,35 @@ https://bugzilla.mozilla.org/show_bug.cg
 <head>
   <title>Test for Bug 435441</title>
   <script type="application/javascript" src="/MochiKit/packed.js"></script>
   <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
   <style type="text/css">
 
   #display p { margin-top: 0; margin-bottom: 0; }
+  #display .before, #display .after {
+    width: -moz-fit-content; border: 1px solid black;
+  }
+  #display .before::before, #display .after::after {
+    display: block;
+    width: 0;
+    text-indent: 0;
+  }
+  #display .before.started::before, #display .after.started::after {
+    width: 100px;
+    text-indent: 100px;
+    -moz-transition: 8s width ease-in-out, 8s text-indent ease-in-out;
+  }
+  #display .before::before {
+    content: "Before";
+  }
+  #display .after::after {
+    content: "After";
+  }
 
   </style>
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=435441">Mozilla Bug 435441</a>
 <div id="display">
 
 </div>
@@ -337,16 +356,29 @@ function make_display_test(initially_non
     div.appendChild(p);
     return p;
 }
 from_none_test   = make_display_test(true,  "transition from display:none");
 to_none_test     = make_display_test(false, "transition to display:none");
 always_none_test = make_display_test(true,  "transition always display:none");
 var display_tests = [ from_none_test, to_none_test, always_none_test ];
 
+// Test transitions on pseudo-elements
+var before_test, after_test;
+function make_pseudo_elem_test(pseudo)
+{
+    var p = document.createElement("p");
+    p.className = pseudo;
+    div.appendChild(p);
+    return {"pseudo": pseudo, element: p};
+}
+before_test = make_pseudo_elem_test("before");
+after_test = make_pseudo_elem_test("after");
+var pseudo_element_tests = [ before_test, after_test ];
+
 // FIXME (Bug 522599): Test a transition that reverses partway through.
 
 var lateref = make_reference_p();
 var laterefcs = getComputedStyle(lateref, "");
 
 // flush style changes
 var x = getComputedStyle(div, "").color;
 
@@ -383,16 +415,20 @@ for (var i in number_tests) {
     var test = number_tests[i];
     test.node.style.marginLeft = "50px";
 }
 from_none_test.style.textIndent = "100px";
 from_none_test.style.display = "";
 to_none_test.style.textIndent = "100px";
 to_none_test.style.display = "none";
 always_none_test.style.textIndent = "100px";
+for (var i in pseudo_element_tests) {
+    var test = pseudo_element_tests[i];
+    test.element.classList.add("started");
+}
 lateref.style.textIndent = "1000px";
 
 // flush style changes
 x = getComputedStyle(div, "").color;
 
 gStartTime2 = Date.now(); // set after all transitions have started
 gCurrentTime = gStartTime2;
 
@@ -680,22 +716,43 @@ function check_display_tests(time)
         var p = display_tests[i];
 
         check_transition_value(tf, 0, 8, 0, 100,
                                getComputedStyle(p, "").textIndent,
                                "display test for test with " +
                                  p.childNodes[0].data,
         // TODO: Making transitions work on 'display:none' elements is
         // still not implemented.
-                               function(range) { return range[1] < 100 });
+                               function(range) { return p != to_none_test &&
+                                                 range[1] < 100 });
     }
 }
 
 check_display_tests(0);
 add_future_call(2, function() { check_display_tests(2); });
 add_future_call(4, function() { check_display_tests(4); });
 add_future_call(6, function() { check_display_tests(6); });
 add_future_call(8, function() { check_display_tests(8); });
 
+function check_pseudo_element_tests(time)
+{
+    var tf = timingFunctions["ease-in-out"];
+    for (var i in pseudo_element_tests) {
+        var test = pseudo_element_tests[i];
+
+        check_transition_value(tf, 0, 8, 0, 100,
+                               getComputedStyle(test.element, "").width,
+                               "::"+test.pseudo+" test");
+        check_transition_value(tf, 0, 8, 0, 100,
+                               getComputedStyle(test.element,
+                                                "::"+test.pseudo).textIndent,
+                               "::"+test.pseudo+" indent test");
+    }
+}
+check_pseudo_element_tests(0);
+add_future_call(2, function() { check_pseudo_element_tests(2); });
+add_future_call(4, function() { check_pseudo_element_tests(4); });
+add_future_call(6, function() { check_pseudo_element_tests(6); });
+add_future_call(8, function() { check_pseudo_element_tests(8); });
 </script>
 </pre>
 </body>
 </html>