[webkit-reviews] review denied: [Bug 57148] Delete table in contentEditable/designMode produces odd contents : [Attachment 94843] 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 57148: Delete table in contentEditable/designMode produces odd contents
https://bugs.webkit.org/show_bug.cgi?id=57148

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

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

> LayoutTests/ChangeLog:8
> +	   Update test expectations and add new test for the case in the bug.

You should describe what kind of test you're adding and what kind of
rebaselines are done.

> LayoutTests/editing/deleting/delete-last-char-in-table-expected.txt:1
> +See this bug: https://bugs.webkit.org/show_bug.cgi?id=57148 "Delete table in
contentEditable/designMode produces odd contents".

This is a little verbose.  I'd prefer to have just the bug number since the
description below makes the context very clear.

> LayoutTests/editing/deleting/delete-last-char-in-table.html:11
> +<style>
> +.editing { 
> +    border: 2px solid red; 
> +    padding: 12px; 
> +    font-size: 24px; 
> +}
> +</style>

You don't need this style, do you?  We used to have this style when we didn't
have dump-as-markup but we don't need to add them anymore.

> LayoutTests/editing/deleting/delete-last-char-in-table.html:21
> +<script src=../editing.js language="JavaScript" type="text/JavaScript"
></script>

Please get rid of language and type attributes and also double-quote the url.

> Source/WebCore/ChangeLog:8
> +	   Removes -webkit-border-horizontal-spacing and
-webkit-border-vertical-spacing from the list of properties which should be
preserved during edit operations.

This line is too long.	Please split into two lines.