REGRESSION(r200769): animations are no longer overridden
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Jul 2016 00:20:38 +0000 (00:20 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Jul 2016 00:20:38 +0000 (00:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=159450
<rdar://problem/27120570>

Reviewed by Zalan Bujtas.

Source/WebCore:

The change in r200769 removed a lot of the prefixing variant
handling, but unfortunately we can't be completely rid
of it until we alias the prefixed transitions and animations
to the non-prefixed form. For example, setting the prefixed
shorthand has to reset the non-prefixed longhands.

The fix was to explicitly call the variant forms when
parsing such longhands, and make sure that MutableStyleProperties
removes all prefixed variants when removing shorthands.

The existing test was amended to cover this case:
fast/css/shorthand-omitted-initial-value-overrides-shorthand.html

* css/CSSParser.cpp:
(WebCore::CSSParser::parseAnimationShorthand):
(WebCore::CSSParser::addPropertyWithPrefixingVariant):
(WebCore::CSSParser::parseTransitionShorthand):
* css/CSSParser.h:
* css/StyleProperties.cpp:
(WebCore::MutableStyleProperties::removeShorthandProperty):

LayoutTests:

Update an existing test to exercise a prefixed form applying
to non-prefixed longhands.

* fast/css/shorthand-omitted-initial-value-overrides-shorthand-expected.txt:
* fast/css/shorthand-omitted-initial-value-overrides-shorthand.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@202950 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/css/shorthand-omitted-initial-value-overrides-shorthand-expected.txt
LayoutTests/fast/css/shorthand-omitted-initial-value-overrides-shorthand.html
Source/WebCore/ChangeLog
Source/WebCore/css/CSSParser.cpp
Source/WebCore/css/CSSParser.h
Source/WebCore/css/StyleProperties.cpp

index 5a696b2..911d3f1 100644 (file)
@@ -1,3 +1,17 @@
+2016-07-07  Dean Jackson  <dino@apple.com>
+
+        REGRESSION(r200769): animations are no longer overridden
+        https://bugs.webkit.org/show_bug.cgi?id=159450
+        <rdar://problem/27120570>
+
+        Reviewed by Zalan Bujtas.
+
+        Update an existing test to exercise a prefixed form applying
+        to non-prefixed longhands.
+
+        * fast/css/shorthand-omitted-initial-value-overrides-shorthand-expected.txt:
+        * fast/css/shorthand-omitted-initial-value-overrides-shorthand.html:
+
 2016-07-07  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Test gardening after r202826
index ae758fa..aa9f0bb 100644 (file)
@@ -9,6 +9,12 @@ PASS transition-duration
 PASS transition-timing-function
 PASS transition-delay
 
+Prefixed transition properties
+PASS transition-property
+PASS transition-property
+PASS transition-property
+PASS transition-property
+
 Animation properties
 PASS animation-name
 PASS animation-duration
@@ -19,6 +25,12 @@ PASS animation-play-state
 PASS animation-delay
 PASS animation-fill-mode
 
+Prefixed animation properties
+PASS -webkit-animation-name
+PASS animation-name
+PASS -webkit-animation-name
+PASS animation-name
+
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 7b806ab..e114e98 100644 (file)
@@ -24,6 +24,13 @@ testStyle("transition-timing-function: linear; transition: none;", "transition-t
 testStyle("transition-delay: 1s; transition: none;", "transition-delay", "0s");
 
 debug("");
+debug("Prefixed transition properties");
+testStyle("-webkit-transition-property: none; transition: 1s;", "transition-property", "all");
+testStyle("-webkit-transition-property: none; -webkit-transition: 1s;", "transition-property", "all");
+testStyle("transition-property: none; transition: 1s;", "transition-property", "all");
+testStyle("transition-property: none; -webkit-transition: 1s;", "transition-property", "all");
+
+debug("");
 debug("Animation properties");
 testStyle("animation-name: foo; animation: 1s;", "animation-name", "none");
 testStyle("animation-duration: 1s; animation: none;", "animation-duration", "0s");
@@ -35,6 +42,13 @@ testStyle("animation-delay: 1s; animation: none;", "animation-delay", "0s");
 testStyle("animation-fill-mode: forwards; animation: none;", "animation-fill-mode", "none");
 
 debug("");
+debug("Prefixed animation properties");
+testStyle("-webkit-animation-name: foo; -webkit-animation: none;", "-webkit-animation-name", "none");
+testStyle("-webkit-animation-name: foo; animation: none;", "animation-name", "none");
+testStyle("animation-name: foo; -webkit-animation: none;", "-webkit-animation-name", "none");
+testStyle("animation-name: foo; animation: none;", "animation-name", "none");
+
+debug("");
 successfullyParsed = true;
 
 </script>
index f416bcf..7fcbd24 100644 (file)
@@ -1,3 +1,32 @@
+2016-07-07  Dean Jackson  <dino@apple.com>
+
+        REGRESSION(r200769): animations are no longer overridden
+        https://bugs.webkit.org/show_bug.cgi?id=159450
+        <rdar://problem/27120570>
+
+        Reviewed by Zalan Bujtas.
+
+        The change in r200769 removed a lot of the prefixing variant
+        handling, but unfortunately we can't be completely rid
+        of it until we alias the prefixed transitions and animations
+        to the non-prefixed form. For example, setting the prefixed
+        shorthand has to reset the non-prefixed longhands.
+
+        The fix was to explicitly call the variant forms when
+        parsing such longhands, and make sure that MutableStyleProperties
+        removes all prefixed variants when removing shorthands.
+
+        The existing test was amended to cover this case:
+        fast/css/shorthand-omitted-initial-value-overrides-shorthand.html
+
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::parseAnimationShorthand):
+        (WebCore::CSSParser::addPropertyWithPrefixingVariant):
+        (WebCore::CSSParser::parseTransitionShorthand):
+        * css/CSSParser.h:
+        * css/StyleProperties.cpp:
+        (WebCore::MutableStyleProperties::removeShorthandProperty):
+
 2016-07-07  Alex Christensen  <achristensen@webkit.org>
 
         Fix CMake build.
index 2d72232..1eac879 100644 (file)
@@ -3812,17 +3812,40 @@ bool CSSParser::parseAnimationShorthand(CSSPropertyID propId, bool important)
             return false;
     }
 
+    // Fill in any remaining properties with the initial value.
     for (i = 0; i < numProperties; ++i) {
-        // If we didn't find the property, set an intial value.
         if (!parsedProperty[i])
             addAnimationValue(values[i], cssValuePool.createImplicitInitialValue());
-
-        addProperty(shorthand.properties()[i], WTFMove(values[i]), important);
     }
 
+    // Now add all of the properties we found.
+    // In this case we have to explicitly set the variant form as well,
+    // to make sure that a shorthand clears all existing prefixed and
+    // unprefixed values.
+    for (i = 0; i < numProperties; ++i)
+        addPropertyWithPrefixingVariant(shorthand.properties()[i], WTFMove(values[i]), important);
+
     return true;
 }
 
+void CSSParser::addPropertyWithPrefixingVariant(CSSPropertyID propId, RefPtr<CSSValue>&& value, bool important, bool implicit)
+{
+    addProperty(propId, value.copyRef(), important, implicit);
+
+    CSSPropertyID prefixingVariant = prefixingVariantForPropertyId(propId);
+    if (prefixingVariant == propId)
+        return;
+
+    if (m_currentShorthand) {
+        // We can't use ShorthandScope here as we can already be inside one (e.g we are parsing CSSTransition).
+        m_currentShorthand = prefixingVariantForPropertyId(m_currentShorthand);
+        addProperty(prefixingVariant, WTFMove(value), important, implicit);
+        m_currentShorthand = prefixingVariantForPropertyId(m_currentShorthand);
+    } else
+        addProperty(prefixingVariant, WTFMove(value), important, implicit);
+}
+
+
 RefPtr<CSSPrimitiveValue> CSSParser::parseColumnWidth()
 {
     ValueWithCalculation valueWithCalculation(*m_valueList->current());
@@ -3950,8 +3973,11 @@ bool CSSParser::parseTransitionShorthand(CSSPropertyID propId, bool important)
     }
 
     // Now add all of the properties we found.
+    // In this case we have to explicitly set the variant form as well,
+    // to make sure that a shorthand clears all existing prefixed and
+    // unprefixed values.
     for (i = 0; i < numProperties; ++i)
-        addProperty(shorthand.properties()[i], WTFMove(values[i]), important);
+        addPropertyWithPrefixingVariant(shorthand.properties()[i], WTFMove(values[i]), important);
 
     return true;
 }
index 139dec3..26837f5 100644 (file)
@@ -149,6 +149,7 @@ public:
     static Ref<ImmutableStyleProperties> parseInlineStyleDeclaration(const String&, Element*);
     std::unique_ptr<MediaQuery> parseMediaQuery(const String&);
 
+    void addPropertyWithPrefixingVariant(CSSPropertyID, RefPtr<CSSValue>&&, bool important, bool implicit = false);
     void addProperty(CSSPropertyID, RefPtr<CSSValue>&&, bool important, bool implicit = false);
     void rollbackLastProperties(int num);
     bool hasProperties() const { return !m_parsedProperties.isEmpty(); }
index f7bc730..e9caffb 100644 (file)
@@ -639,7 +639,16 @@ bool MutableStyleProperties::removeShorthandProperty(CSSPropertyID propertyID)
     StylePropertyShorthand shorthand = shorthandForProperty(propertyID);
     if (!shorthand.length())
         return false;
-    return removePropertiesInSet(shorthand.properties(), shorthand.length());
+
+    bool propertiesWereRemoved = removePropertiesInSet(shorthand.properties(), shorthand.length());
+
+    CSSPropertyID prefixingVariant = prefixingVariantForPropertyId(propertyID);
+    if (prefixingVariant == propertyID)
+        return propertiesWereRemoved;
+
+    StylePropertyShorthand shorthandPrefixingVariant = shorthandForProperty(prefixingVariant);
+    bool prefixedVariantPropertiesWereRemoved = removePropertiesInSet(shorthandPrefixingVariant.properties(), shorthandPrefixingVariant.length());
+    return propertiesWereRemoved || prefixedVariantPropertiesWereRemoved;
 }
 
 bool MutableStyleProperties::removeProperty(CSSPropertyID propertyID, String* returnText)