[webkit-reviews] review denied: [Bug 110815] [CSS Shaders] Parse the geometry descriptor : [Attachment 195374] Patch not for formal review

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied  review:
Bug 110815: [CSS Shaders] Parse the geometry descriptor
https://bugs.webkit.org/show_bug.cgi?id=110815

Attachment 195374: Patch not for formal review
https://bugs.webkit.org/attachment.cgi?id=195374&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=195374&action=review


The parser implementation is not correct. r-

> Source/WebCore/css/CSSParser.cpp:8814
> +    // Third argument must be (detached | attached).

This comment is wrong. The third argument can be and integer. According to the
grammar above you can have:

int
int int
int det
int att
int int det
int int att
att int
att int int
det int
det int int

> Source/WebCore/css/CSSParser.cpp:8820
> +    if (gridParserValue) {
> +	   if (gridParserValue->id == CSSValueAttached || gridParserValue->id
== CSSValueDetached)
> +	      
geometryList->append(cssValuePool().createIdentifierValue(gridParserValue->id))
;
> +	   else {
> +	       geometryList.release();

yes, this does not do what you expect according to the grammar.

>
LayoutTests/css3/filters/custom-with-at-rule-syntax/parsing-geometry-property-i
nvalid-expected.txt:78
> +Attached value before numeric value
> +geometry: grid(attached 1);

This is valid.

>
LayoutTests/css3/filters/custom-with-at-rule-syntax/parsing-geometry-property-i
nvalid-expected.txt:84
> +Detached value before numeric value
> +geometry: grid(detached 1);

valid