Title: [142215] trunk
Revision
142215
Author
espr...@chromium.org
Date
2013-02-07 17:40:30 -0800 (Thu, 07 Feb 2013)

Log Message

getComputedStyle() doesn't report intermediate values during a transition of a pseudo element
https://bugs.webkit.org/show_bug.cgi?id=106535

Reviewed by Ojan Vafai.

Source/WebCore:

Element::computedStyle and CSSComputedStyleDeclaration::getPropertyCSSValue
should use the PseudoElement and it's renderer if they exist so that
querying the computed style while an animation is running returns
the intermediate values.

No new tests, updated existing tests.

* css/CSSComputedStyleDeclaration.cpp:
(WebCore::CSSComputedStyleDeclaration::styledNode): Added, returns either the PseudoElement or the Node.
(WebCore::CSSComputedStyleDeclaration::getPropertyCSSValue): Updated to use styledNode.
* css/CSSComputedStyleDeclaration.h:
(CSSComputedStyleDeclaration):
* dom/Element.cpp:
(WebCore::Element::computedStyle): Check the PseudoElement, not just the cached pseudo style.
* dom/ElementRareData.h:
(WebCore::ElementRareData::pseudoElement): Remove ASSERT_NOT_REACHED so passing other pseudos returns 0.

LayoutTests:

Update tests to also check getComputedStyle during animations and transitions.

* fast/css-generated-content/pseudo-animation-expected.txt:
* fast/css-generated-content/pseudo-animation.html:
* fast/css-generated-content/pseudo-transition-expected.txt:
* fast/css-generated-content/pseudo-transition.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (142214 => 142215)


--- trunk/LayoutTests/ChangeLog	2013-02-08 01:28:52 UTC (rev 142214)
+++ trunk/LayoutTests/ChangeLog	2013-02-08 01:40:30 UTC (rev 142215)
@@ -1,3 +1,17 @@
+2013-02-07  Elliott Sprehn  <espr...@chromium.org>
+
+        getComputedStyle() doesn't report intermediate values during a transition of a pseudo element
+        https://bugs.webkit.org/show_bug.cgi?id=106535
+
+        Reviewed by Ojan Vafai.
+
+        Update tests to also check getComputedStyle during animations and transitions.
+
+        * fast/css-generated-content/pseudo-animation-expected.txt:
+        * fast/css-generated-content/pseudo-animation.html:
+        * fast/css-generated-content/pseudo-transition-expected.txt:
+        * fast/css-generated-content/pseudo-transition.html:
+
 2013-02-07  Kent Tamura  <tk...@chromium.org>
 
         [Chromium-Android] Disable input[type=datetime]

Modified: trunk/LayoutTests/fast/css-generated-content/pseudo-animation-expected.txt (142214 => 142215)


--- trunk/LayoutTests/fast/css-generated-content/pseudo-animation-expected.txt	2013-02-08 01:28:52 UTC (rev 142214)
+++ trunk/LayoutTests/fast/css-generated-content/pseudo-animation-expected.txt	2013-02-08 01:40:30 UTC (rev 142215)
@@ -4,11 +4,15 @@
 
 
 PASS div.offsetWidth is 52
-PASS div.offsetWidth is 20
-PASS div.offsetWidth is 12
+PASS div.offsetWidth is within 1 of 20
+PASS computedTop is within 1 of 170
+PASS div.offsetWidth is within 1 of 12
+PASS computedTop is within 1 of 200
 PASS div.offsetWidth is 52
-PASS div.offsetWidth is 20
-PASS div.offsetWidth is 12
+PASS div.offsetWidth is within 1 of 20
+PASS computedTop is within 1 of 170
+PASS div.offsetWidth is within 1 of 12
+PASS computedTop is within 1 of 200
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/fast/css-generated-content/pseudo-animation.html (142214 => 142215)


--- trunk/LayoutTests/fast/css-generated-content/pseudo-animation.html	2013-02-08 01:28:52 UTC (rev 142214)
+++ trunk/LayoutTests/fast/css-generated-content/pseudo-animation.html	2013-02-08 01:40:30 UTC (rev 142215)
@@ -7,10 +7,12 @@
   from {
     width: 50px;
     height: 50px;
+    top: 50px;
   }
   to {
     width: 10px;
     height: 10px;
+    top: 200px;
   }
 }
 
@@ -18,10 +20,12 @@
   from {
     width: 50px;
     height: 50px;
+    top: 50px;
   }
   to {
     width: 10px;
     height: 10px;
+    top: 200px;
   }
 }
 
@@ -31,10 +35,12 @@
     display: block;
     height: 50px;
     width: 50px;
+    position: relative;
 }
 
 #after.animate:after,
 #before.animate:before {
+    top: 200px;
     width: 10px;
     height: 10px;
     -webkit-animation: example 2s;
@@ -64,6 +70,14 @@
 if (window.testRunner)
     testRunner.dumpAsText();
 
+function getPseudoComputedTop(id)
+{
+    return Math.round(parseFloat(getComputedStyle(document.getElementById(id), ':' + id).top));
+}
+
+// FIXME: This test should be modified so subpixel doesn't cause off by one
+// below and it no longer needs shouldBeCloseTo.
+
 function testAnimation(id)
 {
     var div = document.getElementById(id);
@@ -72,19 +86,27 @@
     shouldBe('div.offsetWidth', '52');
     if (window.internals) {
         internals.pauseAnimationAtTimeOnPseudoElement('example', 1.0, div, id);
-        shouldBe('div.offsetWidth', '20');
+        shouldBeCloseTo('div.offsetWidth', 20, 1);
+        computedTop = getPseudoComputedTop(id);
+        shouldBeCloseTo('computedTop', 170, 1);
         internals.pauseAnimationAtTimeOnPseudoElement('example', 2.0, div, id);
-        shouldBe('div.offsetWidth', '12');
+        shouldBeCloseTo('div.offsetWidth', 12, 1);
+        computedTop = getPseudoComputedTop(id);
+        shouldBeCloseTo('computedTop', 200, 1);
     } else {
         // This will be flaky, but it's a reasonable approximation for testing
         // in a browser instead of DRT.
         setTimeout(function() {
             window.div = div;
-            shouldBe('div.offsetWidth', '20');
+            shouldBeCloseTo('div.offsetWidth', 20, 1);
+            computedTop = getPseudoComputedTop(id);
+            shouldBeCloseTo('computedTop', 170, 1);
         }, 1000);
         setTimeout(function() {
             window.div = div;
-            shouldBe('div.offsetWidth', '12');
+            shouldBeCloseTo('div.offsetWidth', 12, 1);
+            computedTop = getPseudoComputedTop(id);
+            shouldBeCloseTo('computedTop', 200, 1);
         }, 2000);
     }
 }

Modified: trunk/LayoutTests/fast/css-generated-content/pseudo-transition-expected.txt (142214 => 142215)


--- trunk/LayoutTests/fast/css-generated-content/pseudo-transition-expected.txt	2013-02-08 01:28:52 UTC (rev 142214)
+++ trunk/LayoutTests/fast/css-generated-content/pseudo-transition-expected.txt	2013-02-08 01:40:30 UTC (rev 142215)
@@ -4,11 +4,15 @@
 
 
 PASS div.offsetWidth is 52
-PASS div.offsetWidth is 20
-PASS div.offsetWidth is 12
+PASS div.offsetWidth is within 1 of 20
+PASS computedTop is within 1 of 170
+PASS div.offsetWidth is within 1 of 12
+PASS computedTop is within 1 of 200
 PASS div.offsetWidth is 52
-PASS div.offsetWidth is 20
-PASS div.offsetWidth is 12
+PASS div.offsetWidth is within 1 of 20
+PASS computedTop is within 1 of 170
+PASS div.offsetWidth is within 1 of 12
+PASS computedTop is within 1 of 200
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/fast/css-generated-content/pseudo-transition.html (142214 => 142215)


--- trunk/LayoutTests/fast/css-generated-content/pseudo-transition.html	2013-02-08 01:28:52 UTC (rev 142214)
+++ trunk/LayoutTests/fast/css-generated-content/pseudo-transition.html	2013-02-08 01:40:30 UTC (rev 142215)
@@ -9,13 +9,16 @@
     display: block;
     height: 50px;
     width: 50px;
-    -webkit-transition: width 2s;
-    -moz-transition: width 2s;
-    transition: width 2s;
+    top: 50px;
+    position: relative;
+    -webkit-transition: width 2s, top 2s;
+    -moz-transition: width 2s, top 2s;
+    transition: width 2s, top 2s;
 }
 
 #before.transition:before,
 #after.transition:after {
+    top: 200px;
     height: 10px;
     width: 10px;
 }
@@ -42,6 +45,14 @@
 if (window.testRunner)
     testRunner.dumpAsText();
 
+function getPseudoComputedTop(id)
+{
+    return Math.round(parseFloat(getComputedStyle(document.getElementById(id), ':' + id).top));
+}
+
+// FIXME: This test should be modified so subpixel doesn't cause off by one
+// below and it no longer needs shouldBeCloseTo.
+
 function testTransition(id)
 {
     var div = document.getElementById(id);
@@ -50,19 +61,29 @@
     shouldBe('div.offsetWidth', '52');
     if (window.internals) {
         internals.pauseTransitionAtTimeOnPseudoElement('width', 1.0, div, id);
-        shouldBe('div.offsetWidth', '20');
+        shouldBeCloseTo('div.offsetWidth', 20, 1);
+        internals.pauseTransitionAtTimeOnPseudoElement('top', 1.0, div, id);
+        computedTop = getPseudoComputedTop(id);
+        shouldBeCloseTo('computedTop', 170, 1);
         internals.pauseTransitionAtTimeOnPseudoElement('width', 2.0, div, id);
-        shouldBe('div.offsetWidth', '12');
+        shouldBeCloseTo('div.offsetWidth', 12, 1);
+        internals.pauseTransitionAtTimeOnPseudoElement('top', 2.0, div, id);
+        computedTop = getPseudoComputedTop(id);
+        shouldBeCloseTo('computedTop', 200, 1);
     } else {
         // This will be flaky, but it's a reasonable approximation for testing
         // in a browser instead of DRT.
         setTimeout(function() {
             window.div = div;
-            shouldBe('div.offsetWidth', '20');
+            shouldBeCloseTo('div.offsetWidth', 20, 1);
+            computedTop = getPseudoComputedTop(id);
+            shouldBeCloseTo('computedTop', 170, 1);
         }, 1000);
         setTimeout(function() {
             window.div = div;
-            shouldBe('div.offsetWidth', '12');
+            shouldBeCloseTo('div.offsetWidth', 12, 1);
+            computedTop = getPseudoComputedTop(id);
+            shouldBeCloseTo('computedTop', 200, 1);
         }, 2000);
     }
 }

Modified: trunk/Source/WebCore/ChangeLog (142214 => 142215)


--- trunk/Source/WebCore/ChangeLog	2013-02-08 01:28:52 UTC (rev 142214)
+++ trunk/Source/WebCore/ChangeLog	2013-02-08 01:40:30 UTC (rev 142215)
@@ -1,3 +1,27 @@
+2013-02-07  Elliott Sprehn  <espr...@chromium.org>
+
+        getComputedStyle() doesn't report intermediate values during a transition of a pseudo element
+        https://bugs.webkit.org/show_bug.cgi?id=106535
+
+        Reviewed by Ojan Vafai.
+
+        Element::computedStyle and CSSComputedStyleDeclaration::getPropertyCSSValue
+        should use the PseudoElement and it's renderer if they exist so that
+        querying the computed style while an animation is running returns
+        the intermediate values.
+
+        No new tests, updated existing tests.
+
+        * css/CSSComputedStyleDeclaration.cpp:
+        (WebCore::CSSComputedStyleDeclaration::styledNode): Added, returns either the PseudoElement or the Node.
+        (WebCore::CSSComputedStyleDeclaration::getPropertyCSSValue): Updated to use styledNode.
+        * css/CSSComputedStyleDeclaration.h:
+        (CSSComputedStyleDeclaration):
+        * dom/Element.cpp:
+        (WebCore::Element::computedStyle): Check the PseudoElement, not just the cached pseudo style.
+        * dom/ElementRareData.h:
+        (WebCore::ElementRareData::pseudoElement): Remove ASSERT_NOT_REACHED so passing other pseudos returns 0.
+
 2013-02-07  Mark Lam  <mark....@apple.com>
 
         Add a comment about how the SQLTransaction state machine works.

Modified: trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp (142214 => 142215)


--- trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp	2013-02-08 01:28:52 UTC (rev 142214)
+++ trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp	2013-02-08 01:40:30 UTC (rev 142215)
@@ -51,6 +51,7 @@
 #include "FontValue.h"
 #include "HTMLFrameOwnerElement.h"
 #include "Pair.h"
+#include "PseudoElement.h"
 #include "Rect.h"
 #include "RenderBox.h"
 #include "RenderStyle.h"
@@ -1510,17 +1511,28 @@
     }
 }
 
+Node* CSSComputedStyleDeclaration::styledNode() const
+{
+    if (!m_node)
+        return 0;
+    if (m_node->isElementNode()) {
+        if (PseudoElement* element = toElement(m_node.get())->pseudoElement(m_pseudoElementSpecifier))
+            return element;
+    }
+    return m_node.get();
+}
+
 PassRefPtr<CSSValue> CSSComputedStyleDeclaration::getPropertyCSSValue(CSSPropertyID propertyID, EUpdateLayout updateLayout) const
 {
-    Node* node = m_node.get();
-    if (!node)
+    Node* styledNode = this->styledNode();
+    if (!styledNode)
         return 0;
 
     if (updateLayout) {
-        Document* document = m_node->document();
+        Document* document = styledNode->document();
         // FIXME: Some of these cases could be narrowed down or optimized better.
         bool forceFullLayout = isLayoutDependentProperty(propertyID)
-            || node->isInShadowTree()
+            || styledNode->isInShadowTree()
             || (document->styleResolverIfExists() && document->styleResolverIfExists()->hasViewportDependentMediaQueries() && document->ownerElement())
             || document->seamlessParentIFrame();
 
@@ -1528,32 +1540,33 @@
             document->updateLayoutIgnorePendingStylesheets();
         else {
             bool needsStyleRecalc = document->hasPendingForcedStyleRecalc();
-            for (Node* n = m_node.get(); n && !needsStyleRecalc; n = n->parentNode())
+            for (Node* n = styledNode; n && !needsStyleRecalc; n = n->parentNode())
                 needsStyleRecalc = n->needsStyleRecalc();
             if (needsStyleRecalc)
                 document->updateStyleIfNeeded();
         }
+
+        // The style recalc could have caused the styled node to be discarded or replaced
+        // if it was a PseudoElement so we need to update it.
+        styledNode = this->styledNode();
     }
 
-    RenderObject* renderer = node->renderer();
+    RenderObject* renderer = styledNode->renderer();
 
     RefPtr<RenderStyle> style;
     if (renderer && renderer->isComposited() && AnimationController::supportsAcceleratedAnimationOfProperty(propertyID)) {
         AnimationUpdateBlock animationUpdateBlock(renderer->animation());
         style = renderer->animation()->getAnimatedStyleForRenderer(renderer);
-        if (m_pseudoElementSpecifier) {
+        if (m_pseudoElementSpecifier && !styledNode->isPseudoElement()) {
             // FIXME: This cached pseudo style will only exist if the animation has been run at least once.
             style = style->getCachedPseudoStyle(m_pseudoElementSpecifier);
         }
     } else
-        style = node->computedStyle(m_pseudoElementSpecifier);
+        style = styledNode->computedStyle(styledNode->isPseudoElement() ? NOPSEUDO : m_pseudoElementSpecifier);
 
     if (!style)
         return 0;
 
-    if (node->isElementNode() && (m_pseudoElementSpecifier == BEFORE || m_pseudoElementSpecifier == AFTER))
-        renderer = toElement(node)->pseudoElementRenderer(m_pseudoElementSpecifier);
-
     propertyID = CSSProperty::resolveDirectionAwareProperty(propertyID, style->direction(), style->writingMode());
 
     switch (propertyID) {
@@ -1838,8 +1851,9 @@
             return cssValuePool().createValue(style->alignItems());
         case CSSPropertyWebkitAlignSelf:
             if (style->alignSelf() == AlignAuto) {
-                if (m_node && m_node->parentNode() && m_node->parentNode()->computedStyle())
-                    return cssValuePool().createValue(m_node->parentNode()->computedStyle()->alignItems());
+                Node* parent = styledNode->parentNode();
+                if (parent && parent->computedStyle())
+                    return cssValuePool().createValue(parent->computedStyle()->alignItems());
                 return cssValuePool().createValue(AlignStretch);
             }
             return cssValuePool().createValue(style->alignSelf());

Modified: trunk/Source/WebCore/css/CSSComputedStyleDeclaration.h (142214 => 142215)


--- trunk/Source/WebCore/css/CSSComputedStyleDeclaration.h	2013-02-08 01:28:52 UTC (rev 142214)
+++ trunk/Source/WebCore/css/CSSComputedStyleDeclaration.h	2013-02-08 01:40:30 UTC (rev 142215)
@@ -78,6 +78,13 @@
 private:
     CSSComputedStyleDeclaration(PassRefPtr<Node>, bool allowVisitedStyle, const String&);
 
+    // The styled node is either the node passed into getComputedStyle, or the
+    // PseudoElement for :before and :after if they exist.
+    // FIXME: This should be styledElement since in JS getComputedStyle only works
+    // on Elements, but right now editing creates these for text nodes. We should fix
+    // that.
+    Node* styledNode() const;
+
     // CSSOM functions. Don't make these public.
     virtual CSSRule* parentRule() const;
     virtual unsigned length() const;

Modified: trunk/Source/WebCore/dom/Element.cpp (142214 => 142215)


--- trunk/Source/WebCore/dom/Element.cpp	2013-02-08 01:28:52 UTC (rev 142214)
+++ trunk/Source/WebCore/dom/Element.cpp	2013-02-08 01:40:30 UTC (rev 142215)
@@ -1991,6 +1991,9 @@
 
 RenderStyle* Element::computedStyle(PseudoId pseudoElementSpecifier)
 {
+    if (PseudoElement* element = pseudoElement(pseudoElementSpecifier))
+        return element->computedStyle();
+
     // FIXME: Find and use the renderer from the pseudo element instead of the actual element so that the 'length'
     // properties, which are only known by the renderer because it did the layout, will be correct and so that the
     // values returned for the ":selection" pseudo-element will be correct.

Modified: trunk/Source/WebCore/dom/ElementRareData.h (142214 => 142215)


--- trunk/Source/WebCore/dom/ElementRareData.h	2013-02-08 01:28:52 UTC (rev 142214)
+++ trunk/Source/WebCore/dom/ElementRareData.h	2013-02-08 01:40:30 UTC (rev 142215)
@@ -240,7 +240,6 @@
     case AFTER:
         return m_generatedAfter.get();
     default:
-        ASSERT_NOT_REACHED();
         return 0;
     }
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to