[webkit-reviews] review denied: [Bug 52162] Implement CSS3 Images cross-fade() image function : [Attachment 112832] parse -webkit-cross-fade
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Oct 28 10:58:43 PDT 2011
Simon Fraser (smfr) <simon.fraser at apple.com> has denied Tim Horton
<timothy_horton at apple.com>'s request for review:
Bug 52162: Implement CSS3 Images cross-fade() image function
https://bugs.webkit.org/show_bug.cgi?id=52162
Attachment 112832: parse -webkit-cross-fade
https://bugs.webkit.org/attachment.cgi?id=112832&action=review
------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=112832&action=review
Also, patch doesn't apply.
>
LayoutTests/fast/css/getComputedStyle/computed-style-cross-fade-expected.txt:16
> +PASS testCrossfade("background-image: -webkit-cross-fade(20%,
url(http://example.com/a.png), url(http://example.com/b.png))",
"background-image") is "none"
> +PASS successfullyParsed is true
You should test
-webkit-cross-fade(url(..),)
-webkit-cross-fade(,)
-webkit-cross-fade(,url(...)) as well.
> Source/WebCore/css/CSSCrossfadeValue.h:50
> + void setFromImage(const PassRefPtr<CSSImageValue> fromImage) {
m_fromImage = fromImage; }
> + void setToImage(const PassRefPtr<CSSImageValue> toImage) { m_toImage =
toImage; }
> + void setPercentage(const PassRefPtr<CSSPrimitiveValue> percentage) {
m_percentage = percentage; }
No need for the consts.
Perhaps you should just pass the two CSSImageValues into the create() method
and ctor?
> Source/WebCore/css/CSSParser.cpp:2791
> +bool CSSParser::parseFillImage(CSSParserValueList * valueList,
RefPtr<CSSValue>& value)
No space before the * for any of these.
> Source/WebCore/css/CSSParser.cpp:6195
> + RefPtr<CSSCrossfadeValue> result = CSSCrossfadeValue::create();
Might as well delay this until you actually need it.
> Source/WebCore/css/CSSParser.cpp:6229
> + if (!a || a->unit != CSSPrimitiveValue::CSS_PERCENTAGE)
> + return false;
Should we allow fractions here ("0.4")?
More information about the webkit-reviews
mailing list