[webkit-reviews] review denied: [Bug 59369] Convert LayoutTests/fast/events/mouseout-dead-node.html from pixel test to text-based test : [Attachment 91235] Patch

Wed Apr 27 12:42:18 PDT 2011

Ryosuke Niwa <rniwa at webkit.org> has denied Naoki Takano
<takano.naoki at gmail.com>'s request for review:
Bug 59369: Convert LayoutTests/fast/events/mouseout-dead-node.html from pixel
test to text-based test
https://bugs.webkit.org/show_bug.cgi?id=59369

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=91235&action=review

> LayoutTests/fast/events/mouseout-dead-node.html:1
> +<link rel="stylesheet" href="../js/resources/js-test-style.css">

No DOCTYPE / head?

> LayoutTests/fast/events/mouseout-dead-node.html:4
>  <body onload='test()'>

I don't think there's any need to wait until page load.

> LayoutTests/fast/events/mouseout-dead-node.html:15
> +    <div id=d0 style='border:2px solid red'>
> +	   <div onmouseout='testFailed("mouseout")' 
onmouseover='document.getElementById("d0").innerHTML ="you should see PASS
below"'>
> +	       <div onmouseout='testFailed("mouseout")'>
> +		   <span id=target1 onmouseout='testPassed("mouseout")' >
> +		       mouse over me
> +		   </span>
> +	       </div>

I don't think we need to indent each element like this.

> LayoutTests/fast/events/mouseout-dead-node.html:32
>  function test() {

You don't need this function. You can just run the script as it's parsed.

> LayoutTests/fast/events/mouseout-dead-node.html:54
> +description("Test that if node dies under mouse it receives mouseout event
but that the event does not propagate.");

Please move this above the test code so that people reading the test code can
understand the intent of this test before reading the code.