RESOLVED FIXED 113413
Inserting a blank (" ") at the end of a line does not insert anything in Overtype mode
https://bugs.webkit.org/show_bug.cgi?id=113413
Summary Inserting a blank (" ") at the end of a line does not insert anything in Over...
Sergio Villar Senin
Reported 2013-03-27 08:54:18 PDT
That's because we're still calling replaceTextInNode() in that case which is wrong because the replace code makes some assumptions that are not valid for this case.
Attachments
Patch (9.00 KB, patch)
2013-03-27 09:23 PDT, Sergio Villar Senin
rniwa: review+
Sergio Villar Senin
Comment 1 2013-03-27 09:23:32 PDT
Ryosuke Niwa
Comment 2 2013-03-27 09:27:40 PDT
Comment on attachment 195341 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195341&action=review > Source/WebCore/editing/InsertTextCommand.cpp:106 > + if (!selectInsertedText) > + setEndingSelection(VisibleSelection(endingSelection().visibleEnd(), endingSelection().isDirectional())); Do we have a test for this? > LayoutTests/editing/execCommand/overtype.html:33 > - document.execCommand("InsertText", false, 'FO'); > - Markup.dump(element, 'After overwritting the first two characters'); > + document.execCommand("InsertText", false, 'F'); > + Markup.dump(element, 'After overwritting the first character'); Why are we modifying this test case?
Sergio Villar Senin
Comment 3 2013-03-27 10:53:21 PDT
(In reply to comment #2) > (From update of attachment 195341 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=195341&action=review > > > Source/WebCore/editing/InsertTextCommand.cpp:106 > > + if (!selectInsertedText) > > + setEndingSelection(VisibleSelection(endingSelection().visibleEnd(), endingSelection().isDirectional())); > > Do we have a test for this? What do you mean? For the case selectInsertedText == true ? Note that I am not adding any code in that case, it's just a refactoring to allow us to use setEndingSelectionWithoutValidation(). Not really required by the patch, but I just found it convenient. > > LayoutTests/editing/execCommand/overtype.html:33 > > - document.execCommand("InsertText", false, 'FO'); > > - Markup.dump(element, 'After overwritting the first two characters'); > > + document.execCommand("InsertText", false, 'F'); > > + Markup.dump(element, 'After overwritting the first character'); > > Why are we modifying this test case? Just because the original work was "foo" and as I wanted to overwrite with withespace in the middle and in the end.
Ryosuke Niwa
Comment 4 2013-03-27 10:56:07 PDT
Comment on attachment 195341 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195341&action=review >>> Source/WebCore/editing/InsertTextCommand.cpp:106 >>> + setEndingSelection(VisibleSelection(endingSelection().visibleEnd(), endingSelection().isDirectional())); >> >> Do we have a test for this? > > What do you mean? For the case selectInsertedText == true ? Note that I am not adding any code in that case, it's just a refactoring to allow us to use setEndingSelectionWithoutValidation(). Not really required by the patch, but I just found it convenient. Oh, I see. Sorry, I misread your patch.
Sergio Villar Senin
Comment 5 2013-03-27 11:24:35 PDT
Note You need to log in before you can comment on or make changes to this bug.