Start with the following markup: <div>hello</div><div>world</div> Select all, copy and paste over the same content. The markup produced is: <div><div>hello</div><div>world</div></div> Select all again and paste. The markup produced is: <div><div><div>hello</div><div>world</div></div></div> Every time you select and paste one more div is added. Deleting the selection will leave all the nested divs. <div><div><div><br></div></div></div>
Created attachment 117294 [details] Patch
Comment on attachment 117294 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117294&action=review r=me; but the use of deprecatedNode here in this new code seems like a bad idea. > Source/WebCore/editing/DeleteSelectionCommand.cpp:752 > + Node* node = m_endingPosition.deprecatedNode(); Do we have to use deprecatedNode? Can this be containerNode or anchorNode instead? > Source/WebCore/editing/DeleteSelectionCommand.cpp:758 > + node = node->parentNode(); This should be: node = parentNode; > Source/WebCore/editing/DeleteSelectionCommand.cpp:763 > + if (node == m_endingPosition.deprecatedNode()) Do we have to use deprecatedNode? Can this be containerNode or anchorNode instead? > Source/WebCore/editing/DeleteSelectionCommand.cpp:767 > + node = m_endingPosition.deprecatedNode(); Do we have to use deprecatedNode? Can this be containerNode or anchorNode instead? > Source/WebCore/editing/DeleteSelectionCommand.cpp:769 > + node = node->parentNode(); This should be: node = parentNode; > LayoutTests/editing/deleting/delete-and-cleanup.html:31 > + confimedMarkup = node.innerHTML; Typo here: "confimedMarkup".
> Do we have to use deprecatedNode? Can this be containerNode or anchorNode instead? No, it should be anchorNode. Old habit die hard :-) > This should be: > > node = parentNode; Done. > > LayoutTests/editing/deleting/delete-and-cleanup.html:31 > > + confimedMarkup = node.innerHTML; > > Typo here: "confimedMarkup". Fixed: it was wrong on both tests.
Comment on attachment 117294 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117294&action=review >> Source/WebCore/editing/DeleteSelectionCommand.cpp:752 >> + Node* node = m_endingPosition.deprecatedNode(); > > Do we have to use deprecatedNode? Can this be containerNode or anchorNode instead? containerNode should be used here since if m_endingPosition was before or after a div, then we're outside of that div. >> Source/WebCore/editing/DeleteSelectionCommand.cpp:763 >> + if (node == m_endingPosition.deprecatedNode()) > > Do we have to use deprecatedNode? Can this be containerNode or anchorNode instead? This should be containerNode if we start our start from containerNode. >> Source/WebCore/editing/DeleteSelectionCommand.cpp:767 >> + node = m_endingPosition.deprecatedNode(); > > Do we have to use deprecatedNode? Can this be containerNode or anchorNode instead? Ditto.
Committed revision 101575.
The patch you landed seems to still have one deprecatedNode.
(In reply to comment #6) > The patch you landed seems to still have one deprecatedNode. I'll fix it.
(In reply to comment #7) > (In reply to comment #6) > > The patch you landed seems to still have one deprecatedNode. > I'll fix it. Fixed in revision 101830.