WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Sergio Villar Senin
Comment 1
2013-03-27 09:23:32 PDT
Created
attachment 195341
[details]
Patch
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
Committed
r146999
: <
http://trac.webkit.org/changeset/146999
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug