[webkit-reviews] review denied: [Bug 71443] [Part 3] Parse the custom() function in -webkit-filter: parse the 3d-transforms parameters : [Attachment 158909] Patch

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied Michelangelo De Simone
<michelangelo at webkit.org>'s request for review:
Bug 71443: [Part 3] Parse the custom() function in -webkit-filter: parse the
3d-transforms parameters
https://bugs.webkit.org/show_bug.cgi?id=71443

Attachment 158909: Patch
https://bugs.webkit.org/attachment.cgi?id=158909&action=review

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


Patch looks good to me. Just some snippets.

> Source/WebCore/css/CSSParser.cpp:7579
>	   // TODO: Implement other parameters types parsing.
>	   // textures: https://bugs.webkit.org/show_bug.cgi?id=71442
> -	   // 3d-transforms: https://bugs.webkit.org/show_bug.cgi?id=71443
>	   // mat2, mat3, mat4: https://bugs.webkit.org/show_bug.cgi?id=71444
> -	   RefPtr<CSSValueList> paramValueList =
CSSValueList::createSpaceSeparated();
> -	   while ((arg = argsList->current())) {
> -	       // If we hit a comma it means we finished this parameter's
values.
> -	       if (isComma(arg))
> -		   break;
> -	       if (!validUnit(arg, FNumber, CSSStrictMode))
> +	   // array: https://bugs.webkit.org/show_bug.cgi?id=94226
> +	   // 3d-transform shall be the last to be checked
> +	   if (arg->unit == CSSParserValue::Function && arg->function)
> +	       parameterValue = parseCustomFilterTransform(argsList);

Please move the comment in to this if statement, I think you want to check all
functions in the same if statement.