[webkit-reviews] review granted: [Bug 108751] WebKit shouldn't accept "none, none" in transition shorthand property. : [Attachment 188585] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Feb 15 10:21:21 PST 2013
Dean Jackson <dino at apple.com> has granted Alexis Menard (darktears)
<alexis at webkit.org>'s request for review:
Bug 108751: WebKit shouldn't accept "none, none" in transition shorthand
property.
https://bugs.webkit.org/show_bug.cgi?id=108751
Attachment 188585: Patch
https://bugs.webkit.org/attachment.cgi?id=188585&action=review
------- Additional Comments from Dean Jackson <dino at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=188585&action=review
r+ but I think we need to clarify the names before landing.
> Source/WebCore/ChangeLog:23
> + (WebCore::AnimationParseContext::hasFirstAnimationCommitted):
I suggest "hasCommittedFirstAnimation" (getter)
> Source/WebCore/ChangeLog:28
> +
(WebCore::AnimationParseContext::markParsingAnimationPropertyFinished):
> + 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::canParseMoreAnimationProperty):
This one is a tricky name, because it is so similar to the one below, but is
only used within shorthands :(
How about "canParseAnotherAnimationPropertyKeywordInShorthand" (getter) and
"animationPropertyKeywordParsedInShorthand" (setter).
Although I don't like that this has flipped the logic from your existing code,
and also that the getter returns the opposite of the setter (if that makes
sense).
> Source/WebCore/ChangeLog:30
> + (WebCore::AnimationParseContext::hasAnimationPropertyKeyword):
> + (WebCore::AnimationParseContext::commitAnimationPropertyKeyword):
How about "hasSeenAnimationPropertyKeyword" (getter) and
"sawAnimationPropertyKeyword" (setter)?
More information about the webkit-reviews
mailing list