[webkit-reviews] review denied: [Bug 93551] Some CSS pseudo selectors causing clicks to reload resources on input with type=image : [Attachment 189718] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Pravin D <pravind at webkit.org>'s
request for review:
Bug 93551: Some CSS pseudo selectors causing clicks to reload resources on
input with type=image
https://bugs.webkit.org/show_bug.cgi?id=93551

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

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


>
LayoutTests/http/tests/misc/image-type-input-with-display-none-resource-request
.html:9
> +Description: Mouse click events on a page that contains a universal selector
:active, and has an <input type="image"> element E
> +	       should not initiate a HTTP request for the image resource
associated with E where E or its ancestor has display:none.
> +
> +Expected: The last entry in the network log should be PASS. The associated
expected file will have just one entry "PASS"

This should be included in the expected result so that people looking at the
expected  result can tell what's happening in the test.

>
LayoutTests/http/tests/misc/image-type-input-with-display-none-resource-request
.html:16
> +function CallCommand(cmd, shouldEndTest) {

CallCommand is a very vague name.

>
LayoutTests/http/tests/misc/image-type-input-with-display-none-resource-request
.html:21
> +    req.onreadystatechange=function() {

Spaces are needed around =.

>
LayoutTests/http/tests/misc/image-type-input-with-display-none-resource-request
.html:22
> +	       if (req.readyState==4 && req.status==200) {

Wrong indentation. Spaces are needed around ==.

>
LayoutTests/http/tests/misc/image-type-input-with-display-none-resource-request
.html:54
> +    setTimeout(sendMouseEvents, 10);

Why are we waiting for 10ms?

>
LayoutTests/http/tests/misc/image-type-input-with-display-none-resource-request
.html:57
> +function endTest(e) {

"e" is never used here.

>
LayoutTests/http/tests/misc/image-type-input-with-display-none-resource-request
.html:58
> +    setTimeout( function() { CallCommand("get-resource-request-log"); },
100);

Why are we waiting for 100ms?