- Revision
- 202950
- Author
- [email protected]
- Date
- 2016-07-07 17:20:38 -0700 (Thu, 07 Jul 2016)
Log 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:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (202949 => 202950)
--- trunk/LayoutTests/ChangeLog 2016-07-08 00:12:39 UTC (rev 202949)
+++ trunk/LayoutTests/ChangeLog 2016-07-08 00:20:38 UTC (rev 202950)
@@ -1,3 +1,17 @@
+2016-07-07 Dean Jackson <[email protected]>
+
+ 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 <[email protected]>
Test gardening after r202826
Modified: trunk/LayoutTests/fast/css/shorthand-omitted-initial-value-overrides-shorthand-expected.txt (202949 => 202950)
--- trunk/LayoutTests/fast/css/shorthand-omitted-initial-value-overrides-shorthand-expected.txt 2016-07-08 00:12:39 UTC (rev 202949)
+++ trunk/LayoutTests/fast/css/shorthand-omitted-initial-value-overrides-shorthand-expected.txt 2016-07-08 00:20:38 UTC (rev 202950)
@@ -9,6 +9,12 @@
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-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
Modified: trunk/LayoutTests/fast/css/shorthand-omitted-initial-value-overrides-shorthand.html (202949 => 202950)
--- trunk/LayoutTests/fast/css/shorthand-omitted-initial-value-overrides-shorthand.html 2016-07-08 00:12:39 UTC (rev 202949)
+++ trunk/LayoutTests/fast/css/shorthand-omitted-initial-value-overrides-shorthand.html 2016-07-08 00:20:38 UTC (rev 202950)
@@ -24,6 +24,13 @@
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-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>
Modified: trunk/Source/WebCore/ChangeLog (202949 => 202950)
--- trunk/Source/WebCore/ChangeLog 2016-07-08 00:12:39 UTC (rev 202949)
+++ trunk/Source/WebCore/ChangeLog 2016-07-08 00:20:38 UTC (rev 202950)
@@ -1,3 +1,32 @@
+2016-07-07 Dean Jackson <[email protected]>
+
+ 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 <[email protected]>
Fix CMake build.
Modified: trunk/Source/WebCore/css/CSSParser.cpp (202949 => 202950)
--- trunk/Source/WebCore/css/CSSParser.cpp 2016-07-08 00:12:39 UTC (rev 202949)
+++ trunk/Source/WebCore/css/CSSParser.cpp 2016-07-08 00:20:38 UTC (rev 202950)
@@ -3812,17 +3812,40 @@
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 @@
}
// 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;
}
Modified: trunk/Source/WebCore/css/CSSParser.h (202949 => 202950)
--- trunk/Source/WebCore/css/CSSParser.h 2016-07-08 00:12:39 UTC (rev 202949)
+++ trunk/Source/WebCore/css/CSSParser.h 2016-07-08 00:20:38 UTC (rev 202950)
@@ -149,6 +149,7 @@
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(); }
Modified: trunk/Source/WebCore/css/StyleProperties.cpp (202949 => 202950)
--- trunk/Source/WebCore/css/StyleProperties.cpp 2016-07-08 00:12:39 UTC (rev 202949)
+++ trunk/Source/WebCore/css/StyleProperties.cpp 2016-07-08 00:20:38 UTC (rev 202950)
@@ -639,7 +639,16 @@
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)
