[webkit-reviews] review canceled: [Bug 44650] Support element() CSS image type : [Attachment 163510] Patch v1: Grammar & Parsing

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Sep 12 16:18:58 PDT 2012


Tony Chang <tony at chromium.org> has canceled Anish <bhayani at adobe.com>'s request
for review:
Bug 44650: Support element() CSS image type
https://bugs.webkit.org/show_bug.cgi?id=44650

Attachment 163510: Patch v1: Grammar & Parsing
https://bugs.webkit.org/attachment.cgi?id=163510&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=163510&action=review


This change seems much better. I just have some style nits.

As mentioned earlier, I don't think we should land this change until someone is
ready to finish implementing.  I'm going to remove the review flag to try and
avoid confusion.

> Source/WebCore/ChangeLog:10
> +	   CSSPrimitiveValue::CSS_ELEMENT_FUNCTION. Webkit-element is treated
similarly to webkit-image-set 

Nit: Webkit-element -> -webkit-element

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:21216
> +				93CA4C9C09DF93FA00DF8677 /* maketokenizer */,

This seems like an unintentional change.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:28715
> +				SYMROOT = bhayani/WebKit/WebKitBuild;

This seems like an unintentional change.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:28724
> +				SYMROOT = bhayani/WebKit/WebKitBuild;

This seems like an unintentional change.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:28733
> +				SYMROOT = bhayani/WebKit/WebKitBuild;

This seems like an unintentional change.

> Source/WebCore/css/CSSParser.cpp:7246
> +    if (arg->unit != CSSPrimitiveValue::CSS_ELEMENT_FUNCTION && arg->unit !=
CSSPrimitiveValue::CSS_PARSER_HEXCOLOR)

Nit: The CSS_PARSER_HEXCOLOR check probably deserves a comment.

> LayoutTests/fast/css/cssElement/webkit-element-parsing.html:48
> +    if (elementRule) {
> +	       subRule = elementRule.cssText;
> +	       shouldBe("subRule", "'" + expectedTexts + "'");

Nit: weird indent


More information about the webkit-reviews mailing list