[webkit-reviews] review denied: [Bug 96752] CSS Style is not recalculated when media attribute of style element is changed : [Attachment 167298] Patch 7

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Alexander Shalamov
<alexander.shalamov at gmail.com>'s request for review:
Bug 96752: CSS Style is not recalculated when media attribute of style element
is changed
https://bugs.webkit.org/show_bug.cgi?id=96752

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

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=167298&action=review


> LayoutTests/fast/media/mq-js-update-media.html:13
> +	     if (window.testRunner) {
> +		 testRunner.dumpAsText();
> +		 testRunner.waitUntilDone();
> +	     }

Why not do this outside the function?

> LayoutTests/fast/media/mq-js-update-media.html:19
> +	     test(divComputedStyle.backgroundColor, "255", "Div should have
rgb(255, 0, 0) background color.");

Maybe there is a shouldBe method for comparing this

> LayoutTests/fast/media/mq-js-update-media.html:33
> +	 function test(actual, expected, message) {
> +	     var re =
/^rgba?\((\d+),\s*(\d+),\s*(\d+)(?:,\s*(\d+(?:\.\d+)?))?\)$/;
> +	     var color = re.exec(actual)[1];
> +	     if(color === expected)

This test should include the js test frame work used by other tests, and ie.
not define log() itself.