[webkit-reviews] review denied: [Bug 114960] Wrong text position when you click backspace on the line below the image : [Attachment 226484] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Mar 13 10:59:55 PDT 2014


Darin Adler <darin at apple.com> has denied Sudarshan C P
<sudarshan.cp at samsung.com>'s request for review:
Bug 114960: Wrong text position when you click backspace on the line below the
image
https://bugs.webkit.org/show_bug.cgi?id=114960

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=226484&action=review


This patch is OK, but I’d like to see a better test. We shouldn’t land with a
test that we know will fail on some platforms.

> LayoutTests/editing/deleting/merge-image-and-text-expected.txt:11
> +	       RenderText {#text} at (4,0) size 31x17

This is non-portable, so this test is going to fail on some platforms. The text
might not be 31 pixels wide on every platform due to font differences.

> LayoutTests/editing/deleting/merge-image-and-text.html:1
> +<!DOCTYPE html>

It’s much better for editing tests to dump markup rather than a render tree.
Look at a recently added editing tests such as
editing/execCommand/indent-img-twice.html to see how to do this. It makes a
much useful test for the WebKit project. And deals with the portability issue I
mentioned above.


More information about the webkit-reviews mailing list