REGRESSION(r8780): Backwards delete by word incorrectly appends deleted text to kill ring, should be prepend
https://bugs.webkit.org/show_bug.cgi?id=151300
Reviewed by Darin Adler.
Source/WebCore:
Over 11 years ago, someone was in a big hurry to fix a bunch
of emacs keybindings bugs, and accidentally regressed the kill ring
behavior for backwards-delete-word. It should prepend to the beginning.
This patch fixes the regression and cleans up the kill ring-related
code in Editor and commands. It also adds some tests to cover the
regressed code a bit better.
Tests: editing/pasteboard/emacs-killring-alternating-append-prepend.html
editing/pasteboard/emacs-killring-backward-delete-prepend.html
* editing/Editor.cpp:
Use more explicit names for insertion mode parameters and member variables.
(WebCore::Editor::deleteWithDirection):
(WebCore::Editor::performDelete):
(WebCore::Editor::addRangeToKillRing):
(WebCore::Editor::addTextToKillRing):
Only one call site for now, but another will be added in a dependent fix.
(WebCore::Editor::addToKillRing): Deleted.
* editing/Editor.h:
* editing/TypingCommand.cpp:
(WebCore::TypingCommand::TypingCommand):
(WebCore::TypingCommand::deleteKeyPressed):
(WebCore::TypingCommand::forwardDeleteKeyPressed):
(WebCore::TypingCommand::doApply):
* editing/TypingCommand.h:
* platform/mac/KillRingMac.mm:
(WebCore::KillRing::append):
(WebCore::KillRing::prepend):
It turns out that the native API implicitly clears the kill sequence when
alternating between prepend and append operations. Its behavior does not match
what Sublime Text or Emacs do in this case. Clear the previous operation flag
to prevent this behavior from happening.
LayoutTests:
* editing/pasteboard/emacs-killring-alternating-append-prepend-expected.txt: Added.
* editing/pasteboard/emacs-killring-alternating-append-prepend.html: Added.
* editing/pasteboard/emacs-killring-backward-delete-prepend-expected.txt: Added.
* editing/pasteboard/emacs-killring-backward-delete-prepend.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@192641 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/editing/TypingCommand.cpp b/Source/WebCore/editing/TypingCommand.cpp
index f074448..801b53f 100644
--- a/Source/WebCore/editing/TypingCommand.cpp
+++ b/Source/WebCore/editing/TypingCommand.cpp
@@ -83,7 +83,7 @@
, m_smartDelete(options & SmartDelete)
, m_granularity(granularity)
, m_compositionType(compositionType)
- , m_killRing(options & KillRing)
+ , m_shouldAddToKillRing(options & AddsToKillRing)
, m_openedByBackwardDelete(false)
, m_shouldRetainAutocorrectionIndicator(options & RetainAutocorrectionIndicator)
, m_shouldPreventSpellChecking(options & PreventSpellChecking)
@@ -114,7 +114,7 @@
if (RefPtr<TypingCommand> lastTypingCommand = lastTypingCommandIfStillOpenForTyping(document.frame())) {
updateSelectionIfDifferentFromCurrentSelection(lastTypingCommand.get(), document.frame());
lastTypingCommand->setShouldPreventSpellChecking(options & PreventSpellChecking);
- lastTypingCommand->deleteKeyPressed(granularity, options & KillRing);
+ lastTypingCommand->deleteKeyPressed(granularity, options & AddsToKillRing);
return;
}
}
@@ -130,7 +130,7 @@
if (RefPtr<TypingCommand> lastTypingCommand = lastTypingCommandIfStillOpenForTyping(frame)) {
updateSelectionIfDifferentFromCurrentSelection(lastTypingCommand.get(), frame);
lastTypingCommand->setShouldPreventSpellChecking(options & PreventSpellChecking);
- lastTypingCommand->forwardDeleteKeyPressed(granularity, options & KillRing);
+ lastTypingCommand->forwardDeleteKeyPressed(granularity, options & AddsToKillRing);
return;
}
}
@@ -263,10 +263,10 @@
deleteSelection(m_smartDelete);
return;
case DeleteKey:
- deleteKeyPressed(m_granularity, m_killRing);
+ deleteKeyPressed(m_granularity, m_shouldAddToKillRing);
return;
case ForwardDeleteKey:
- forwardDeleteKeyPressed(m_granularity, m_killRing);
+ forwardDeleteKeyPressed(m_granularity, m_shouldAddToKillRing);
return;
case InsertLineBreak:
insertLineBreak();
@@ -433,7 +433,7 @@
return true;
}
-void TypingCommand::deleteKeyPressed(TextGranularity granularity, bool killRing)
+void TypingCommand::deleteKeyPressed(TextGranularity granularity, bool shouldAddToKillRing)
{
Frame& frame = this->frame();
@@ -458,7 +458,7 @@
FrameSelection selection;
selection.setSelection(endingSelection());
selection.modify(FrameSelection::AlterationExtend, DirectionBackward, granularity);
- if (killRing && selection.isCaret() && granularity != CharacterGranularity)
+ if (shouldAddToKillRing && selection.isCaret() && granularity != CharacterGranularity)
selection.modify(FrameSelection::AlterationExtend, DirectionBackward, CharacterGranularity);
if (endingSelection().visibleStart().previous(CannotCrossEditingBoundary).isNull()) {
@@ -530,8 +530,8 @@
if (selectionToDelete.isCaret() || !frame.selection().shouldDeleteSelection(selectionToDelete))
return;
- if (killRing)
- frame.editor().addToKillRing(selectionToDelete.toNormalizedRange().get(), false);
+ if (shouldAddToKillRing)
+ frame.editor().addRangeToKillRing(*selectionToDelete.toNormalizedRange().get(), Editor::KillRingInsertionMode::PrependText);
// Make undo select everything that has been deleted, unless an undo will undo more than just this deletion.
// FIXME: This behaves like TextEdit except for the case where you open with text insertion and then delete
// more text than you insert. In that case all of the text that was around originally should be selected.
@@ -542,7 +542,7 @@
typingAddedToOpenCommand(DeleteKey);
}
-void TypingCommand::forwardDeleteKeyPressed(TextGranularity granularity, bool killRing)
+void TypingCommand::forwardDeleteKeyPressed(TextGranularity granularity, bool shouldAddToKillRing)
{
Frame& frame = this->frame();
@@ -565,7 +565,7 @@
FrameSelection selection;
selection.setSelection(endingSelection());
selection.modify(FrameSelection::AlterationExtend, DirectionForward, granularity);
- if (killRing && selection.isCaret() && granularity != CharacterGranularity)
+ if (shouldAddToKillRing && selection.isCaret() && granularity != CharacterGranularity)
selection.modify(FrameSelection::AlterationExtend, DirectionForward, CharacterGranularity);
Position downstreamEnd = endingSelection().end().downstream();
@@ -628,8 +628,8 @@
if (selectionToDelete.isCaret() || !frame.selection().shouldDeleteSelection(selectionToDelete))
return;
- if (killRing)
- frame.editor().addToKillRing(selectionToDelete.toNormalizedRange().get(), false);
+ if (shouldAddToKillRing)
+ frame.editor().addRangeToKillRing(*selectionToDelete.toNormalizedRange().get(), Editor::KillRingInsertionMode::AppendText);
// make undo select what was deleted
setStartingSelection(selectionAfterUndo);
CompositeEditCommand::deleteSelection(selectionToDelete, m_smartDelete);
