Inserting a blank (" ") at the end of a line does not insert anything in Overtype mode

• Component: HTML Editing
• Status: RESOLVED FIXED

[Sergio Villar Senin]

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.

[Sergio Villar Senin]

Created attachment 195341 [details]
Patch

[Ryosuke Niwa]

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]

(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 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]

Committed r146999: <http://trac.webkit.org/changeset/146999>