[webkit-reviews] review denied: [Bug 61332] Convert LayoutTests/editing/deleting/5206311-2.html to dump-as-markup : [Attachment 94531] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Annie Sullivan
<sullivan at chromium.org>'s request for review:
Bug 61332: Convert LayoutTests/editing/deleting/5206311-2.html to
dump-as-markup
https://bugs.webkit.org/show_bug.cgi?id=61332

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

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

In general, we'd like to improve the readability & usability of a test when
we're converting it.  While this patch does convert the test, I'd like to see
more improvements in test descriptions and code.

> LayoutTests/ChangeLog:7
> +

Missing description.

> LayoutTests/editing/deleting/5206311-2-expected.txt:2
> +This empties the last row, it should be removed. 'world!' should also be
brought into the second cell of the second row.:

It's odd to have a period before a colon.

> LayoutTests/editing/deleting/5206311-2-expected.txt:33
> +This empties a the last row of the first table and the first of the second,
they should both be removed.:

the first row of the second table?  I know you're just converting the existing
tests but please revise these descriptions.

> LayoutTests/editing/deleting/5206311-2.html:1
> +<script src="../../resources/dump-as-markup.js"></script>

Missing DOCTYPE, html, & body.

> LayoutTests/editing/deleting/5206311-2.html:3
>  function runTest(num)

I would have modified this function to take an element instead of a number like
this.