transition-property property doesn't accept "all, <IDENT>".
authoralexis@webkit.org <alexis@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Feb 2013 18:56:19 +0000 (18:56 +0000)
committeralexis@webkit.org <alexis@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Feb 2013 18:56:19 +0000 (18:56 +0000)
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:

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

LayoutTests/ChangeLog
LayoutTests/transitions/transitions-parsing-expected.txt
LayoutTests/transitions/transitions-parsing.html
Source/WebCore/ChangeLog
Source/WebCore/css/CSSParser.cpp

index bcc89a6..648674a 100644 (file)
@@ -1,3 +1,15 @@
+2013-02-25  Alexis Menard  <alexis@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  <svillar@igalia.com>
 
         [soup] "Too many redirects" error loading chat in plus.google.com
index 01445ea..d68ae7c 100644 (file)
@@ -26,6 +26,18 @@ PASS style.transitionProperty is 'background-position, font-size, color'
 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'
index 8d55035..7dcc9f4 100644 (file)
@@ -59,6 +59,24 @@ shouldBe("computedStyle.transitionProperty", "'background-position, font-size, c
 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 = "";
 
index ec32013..10d6daa 100644 (file)
@@ -1,3 +1,36 @@
+2013-02-25  Alexis Menard  <alexis@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  <noam@webkit.org>
 
         [Texmap] LayoutTests/compositing/animation/state-at-end-event-transform-layer.html shows a red square where it shouldn't
index 014a66b..e002e10 100644 (file)
@@ -202,7 +202,7 @@ static PassRefPtr<CSSPrimitiveValue> createPrimitiveValuePair(PassRefPtr<CSSPrim
 class AnimationParseContext {
 public:
     AnimationParseContext()
-        : m_animationPropertyKeywordInShorthandAllowed(true)
+        : m_animationPropertyKeywordAllowed(true)
         , m_firstAnimationCommitted(false)
         , m_hasSeenAnimationPropertyKeyword(false)
     {
@@ -218,14 +218,14 @@ public:
         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 @@ public:
     }
 
 private:
-    bool m_animationPropertyKeywordInShorthandAllowed;
+    bool m_animationPropertyKeywordAllowed;
     bool m_firstAnimationCommitted;
     bool m_hasSeenAnimationPropertyKeyword;
 };
@@ -3308,7 +3308,7 @@ bool CSSParser::parseAnimationShorthand(bool important)
             }
 
             // 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 @@ bool CSSParser::parseTransitionShorthand(bool important)
                 }
 
                 // 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 @@ PassRefPtr<CSSValue> CSSParser::parseAnimationProperty(AnimationParseContext& co
     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 @@ bool CSSParser::parseAnimationProperty(CSSPropertyID propId, RefPtr<CSSValue>& r
                     break;
                 case CSSPropertyWebkitTransitionProperty:
                     currValue = parseAnimationProperty(context);
-                    if (value && context.hasSeenAnimationPropertyKeyword())
+                    if (value && !context.animationPropertyKeywordAllowed())
                         return false;
                     if (currValue)
                         m_valueList->next();