Summary: | convert editing/deleting/5156801-2.html to dumpAsText and rename | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Wyatt Carss <wcarss> | ||||||||||||
Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Enhancement | CC: | jparent, ojan, rniwa, tony, wcarss, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Attachments: |
|
Description
Wyatt Carss
2011-06-24 15:22:54 PDT
Created attachment 98550 [details]
Patch
Created attachment 98557 [details]
Patch
Renamed, converted, and rdar info included Comment on attachment 98557 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98557&action=review > LayoutTests/editing/deleting/delete-node-after-DOMNodeRemoved-2.html:2 > +<p>This tests for a crash when deleting the contents of a table cell. You should just see 'Cached' below.</p> Please add rdar id. > LayoutTests/editing/deleting/delete-node-after-DOMNodeRemoved-2.html:16 > +s = window.getSelection(); > +table = document.getElementById("table"); > +s.setPosition(table, table.childNodes.length); > +s.modify("move", "backward", "character"); > +document.execCommand("Delete"); > +document.execCommand("Delete"); > +document.execCommand("Delete"); > +document.execCommand("Delete"); > +document.execCommand("Delete"); > +document.execCommand("Delete"); > +document.execCommand("Delete"); What is this test to do with DOMNodeRemoved? That's a DOM level 3 event. I don't see an event listener attached in this test case. r- due to this. Created attachment 98755 [details]
Patch
The rdar was present already; the diff shows the old file's deletion, and then the new file's creation with identical contents to the old. Then it shows the changes to the new file separately. I've renamed the second test, because its only relation was through the rdar number - there's no reason I know of that these tests have to be kept together. Comment on attachment 98755 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98755&action=review > LayoutTests/editing/deleting/delete-table-cell-contents-expected.txt:4 > + > +Cached You should use dump-as-markup.js here because it's significant that "Cached" is inside a table cell. Created attachment 98774 [details]
Patch
Comment on attachment 98774 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98774&action=review > LayoutTests/editing/deleting/delete-table-cell-contents.html:24 > +if (window.layoutTestController) { > + Markup.description(description.textContent); > + Markup.dump("test"); > +} You shouldn't wrap Markup in window.layoutTestController. Markup takes care of that. Created attachment 98818 [details]
Patch
Comment on attachment 98818 [details] Patch Clearing flags on attachment: 98818 Committed r89883: <http://trac.webkit.org/changeset/89883> All reviewed patches have been landed. Closing bug. |