Diff
Modified: trunk/LayoutTests/ChangeLog (143018 => 143019)
--- trunk/LayoutTests/ChangeLog 2013-02-15 18:56:27 UTC (rev 143018)
+++ trunk/LayoutTests/ChangeLog 2013-02-15 19:06:30 UTC (rev 143019)
@@ -1,3 +1,19 @@
+2013-02-15 Alexis Menard <[email protected]>
+
+ WebKit shouldn't accept "none, none" in transition shorthand property.
+ https://bugs.webkit.org/show_bug.cgi?id=108751
+
+ Reviewed by Dean Jackson.
+
+ Extend exisiting tests to cover the bug. Modify old tests with invalid declarations.
+
+ * fast/css/transform-inline-style-expected.txt:
+ * fast/css/transform-inline-style-remove-expected.txt:
+ * fast/css/transform-inline-style-remove.html:
+ * fast/css/transform-inline-style.html:
+ * transitions/transitions-parsing-expected.txt:
+ * transitions/transitions-parsing.html:
+
2013-02-15 Sudarsana Nagineni <[email protected]>
Unreviewed EFL gardening.
Modified: trunk/LayoutTests/fast/css/transform-inline-style-expected.txt (143018 => 143019)
--- trunk/LayoutTests/fast/css/transform-inline-style-expected.txt 2013-02-15 18:56:27 UTC (rev 143018)
+++ trunk/LayoutTests/fast/css/transform-inline-style-expected.txt 2013-02-15 19:06:30 UTC (rev 143019)
@@ -1,6 +1,6 @@
Tests reading inline style of transition, and -webkit-transform-origin
https://bugs.webkit.org/show_bug.cgi?id=22594
-style: all 1s ease, none 2s ease-in, left 3s cubic-bezier(0.2, 0.3, 0.6, 0.8) 2s
+style: all 1s ease, left 3s cubic-bezier(0.2, 0.3, 0.6, 0.8) 2s
style: 0% 30%
Modified: trunk/LayoutTests/fast/css/transform-inline-style-remove-expected.txt (143018 => 143019)
--- trunk/LayoutTests/fast/css/transform-inline-style-remove-expected.txt 2013-02-15 18:56:27 UTC (rev 143018)
+++ trunk/LayoutTests/fast/css/transform-inline-style-remove-expected.txt 2013-02-15 19:06:30 UTC (rev 143019)
@@ -3,10 +3,10 @@
All "after" results should be null/empty
-transition (before): all 1s ease, none 2s ease-in, left 3s cubic-bezier(0.2, 0.3, 0.6, 0.8) 2s
-transition property (before): all, none, left
-transition duration (before): 1s, 2s, 3s
-transition timing function (before): ease, ease-in, cubic-bezier(0.2, 0.3, 0.6, 0.8)
+transition (before): all 1s ease, left 3s cubic-bezier(0.2, 0.3, 0.6, 0.8) 2s
+transition property (before): all, left
+transition duration (before): 1s, 3s
+transition timing function (before): ease, cubic-bezier(0.2, 0.3, 0.6, 0.8)
transition (after):
transition property (after):
transition duration (after):
Modified: trunk/LayoutTests/fast/css/transform-inline-style-remove.html (143018 => 143019)
--- trunk/LayoutTests/fast/css/transform-inline-style-remove.html 2013-02-15 18:56:27 UTC (rev 143018)
+++ trunk/LayoutTests/fast/css/transform-inline-style-remove.html 2013-02-15 19:06:30 UTC (rev 143019)
@@ -66,7 +66,7 @@
</p>
<p>All "after" results should be null/empty</p>
<div id="box" style="border: 1px black;
- -webkit-transition: all 1s ease, none 2s ease-in, left 3s cubic-bezier(0.2, 0.3, 0.6, 0.8) 2s;
+ -webkit-transition: all 1s ease, left 3s cubic-bezier(0.2, 0.3, 0.6, 0.8) 2s;
-webkit-transform-origin: left 30%;
-webkit-animation: test 5s ease-in-out alternate infinite;">
</div>
Modified: trunk/LayoutTests/fast/css/transform-inline-style.html (143018 => 143019)
--- trunk/LayoutTests/fast/css/transform-inline-style.html 2013-02-15 18:56:27 UTC (rev 143018)
+++ trunk/LayoutTests/fast/css/transform-inline-style.html 2013-02-15 19:06:30 UTC (rev 143019)
@@ -26,7 +26,7 @@
<a href=""
</p>
<div id="box" style="border: 1px black;
- -webkit-transition: all 1s ease, none 2s ease-in, left 3s cubic-bezier(0.2, 0.3, 0.6, 0.8) 2s;
+ -webkit-transition: all 1s ease, left 3s cubic-bezier(0.2, 0.3, 0.6, 0.8) 2s;
-webkit-transform-origin: left 30%;">
</div>
<div id="console"></div>
Modified: trunk/LayoutTests/transitions/transitions-parsing-expected.txt (143018 => 143019)
--- trunk/LayoutTests/transitions/transitions-parsing-expected.txt 2013-02-15 18:56:27 UTC (rev 143018)
+++ trunk/LayoutTests/transitions/transitions-parsing-expected.txt 2013-02-15 19:06:30 UTC (rev 143019)
@@ -79,6 +79,10 @@
PASS computedStyle.transitionProperty is 'all'
PASS style.webkitTransitionProperty is ''
PASS computedStyle.webkitTransitionProperty is 'all'
+PASS style.transitionProperty is ''
+PASS computedStyle.transitionProperty is 'all'
+PASS style.webkitTransitionProperty is ''
+PASS computedStyle.webkitTransitionProperty is 'all'
Valid transition-duration values.
PASS computedStyle.transitionDuration is '0s'
PASS computedStyle.webkitTransitionDuration is '0s'
@@ -369,6 +373,16 @@
PASS computedStyle.transitionDelay is '10s, 20s'
PASS computedStyle.webkitTransitionDelay is '10s, 20s'
PASS checkTransitionShorthandValue() is true
+PASS style.transition is 'opacity 20s ease-in 10s, all 20s ease-out'
+PASS computedStyle.transition is 'opacity 20s ease-in 10s, all 20s ease-out 0s'
+PASS style.webkitTransition is 'opacity 20s ease-in 10s, all 20s ease-out'
+PASS computedStyle.webkitTransition is 'opacity 20s ease-in 10s, all 20s ease-out 0s'
+PASS checkTransitionShorthandValue() is true
+PASS style.transition is 'all 20s ease-out, opacity 20s ease-in 10s'
+PASS computedStyle.transition is 'all 20s ease-out 0s, opacity 20s ease-in 10s'
+PASS style.webkitTransition is 'all 20s ease-out, opacity 20s ease-in 10s'
+PASS computedStyle.webkitTransition is 'all 20s ease-out 0s, opacity 20s ease-in 10s'
+PASS checkTransitionShorthandValue() is true
Invalid transition shorthand values.
PASS style.transition is ''
PASS computedStyle.transition is 'all 0s ease 0s'
@@ -398,6 +412,34 @@
PASS computedStyle.transition is 'all 0s ease 0s'
PASS style.webkitTransition is ''
PASS computedStyle.webkitTransition is 'all 0s ease 0s'
+PASS style.transition is ''
+PASS computedStyle.transition is 'all 0s ease 0s'
+PASS style.webkitTransition is ''
+PASS computedStyle.webkitTransition is 'all 0s ease 0s'
+PASS style.transition is ''
+PASS computedStyle.transition is 'all 0s ease 0s'
+PASS style.webkitTransition is ''
+PASS computedStyle.webkitTransition is 'all 0s ease 0s'
+PASS style.transition is ''
+PASS computedStyle.transition is 'all 0s ease 0s'
+PASS style.webkitTransition is ''
+PASS computedStyle.webkitTransition is 'all 0s ease 0s'
+PASS style.transition is ''
+PASS computedStyle.transition is 'all 0s ease 0s'
+PASS style.webkitTransition is ''
+PASS computedStyle.webkitTransition is 'all 0s ease 0s'
+PASS style.transition is ''
+PASS computedStyle.transition is 'all 0s ease 0s'
+PASS style.webkitTransition is ''
+PASS computedStyle.webkitTransition is 'all 0s ease 0s'
+PASS style.transition is ''
+PASS computedStyle.transition is 'all 0s ease 0s'
+PASS style.webkitTransition is ''
+PASS computedStyle.webkitTransition is 'all 0s ease 0s'
+PASS style.transition is ''
+PASS computedStyle.transition is 'all 0s ease 0s'
+PASS style.webkitTransition is ''
+PASS computedStyle.webkitTransition is 'all 0s ease 0s'
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/transitions/transitions-parsing.html (143018 => 143019)
--- trunk/LayoutTests/transitions/transitions-parsing.html 2013-02-15 18:56:27 UTC (rev 143018)
+++ trunk/LayoutTests/transitions/transitions-parsing.html 2013-02-15 19:06:30 UTC (rev 143019)
@@ -140,6 +140,12 @@
shouldBe("style.webkitTransitionProperty", "''");
shouldBe("computedStyle.webkitTransitionProperty", "'all'");
+style.transitionProperty = "all, all";
+shouldBe("style.transitionProperty", "''");
+shouldBe("computedStyle.transitionProperty", "'all'");
+shouldBe("style.webkitTransitionProperty", "''");
+shouldBe("computedStyle.webkitTransitionProperty", "'all'");
+
style.transitionProperty = "";
debug("Valid transition-duration values.");
@@ -589,6 +595,20 @@
shouldBe("computedStyle.webkitTransitionDelay", "'10s, 20s'");
shouldBe("checkTransitionShorthandValue()", "true");
+style.transition = "ease-in opacity 20s 10s, all ease-out 20s";
+shouldBe("style.transition", "'opacity 20s ease-in 10s, all 20s ease-out'");
+shouldBe("computedStyle.transition", "'opacity 20s ease-in 10s, all 20s ease-out 0s'");
+shouldBe("style.webkitTransition", "'opacity 20s ease-in 10s, all 20s ease-out'");
+shouldBe("computedStyle.webkitTransition", "'opacity 20s ease-in 10s, all 20s ease-out 0s'");
+shouldBe("checkTransitionShorthandValue()", "true");
+
+style.transition = " all ease-out 20s, ease-in opacity 20s 10s";
+shouldBe("style.transition", "'all 20s ease-out, opacity 20s ease-in 10s'");
+shouldBe("computedStyle.transition", "'all 20s ease-out 0s, opacity 20s ease-in 10s'");
+shouldBe("style.webkitTransition", "'all 20s ease-out, opacity 20s ease-in 10s'");
+shouldBe("computedStyle.webkitTransition", "'all 20s ease-out 0s, opacity 20s ease-in 10s'");
+shouldBe("checkTransitionShorthandValue()", "true");
+
debug("Invalid transition shorthand values.");
style.transition = "";
@@ -634,6 +654,48 @@
shouldBe("style.webkitTransition", "''");
shouldBe("computedStyle.webkitTransition", "'all 0s ease 0s'");
+style.transition = "ease-in opacity 20s 10s, none";
+shouldBe("style.transition", "''");
+shouldBe("computedStyle.transition", "'all 0s ease 0s'");
+shouldBe("style.webkitTransition", "''");
+shouldBe("computedStyle.webkitTransition", "'all 0s ease 0s'");
+
+style.transition = "none, ease-in opacity 20s 10s, none";
+shouldBe("style.transition", "''");
+shouldBe("computedStyle.transition", "'all 0s ease 0s'");
+shouldBe("style.webkitTransition", "''");
+shouldBe("computedStyle.webkitTransition", "'all 0s ease 0s'");
+
+style.transition = "none, ease-in opacity 20s 10s";
+shouldBe("style.transition", "''");
+shouldBe("computedStyle.transition", "'all 0s ease 0s'");
+shouldBe("style.webkitTransition", "''");
+shouldBe("computedStyle.webkitTransition", "'all 0s ease 0s'");
+
+style.transition = "ease-in opacity 20s 10s, all 20s ease-out 0s, none";
+shouldBe("style.transition", "''");
+shouldBe("computedStyle.transition", "'all 0s ease 0s'");
+shouldBe("style.webkitTransition", "''");
+shouldBe("computedStyle.webkitTransition", "'all 0s ease 0s'");
+
+style.transition = "ease-in opacity 20s 10s, ease-in width 20s 10s, none";
+shouldBe("style.transition", "''");
+shouldBe("computedStyle.transition", "'all 0s ease 0s'");
+shouldBe("style.webkitTransition", "''");
+shouldBe("computedStyle.webkitTransition", "'all 0s ease 0s'");
+
+style.transition = "none, none";
+shouldBe("style.transition", "''");
+shouldBe("computedStyle.transition", "'all 0s ease 0s'");
+shouldBe("style.webkitTransition", "''");
+shouldBe("computedStyle.webkitTransition", "'all 0s ease 0s'");
+
+style.transition = "all, all";
+shouldBe("style.transition", "''");
+shouldBe("computedStyle.transition", "'all 0s ease 0s'");
+shouldBe("style.webkitTransition", "''");
+shouldBe("computedStyle.webkitTransition", "'all 0s ease 0s'");
+
document.body.removeChild(testContainer);
</script>
<script src=""
Modified: trunk/Source/WebCore/ChangeLog (143018 => 143019)
--- trunk/Source/WebCore/ChangeLog 2013-02-15 18:56:27 UTC (rev 143018)
+++ trunk/Source/WebCore/ChangeLog 2013-02-15 19:06:30 UTC (rev 143019)
@@ -1,3 +1,41 @@
+2013-02-15 Alexis Menard <[email protected]>
+
+ WebKit shouldn't accept "none, none" in transition shorthand property.
+ https://bugs.webkit.org/show_bug.cgi?id=108751
+
+ Reviewed by Dean Jackson.
+
+ http://dev.w3.org/csswg/css3-transitions/#transition-shorthand-property
+ specifies that if there is more than one transition defined in the
+ shorthand and any of them has a value of 'none' then the declaration is
+ invalid. This patch fixes the problem by passing a parsing context to
+ track if a keyword has been set for the transition-property and if so
+ then use it to invalidate or not the declaration.
+
+ Test: transitions/transitions-parsing.html
+
+ * css/CSSParser.cpp:
+ (AnimationParseContext):
+ (WebCore::AnimationParseContext::AnimationParseContext):
+ (WebCore::AnimationParseContext::commitFirstAnimation): track whether
+ it's the first <single-transition/animation> or not defined in the
+ shorthand.
+ (WebCore::AnimationParseContext::hasCommittedFirstAnimation):
+ (WebCore::AnimationParseContext::commitAnimationPropertyKeywordInShorthand):
+ In the shorthand as soon as a keyword has been found then the parsing
+ is 'finished', if any other animation/transition declaration part of
+ the shorthand are with a keyword then it's invalid.
+ (WebCore::AnimationParseContext::animationPropertyKeywordInShorthandAllowed):
+ (WebCore::AnimationParseContext::hasSeenAnimationPropertyKeyword):
+ (WebCore::AnimationParseContext::sawAnimationPropertyKeyword):
+ (WebCore):
+ (WebCore::CSSParser::parseValue):
+ (WebCore::CSSParser::parseAnimationShorthand):
+ (WebCore::CSSParser::parseTransitionShorthand):
+ (WebCore::CSSParser::parseAnimationProperty):
+ * css/CSSParser.h:
+ (WebCore):
+
2013-02-15 Andreas Kling <[email protected]>
ElementData: Move leafy things out of the base class.
Modified: trunk/Source/WebCore/css/CSSParser.cpp (143018 => 143019)
--- trunk/Source/WebCore/css/CSSParser.cpp 2013-02-15 18:56:27 UTC (rev 143018)
+++ trunk/Source/WebCore/css/CSSParser.cpp 2013-02-15 19:06:30 UTC (rev 143019)
@@ -199,6 +199,51 @@
return cssValuePool().createValue(Pair::create(first, second));
}
+class AnimationParseContext {
+public:
+ AnimationParseContext()
+ : m_animationPropertyKeywordInShorthandAllowed(true)
+ , m_firstAnimationCommitted(false)
+ , m_hasSeenAnimationPropertyKeyword(false)
+ {
+ }
+
+ void commitFirstAnimation()
+ {
+ m_firstAnimationCommitted = true;
+ }
+
+ bool hasCommittedFirstAnimation() const
+ {
+ return m_firstAnimationCommitted;
+ }
+
+ void commitAnimationPropertyKeywordInShorthand()
+ {
+ m_animationPropertyKeywordInShorthandAllowed = false;
+ }
+
+ bool animationPropertyKeywordInShorthandAllowed() const
+ {
+ return m_animationPropertyKeywordInShorthandAllowed;
+ }
+
+ bool hasSeenAnimationPropertyKeyword() const
+ {
+ return m_hasSeenAnimationPropertyKeyword;
+ }
+
+ void sawAnimationPropertyKeyword()
+ {
+ m_hasSeenAnimationPropertyKeyword = true;
+ }
+
+private:
+ bool m_animationPropertyKeywordInShorthandAllowed;
+ bool m_firstAnimationCommitted;
+ bool m_hasSeenAnimationPropertyKeyword;
+};
+
const CSSParserContext& strictCSSParserContext()
{
DEFINE_STATIC_LOCAL(CSSParserContext, strictContext, (CSSStrictMode));
@@ -2518,7 +2563,8 @@
case CSSPropertyWebkitTransitionTimingFunction:
case CSSPropertyWebkitTransitionProperty: {
RefPtr<CSSValue> val;
- if (parseAnimationProperty(propId, val)) {
+ AnimationParseContext context;
+ if (parseAnimationProperty(propId, val, context)) {
addProperty(propId, val.release(), important);
return true;
}
@@ -3224,6 +3270,7 @@
ShorthandScope scope(this, CSSPropertyWebkitAnimation);
bool parsedProperty[numProperties] = { false };
+ AnimationParseContext context;
RefPtr<CSSValue> values[numProperties];
unsigned i;
@@ -3239,18 +3286,23 @@
}
if (!m_valueList->current())
break;
+ context.commitFirstAnimation();
}
bool found = false;
for (i = 0; i < numProperties; ++i) {
if (!parsedProperty[i]) {
RefPtr<CSSValue> val;
- if (parseAnimationProperty(animationProperties.properties()[i], val)) {
+ if (parseAnimationProperty(animationProperties.properties()[i], val, context)) {
parsedProperty[i] = found = true;
addAnimationValue(values[i], val.release());
break;
}
}
+
+ // There are more values to process but 'none' or 'all' were already defined as the animation property, the declaration becomes invalid.
+ if (!context.animationPropertyKeywordInShorthandAllowed() && context.hasCommittedFirstAnimation())
+ return false;
}
// if we didn't find at least one match, this is an
@@ -3278,6 +3330,7 @@
ShorthandScope scope(this, CSSPropertyWebkitTransition);
bool parsedProperty[numProperties] = { false };
+ AnimationParseContext context;
RefPtr<CSSValue> values[numProperties];
unsigned i;
@@ -3293,16 +3346,21 @@
}
if (!m_valueList->current())
break;
+ context.commitFirstAnimation();
}
bool found = false;
for (i = 0; !found && i < numProperties; ++i) {
if (!parsedProperty[i]) {
RefPtr<CSSValue> val;
- if (parseAnimationProperty(webkitTransitionShorthand().properties()[i], val)) {
+ if (parseAnimationProperty(webkitTransitionShorthand().properties()[i], val, context)) {
parsedProperty[i] = found = true;
addAnimationValue(values[i], val.release());
}
+
+ // There are more values to process but 'none' or 'all' were already defined as the animation property, the declaration becomes invalid.
+ if (!context.animationPropertyKeywordInShorthandAllowed() && context.hasCommittedFirstAnimation())
+ return false;
}
}
@@ -4354,7 +4412,7 @@
return 0;
}
-PassRefPtr<CSSValue> CSSParser::parseAnimationProperty(bool& allowAnimationProperty)
+PassRefPtr<CSSValue> CSSParser::parseAnimationProperty(AnimationParseContext& context)
{
CSSParserValue* value = m_valueList->current();
if (value->unit != CSSPrimitiveValue::CSS_IDENT)
@@ -4362,10 +4420,16 @@
int result = cssPropertyID(value->string);
if (result)
return cssValuePool().createIdentifierValue(result);
- if (equalIgnoringCase(value, "all"))
+ if (equalIgnoringCase(value, "all")) {
+ if (inShorthand() && context.hasSeenAnimationPropertyKeyword())
+ context.commitAnimationPropertyKeywordInShorthand();
+ context.sawAnimationPropertyKeyword();
return cssValuePool().createIdentifierValue(CSSValueAll);
+ }
if (equalIgnoringCase(value, "none")) {
- allowAnimationProperty = false;
+ if (inShorthand())
+ context.commitAnimationPropertyKeywordInShorthand();
+ context.sawAnimationPropertyKeyword();
return cssValuePool().createIdentifierValue(CSSValueNone);
}
return 0;
@@ -4474,13 +4538,12 @@
return 0;
}
-bool CSSParser::parseAnimationProperty(CSSPropertyID propId, RefPtr<CSSValue>& result)
+bool CSSParser::parseAnimationProperty(CSSPropertyID propId, RefPtr<CSSValue>& result, AnimationParseContext& context)
{
RefPtr<CSSValueList> values;
CSSParserValue* val;
RefPtr<CSSValue> value;
bool allowComma = false;
- bool allowAnimationProperty = true;
result = 0;
@@ -4532,9 +4595,8 @@
m_valueList->next();
break;
case CSSPropertyWebkitTransitionProperty:
- if (allowAnimationProperty)
- currValue = parseAnimationProperty(allowAnimationProperty);
- if (value && !allowAnimationProperty)
+ currValue = parseAnimationProperty(context);
+ if (value && context.hasSeenAnimationPropertyKeyword())
return false;
if (currValue)
m_valueList->next();
Modified: trunk/Source/WebCore/css/CSSParser.h (143018 => 143019)
--- trunk/Source/WebCore/css/CSSParser.h 2013-02-15 18:56:27 UTC (rev 143018)
+++ trunk/Source/WebCore/css/CSSParser.h 2013-02-15 19:06:30 UTC (rev 143019)
@@ -45,6 +45,7 @@
namespace WebCore {
+class AnimationParseContext;
class CSSBorderImageSliceValue;
class CSSPrimitiveValue;
class CSSSelectorList;
@@ -143,12 +144,12 @@
PassRefPtr<CSSValue> parseAnimationIterationCount();
PassRefPtr<CSSValue> parseAnimationName();
PassRefPtr<CSSValue> parseAnimationPlayState();
- PassRefPtr<CSSValue> parseAnimationProperty(bool& allowAnimationProperty);
+ PassRefPtr<CSSValue> parseAnimationProperty(AnimationParseContext&);
PassRefPtr<CSSValue> parseAnimationTimingFunction();
bool parseTransformOriginShorthand(RefPtr<CSSValue>&, RefPtr<CSSValue>&, RefPtr<CSSValue>&);
bool parseCubicBezierTimingFunctionValue(CSSParserValueList*& args, double& result);
- bool parseAnimationProperty(CSSPropertyID, RefPtr<CSSValue>&);
+ bool parseAnimationProperty(CSSPropertyID, RefPtr<CSSValue>&, AnimationParseContext&);
bool parseTransitionShorthand(bool important);
bool parseAnimationShorthand(bool important);
