RESOLVED FIXED 26361
editing/deleting/5156801.html doesn't test what it claims to test
https://bugs.webkit.org/show_bug.cgi?id=26361
Summary editing/deleting/5156801.html doesn't test what it claims to test
Ryosuke Niwa
Reported 2009-06-12 16:27:30 PDT
The explanation and code in LayoutTests\editing\deleting\5156801.html does not match. In fact, the test seems to be missing some code. The following is the original code: <p>This tests for a bug where delete would crash if a node just after the selection to delete is removed in a DOMNodeRemoved event handler for the last node in the selection to delete.</p> <div contenteditable="true">foo<span id="foo">x</span><span id="removeme">y</span></div> <script> function removeHandler() { var removeme = document.getElementById("removeme"); removeme.parentNode.removeChild(removeme); } document.getElementById("foo").addEventListener("DOMNodeRemoved", removeHandler); </script>
Attachments
This patch fixes the bug, and converts the text from a pixel test to a dumpAsText test. (6.05 KB, patch)
2009-06-17 11:48 PDT, Ryosuke Niwa
justin.garcia: review+
fixes the bug. The previous patch had wrong relative path with "WebKit5/" at the beginning (5.90 KB, patch)
2009-06-17 14:53 PDT, Ryosuke Niwa
mjs: review+
Ryosuke Niwa
Comment 1 2009-06-12 16:28:56 PDT
My guess is that the test should look something like this: <p>This tests for a bug where delete would crash if a node just after the selection to delete is removed in a DOMNodeRemoved event handler for the last node in the selection to delete.</p> <div id="div" contenteditable="true">foo<span id="foo">x</span><span id="removeme">y</span></div> <p id="console"></p> <script> if (window.layoutTestController) window.layoutTestController.dumpAsText(); function log(message) { var console = document.getElementById("console"); var text = document.createTextNode(message); console.appendChild(text); } function removeHandler() { var removeme = document.getElementById("removeme"); removeme.parentNode.removeChild(removeme); } document.getElementById("foo").addEventListener("DOMNodeRemoved", removeHandler); var sel = window.getSelection(); sel.setBaseAndExtent( document.getElementById('foo'), 0, document.getElementById('foo'), 1 ); document.execCommand("Delete"); log(document.getElementById('div'); </script>
Justin Garcia
Comment 2 2009-06-12 19:11:46 PDT
> My guess is that the test should look something like this: Looks right.
Ryosuke Niwa
Comment 3 2009-06-12 23:41:33 PDT
Thanks, Justin. I'll make a patch for it since I've been already rewriting a lot of test cases in editing\deleting.
Ryosuke Niwa
Comment 4 2009-06-17 11:48:49 PDT
Created attachment 31428 [details] This patch fixes the bug, and converts the text from a pixel test to a dumpAsText test.
Ryosuke Niwa
Comment 5 2009-06-17 14:53:54 PDT
Created attachment 31442 [details] fixes the bug. The previous patch had wrong relative path with "WebKit5/" at the beginning Sorry Justin, I didn't know that I had to svn-create-patch on src/. I had "WebKit5/" all over the patch.
Maciej Stachowiak
Comment 6 2009-06-18 09:01:42 PDT
Comment on attachment 31442 [details] fixes the bug. The previous patch had wrong relative path with "WebKit5/" at the beginning r=me This might be a good candidate for a dumpAsMarkup() text if we add that mode.
Ryosuke Niwa
Comment 7 2009-06-18 11:17:46 PDT
(In reply to comment #6) > (From update of attachment 31442 [details] [review]) > r=me > > This might be a good candidate for a dumpAsMarkup() text if we add that mode. > I agree.
Brent Fulgham
Comment 8 2009-06-19 17:00:41 PDT
Assign for landing.
Brent Fulgham
Comment 9 2009-06-19 17:30:34 PDT
Note You need to log in before you can comment on or make changes to this bug.