[webkit-reviews] review denied: [Bug 53437] Convert editing/deleting/5168598.html to dumpAsText test : [Attachment 80656] fix patch

Mon Jan 31 11:10:28 PST 2011

Ryosuke Niwa <rniwa at webkit.org> has denied Chang Shu <Chang.Shu at nokia.com>'s
request for review:
Bug 53437: Convert editing/deleting/5168598.html to dumpAsText test
https://bugs.webkit.org/show_bug.cgi?id=53437

Attachment 80656: fix patch
https://bugs.webkit.org/attachment.cgi?id=80656&action=review

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

> LayoutTests/ChangeLog:5
> +	   Convert the test case to text-only so the expected result is
cross-platform.

Please explicitly say we're converting this to a dumpAsText test.

> LayoutTests/editing/deleting/5168598.html:1
>  <body>

Can we add <!DOCTYPE html>?

> LayoutTests/editing/deleting/5168598.html:4
> +<div id="console"></div>

We won't need this if we make the following changes.

> LayoutTests/editing/deleting/5168598.html:14
> +function log(message) {
> +    var console = document.getElementById("console");
> +    var text = document.createTextNode(message);
> +    console.appendChild(text);
> +}

I'm not sure if it makes sense to add this log function just to print one line.

> LayoutTests/editing/deleting/5168598.html:21
> +log(document.getElementsByTagName('a').item(0) + "\n");

I would have done:
var numberOfElements = document.getElementsByTagName('a').length;
document.write(numberOfElements ? 'FAIL: there were ' + numberOfElements + '
anchor elements but expected none' : 'PASS');