[webkit-reviews] review denied: [Bug 62620] dump-as-text conversion: editing/deleting/5300379.html : [Attachment 97064] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 13 21:14:48 PDT 2011


Ryosuke Niwa <rniwa at webkit.org> has denied Wyatt Carss <wcarss at google.com>'s
request for review:
Bug 62620: dump-as-text conversion: editing/deleting/5300379.html
https://bugs.webkit.org/show_bug.cgi?id=62620

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

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

> LayoutTests/editing/deleting/5300379.html:4
> +<div class="description">Sets cursor to 0 in the container above and deletes
by word backward, checking for a hang, then inserts 2 words and repeats, to
ensure that backward word deletion isn't a no-op.</div>

I'm afraid that adding extra div will cause this hang not to reproduce.  I'd
prefer printing this using InsertText as done in the case
!window.layoutTestController.

Also, this description is kind of convoluted.  It doesn't really tell us what
we're trying to test.  We should at least say that "you should see 'Success'
below" because that's the minimum requirement for this test to pass.  Also,
setting cursor (note, it's caret/insertion point; cursor usually refers to a
mark that indicates the current position of your pointing device) to offset 0
isn't really useful information because the div was empty to begin with. 
What's significant is the fact we're deleting word backward in an empty
editable div, and we're then making sure deleting word backward will remove
exactly one word.


More information about the webkit-reviews mailing list