- Revision
- 143948
- Author
- ale...@webkit.org
- Date
- 2013-02-25 10:56:19 -0800 (Mon, 25 Feb 2013)
Log Message
transition-property property doesn't accept "all, <IDENT>".
https://bugs.webkit.org/show_bug.cgi?id=110074
Reviewed by Dean Jackson.
Source/WebCore:
http://dev.w3.org/csswg/css3-transitions/#transition-property-property
allows all, <IDENT> as a value for the transition-property property. In
fact thanks to http://trac.webkit.org/changeset/143019 we correctly
implemented that behavior for transition shorthand property while
fixing bugs on the previous implementation. We did introduce a
AnimationParseContext to track whether the parsing of the
transition-property was finished or not in relation to the keyword.
This patch extend that mechanism to the longhand by renaming the
boolean and the functions to use it in the context class and set it
correctly while parsing the longhand property.
Test: LayoutTests/transitions/transitions-parsing.html
* css/CSSParser.cpp:
(WebCore::AnimationParseContext::AnimationParseContext):
(WebCore::AnimationParseContext::commitAnimationPropertyKeyword):
(WebCore::AnimationParseContext::animationPropertyKeywordAllowed):
(AnimationParseContext):
(WebCore::CSSParser::parseAnimationShorthand):
(WebCore::CSSParser::parseTransitionShorthand):
(WebCore::CSSParser::parseAnimationProperty): We can remove the
condition inShorthand() here, if 'none' is parsed then no more keyword
can appear, if 'all' is parsed then we can continue the parsing but
invalidate the property if another keyword is encountered. These
conditions are valid for the shorthand and the longhand.
LayoutTests:
Extend exising test to cover the bug.
* transitions/transitions-parsing-expected.txt:
* transitions/transitions-parsing.html:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (143947 => 143948)
--- trunk/LayoutTests/ChangeLog 2013-02-25 18:39:11 UTC (rev 143947)
+++ trunk/LayoutTests/ChangeLog 2013-02-25 18:56:19 UTC (rev 143948)
@@ -1,3 +1,15 @@
+2013-02-25 Alexis Menard <ale...@webkit.org>
+
+ transition-property property doesn't accept "all, <IDENT>".
+ https://bugs.webkit.org/show_bug.cgi?id=110074
+
+ Reviewed by Dean Jackson.
+
+ Extend exising test to cover the bug.
+
+ * transitions/transitions-parsing-expected.txt:
+ * transitions/transitions-parsing.html:
+
2013-02-25 Sergio Villar Senin <svil...@igalia.com>
[soup] "Too many redirects" error loading chat in plus.google.com
Modified: trunk/LayoutTests/transitions/transitions-parsing-expected.txt (143947 => 143948)
--- trunk/LayoutTests/transitions/transitions-parsing-expected.txt 2013-02-25 18:39:11 UTC (rev 143947)
+++ trunk/LayoutTests/transitions/transitions-parsing-expected.txt 2013-02-25 18:56:19 UTC (rev 143948)
@@ -26,6 +26,18 @@
PASS computedStyle.transitionProperty is 'background-position, font-size, color'
PASS style.webkitTransitionProperty is 'background-position, font-size, color'
PASS computedStyle.webkitTransitionProperty is 'background-position, font-size, color'
+PASS style.transitionProperty is 'all, font-size, color'
+PASS computedStyle.transitionProperty is 'all, font-size, color'
+PASS style.webkitTransitionProperty is 'all, font-size, color'
+PASS computedStyle.webkitTransitionProperty is 'all, font-size, color'
+PASS style.transitionProperty is 'font-size, color, all'
+PASS computedStyle.transitionProperty is 'font-size, color, all'
+PASS style.webkitTransitionProperty is 'font-size, color, all'
+PASS computedStyle.webkitTransitionProperty is 'font-size, color, all'
+PASS style.transitionProperty is 'font-size, all, color'
+PASS computedStyle.transitionProperty is 'font-size, all, color'
+PASS style.webkitTransitionProperty is 'font-size, all, color'
+PASS computedStyle.webkitTransitionProperty is 'font-size, all, color'
Invalid transition-property values.
PASS style.transitionProperty is ''
PASS computedStyle.transitionProperty is 'all'
Modified: trunk/LayoutTests/transitions/transitions-parsing.html (143947 => 143948)
--- trunk/LayoutTests/transitions/transitions-parsing.html 2013-02-25 18:39:11 UTC (rev 143947)
+++ trunk/LayoutTests/transitions/transitions-parsing.html 2013-02-25 18:56:19 UTC (rev 143948)
@@ -59,6 +59,24 @@
shouldBe("style.webkitTransitionProperty", "'background-position, font-size, color'");
shouldBe("computedStyle.webkitTransitionProperty", "'background-position, font-size, color'");
+style.transitionProperty = "all, font-size, color";
+shouldBe("style.transitionProperty", "'all, font-size, color'");
+shouldBe("computedStyle.transitionProperty", "'all, font-size, color'");
+shouldBe("style.webkitTransitionProperty", "'all, font-size, color'");
+shouldBe("computedStyle.webkitTransitionProperty", "'all, font-size, color'");
+
+style.transitionProperty = "font-size, color, all";
+shouldBe("style.transitionProperty", "'font-size, color, all'");
+shouldBe("computedStyle.transitionProperty", "'font-size, color, all'");
+shouldBe("style.webkitTransitionProperty", "'font-size, color, all'");
+shouldBe("computedStyle.webkitTransitionProperty", "'font-size, color, all'");
+
+style.transitionProperty = "font-size, all, color";
+shouldBe("style.transitionProperty", "'font-size, all, color'");
+shouldBe("computedStyle.transitionProperty", "'font-size, all, color'");
+shouldBe("style.webkitTransitionProperty", "'font-size, all, color'");
+shouldBe("computedStyle.webkitTransitionProperty", "'font-size, all, color'");
+
debug("Invalid transition-property values.");
style.transitionProperty = "";
Modified: trunk/Source/WebCore/ChangeLog (143947 => 143948)
--- trunk/Source/WebCore/ChangeLog 2013-02-25 18:39:11 UTC (rev 143947)
+++ trunk/Source/WebCore/ChangeLog 2013-02-25 18:56:19 UTC (rev 143948)
@@ -1,3 +1,36 @@
+2013-02-25 Alexis Menard <ale...@webkit.org>
+
+ transition-property property doesn't accept "all, <IDENT>".
+ https://bugs.webkit.org/show_bug.cgi?id=110074
+
+ Reviewed by Dean Jackson.
+
+ http://dev.w3.org/csswg/css3-transitions/#transition-property-property
+ allows all, <IDENT> as a value for the transition-property property. In
+ fact thanks to http://trac.webkit.org/changeset/143019 we correctly
+ implemented that behavior for transition shorthand property while
+ fixing bugs on the previous implementation. We did introduce a
+ AnimationParseContext to track whether the parsing of the
+ transition-property was finished or not in relation to the keyword.
+ This patch extend that mechanism to the longhand by renaming the
+ boolean and the functions to use it in the context class and set it
+ correctly while parsing the longhand property.
+
+ Test: LayoutTests/transitions/transitions-parsing.html
+
+ * css/CSSParser.cpp:
+ (WebCore::AnimationParseContext::AnimationParseContext):
+ (WebCore::AnimationParseContext::commitAnimationPropertyKeyword):
+ (WebCore::AnimationParseContext::animationPropertyKeywordAllowed):
+ (AnimationParseContext):
+ (WebCore::CSSParser::parseAnimationShorthand):
+ (WebCore::CSSParser::parseTransitionShorthand):
+ (WebCore::CSSParser::parseAnimationProperty): We can remove the
+ condition inShorthand() here, if 'none' is parsed then no more keyword
+ can appear, if 'all' is parsed then we can continue the parsing but
+ invalidate the property if another keyword is encountered. These
+ conditions are valid for the shorthand and the longhand.
+
2013-02-25 No'am Rosenthal <n...@webkit.org>
[Texmap] LayoutTests/compositing/animation/state-at-end-event-transform-layer.html shows a red square where it shouldn't
Modified: trunk/Source/WebCore/css/CSSParser.cpp (143947 => 143948)
--- trunk/Source/WebCore/css/CSSParser.cpp 2013-02-25 18:39:11 UTC (rev 143947)
+++ trunk/Source/WebCore/css/CSSParser.cpp 2013-02-25 18:56:19 UTC (rev 143948)
@@ -202,7 +202,7 @@
class AnimationParseContext {
public:
AnimationParseContext()
- : m_animationPropertyKeywordInShorthandAllowed(true)
+ : m_animationPropertyKeywordAllowed(true)
, m_firstAnimationCommitted(false)
, m_hasSeenAnimationPropertyKeyword(false)
{
@@ -218,14 +218,14 @@
return m_firstAnimationCommitted;
}
- void commitAnimationPropertyKeywordInShorthand()
+ void commitAnimationPropertyKeyword()
{
- m_animationPropertyKeywordInShorthandAllowed = false;
+ m_animationPropertyKeywordAllowed = false;
}
- bool animationPropertyKeywordInShorthandAllowed() const
+ bool animationPropertyKeywordAllowed() const
{
- return m_animationPropertyKeywordInShorthandAllowed;
+ return m_animationPropertyKeywordAllowed;
}
bool hasSeenAnimationPropertyKeyword() const
@@ -239,7 +239,7 @@
}
private:
- bool m_animationPropertyKeywordInShorthandAllowed;
+ bool m_animationPropertyKeywordAllowed;
bool m_firstAnimationCommitted;
bool m_hasSeenAnimationPropertyKeyword;
};
@@ -3308,7 +3308,7 @@
}
// 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())
+ if (!context.animationPropertyKeywordAllowed() && context.hasCommittedFirstAnimation())
return false;
}
@@ -3366,7 +3366,7 @@
}
// 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())
+ if (!context.animationPropertyKeywordAllowed() && context.hasCommittedFirstAnimation())
return false;
}
}
@@ -4428,14 +4428,13 @@
if (result)
return cssValuePool().createIdentifierValue(result);
if (equalIgnoringCase(value, "all")) {
- if (inShorthand() && context.hasSeenAnimationPropertyKeyword())
- context.commitAnimationPropertyKeywordInShorthand();
+ if (context.hasSeenAnimationPropertyKeyword())
+ context.commitAnimationPropertyKeyword();
context.sawAnimationPropertyKeyword();
return cssValuePool().createIdentifierValue(CSSValueAll);
}
if (equalIgnoringCase(value, "none")) {
- if (inShorthand())
- context.commitAnimationPropertyKeywordInShorthand();
+ context.commitAnimationPropertyKeyword();
context.sawAnimationPropertyKeyword();
return cssValuePool().createIdentifierValue(CSSValueNone);
}
@@ -4603,7 +4602,7 @@
break;
case CSSPropertyWebkitTransitionProperty:
currValue = parseAnimationProperty(context);
- if (value && context.hasSeenAnimationPropertyKeyword())
+ if (value && !context.animationPropertyKeywordAllowed())
return false;
if (currValue)
m_valueList->next();
