Bug 55354

Created attachment 84023 [details] work in progress

Now that the bug 51389 has been fixed, the only thing that prevents me from posting a patch for this bug is TypingCommand::forwardDeleteKeyPressed: if (!startingSelection().isRange() || selectionToDelete.base() != startingSelection().start()) selectionAfterUndo = selectionToDelete; else { // It's a little tricky to compute what the starting selection would have been in the original document. // We can't let the VisibleSelection class's validation kick in or it'll adjust for us based on // the current state of the document and we'll get the wrong result. Position extent = startingSelection().end(); if (extent.deprecatedNode() != selectionToDelete.end().deprecatedNode()) extent = selectionToDelete.extent(); else { int extraCharacters; if (selectionToDelete.start().deprecatedNode() == selectionToDelete.end().deprecatedNode()) extraCharacters = selectionToDelete.end().deprecatedEditingOffset() - selectionToDelete.start().deprecatedEditingOffset(); else extraCharacters = selectionToDelete.end().deprecatedEditingOffset(); @ extent = Position(extent.deprecatedNode(), extent.deprecatedEditingOffset() + extraCharacters, Position::PositionIsOffsetInAnchor); } selectionAfterUndo.setWithoutValidation(startingSelection().start(), extent); } In running editing/undo/undo-forward-delete-boundary.html, we forward delete each letter in "wo<b>rd</b> and undo it. When we forward delete the first letter, we get the following values at @: (gdb) p startingSelection().showTreeForThis() BODY 0x129859dc0 #text 0x1298617c0 "\n" DIV 0x129862a60 CLASS=editing #text 0x1298604e0 "\n" SPAN 0x129860540 SE #text 0x129848440 "This o" B 0x12985d950 #text 0x129868c60 "rd " #text 0x12985bae0 "should be selected, since the test deleted it a character at a time and then did an undo." #text 0x12985bbc0 "\n" #text 0x129858560 "\n\n" SCRIPT 0x1298585c0 #text 0x1298586e0 "\nrunEditingTest();\n" start: legacy, offset, offset:5 end: legacy, offset, offset:6 $1 = void Current language: auto; currently c++ (gdb) p selectionToDelete.showTreeForThis() BODY 0x129859dc0 #text 0x1298617c0 "\n" DIV 0x129862a60 CLASS=editing #text 0x1298604e0 "\n" SPAN 0x129860540 SE #text 0x129848440 "This o" B 0x12985d950 #text 0x129868c60 "rd " #text 0x12985bae0 "should be selected, since the test deleted it a character at a time and then did an undo." #text 0x12985bbc0 "\n" #text 0x129858560 "\n\n" SCRIPT 0x1298585c0 #text 0x1298586e0 "\nrunEditingTest();\n" start: legacy, offset, offset:5 end: legacy, offset, offset:6 $2 = void And we end up creating a position with offset 7 to incorporate "w" that has already been deleted. In various parts of editing code, we do need to store such "invalid" positions for undo/redo operations. Maybe that we need a variant of Position (PositionForUndo or UndoablePosition?) that tolerates such positions.

Created attachment 84529 [details] work in progress 2