Changeset 202950 in webkit

Timestamp:

Jul 7, 2016, 5:20:38 PM (10 years ago)

Author:

dino@apple.com

Message:

REGRESSION(r200769): animations are no longer overridden
​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:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css/shorthand-omitted-initial-value-overrides-shorthand-expected.txt (modified) (2 diffs)
• LayoutTests/fast/css/shorthand-omitted-initial-value-overrides-shorthand.html (modified) (2 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/CSSParser.cpp (modified) (2 diffs)
• Source/WebCore/css/CSSParser.h (modified) (1 diff)
• Source/WebCore/css/StyleProperties.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/fast/css/shorthand-omitted-initial-value-overrides-shorthand-expected.txt

@@ -10 +10 @@
 PASS transition-delay
 
+Prefixed transition properties
+PASS transition-property
+PASS transition-property
+PASS transition-property
+PASS transition-property
+
 Animation properties
 PASS animation-name
@@ -20 +26 @@
 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
 

trunk/LayoutTests/fast/css/shorthand-omitted-initial-value-overrides-shorthand.html

@@ -25 +25 @@
 
 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");
@@ -36 +43 @@
 
 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;
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/css/CSSParser.cpp

@@ -3813 +3813 @@
     }
 
+    // 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()
@@ -3951 +3974 @@
 
     // 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;

trunk/Source/WebCore/css/CSSParser.h

@@ -150 +150 @@
     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);

trunk/Source/WebCore/css/StyleProperties.cpp

@@ -640 +640 @@
     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;
 }