[webkit-reviews] review granted: [Bug 15360] color:#{predefined colorName} is okay in Safari : [Attachment 22925] Potential fix to bug 15360 with no tabs

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Glenn Wilson <wilsong at gmail.com>'s
request for review:
Bug 15360: color:#{predefined colorName} is okay in Safari
https://bugs.webkit.org/show_bug.cgi?id=15360

Attachment 22925: Potential fix to bug 15360 with no tabs
https://bugs.webkit.org/attachment.cgi?id=22925&action=edit

------- Additional Comments from Eric Seidel <eric at webkit.org>
The change looks fine.	The test could still be "better"

<p id="test" style="color:#red">FAIL - testing that #color is ignored as a
color name, bug 15360</p>
<script>
if (window.layoutTestController)
   layoutTestController.dumpAsText();
var test = document.getElementById("test");
if (window.getComputedStyle(test).color == "rgb(0,0,0)") {
    test.innerText = "PASS";
}
</script>

would be more concise.	and shows only PASS/FAIL (to make it hopefully easier
to understand at a glance.

Even better would be to use a .js test, but that's a bit trickier for your
first patch (due to our lack of documentation on the subject, fast/js/resources
has a bunch of examples, make-js-test-wrappers is the script which generates
the .html files to run the .js tests from the TEMPLATE.html files).  We'll talk
about .js tests some time over IRC. :)

This is fine to land as-is though.