[webkit-reviews] review denied: [Bug 94024] Add parsing of CSS compositing's blend-mode : [Attachment 158921] second patch with test results

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied  review:
Bug 94024: Add parsing of CSS compositing's blend-mode
https://bugs.webkit.org/show_bug.cgi?id=94024

Attachment 158921: second patch with test results
https://bugs.webkit.org/attachment.cgi?id=158921&action=review

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


The patch LGTM. Just the tests need more tweaks.

> Source/WebCore/ChangeLog:8
> +	   Added parsing and general CSS handling of -webkit-blend-mode per
http://www.w3.org/TR/2012/WD-compositing-20120816/

Link to WD, wow! :D

> LayoutTests/css3/compositing/script-tests/blend-mode-property-parsing.js:47
> +testFilterRule("Basic reference",
> +		  "multiply", 1, "multiply");

I would like to see a test for all keywords, not just 'multiply' so that we can
make sure that we support all and not accidentally remove support of one. This
can be done with a simple for-loop on an array ["multiply", '"normal", ....].

> LayoutTests/css3/compositing/script-tests/blend-mode-property.js:16
> +var declaration = cssRule.style;
> +shouldBe("declaration.length", "1");
> +shouldBe("declaration.getPropertyValue('-webkit-blend-mode')",
"\'multiply\'");

Same comment as above.