Fix undo and redo actions for replacing string.
1. Add embeddertest for ReplaceSelection action.
2. Add CFXEU_ReplaceSel to |m_UndoItemStack| to indicate the
beginning and end of a replace action. So every time
ReplaceSelection() is called, the size of |m_UndoItemStack| will
increase by 4 (Including CFXEU_Clear, CFXEU_InsertText and two
CFXEU_ReplaceSels as begin/end markers). When undo/redo is
performed for ReplaceSelection(), |m_UndoItemStack| skips the
CFXEU_ReplaceSel markers, and keeps processing undo/redo for
InsertText() and ClearSelection() actions.
Bug: pdfium:838
Change-Id: Ia0e30a7cef9268d03e8d34c11eded9f1511798bc
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/56671
Commit-Queue: Hui Yingst <nigi@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/fpdfsdk/fpdf_formfill_embeddertest.cpp b/fpdfsdk/fpdf_formfill_embeddertest.cpp
index d5f6fba..bfcbe8c 100644
--- a/fpdfsdk/fpdf_formfill_embeddertest.cpp
+++ b/fpdfsdk/fpdf_formfill_embeddertest.cpp
@@ -2270,3 +2270,51 @@
}
CheckFocusedFieldText(L"Banana");
}
+
+TEST_F(FPDFFormFillTextFormEmbedderTest, ReplaceSelection) {
+ ScopedFPDFWideString text_to_insert = GetFPDFWideString(L"XYZ");
+ ClickOnFormFieldAtPoint(RegularFormBegin());
+ CheckCanUndo(false);
+ CheckCanRedo(false);
+
+ TypeTextIntoTextField(2, RegularFormBegin());
+ CheckFocusedFieldText(L"AB");
+ CheckSelection(L"");
+ SelectTextWithKeyboard(1, FWL_VKEY_Right, RegularFormBegin());
+ CheckSelection(L"A");
+
+ FORM_ReplaceSelection(form_handle(), page(), text_to_insert.get());
+ CheckFocusedFieldText(L"XYZB");
+ CheckCanUndo(true);
+ CheckCanRedo(false);
+
+ PerformUndo();
+ CheckFocusedFieldText(L"AB");
+ CheckCanUndo(true);
+ CheckCanRedo(true);
+
+ PerformUndo();
+ CheckFocusedFieldText(L"A");
+ CheckCanUndo(true);
+ CheckCanRedo(true);
+
+ PerformUndo();
+ CheckFocusedFieldText(L"");
+ CheckCanUndo(false);
+ CheckCanRedo(true);
+
+ PerformRedo();
+ CheckFocusedFieldText(L"A");
+ CheckCanUndo(true);
+ CheckCanRedo(true);
+
+ PerformRedo();
+ CheckFocusedFieldText(L"AB");
+ CheckCanUndo(true);
+ CheckCanRedo(true);
+
+ PerformRedo();
+ CheckFocusedFieldText(L"XYZB");
+ CheckCanUndo(true);
+ CheckCanRedo(false);
+}
diff --git a/fpdfsdk/pwl/cpwl_edit.cpp b/fpdfsdk/pwl/cpwl_edit.cpp
index d006e55..cce0b1c 100644
--- a/fpdfsdk/pwl/cpwl_edit.cpp
+++ b/fpdfsdk/pwl/cpwl_edit.cpp
@@ -406,8 +406,7 @@
}
void CPWL_Edit::ReplaceSel(const WideString& wsText) {
- m_pEdit->ClearSelection();
- m_pEdit->InsertText(wsText, FX_CHARSET_Default);
+ m_pEdit->ReplaceSelection(wsText);
}
CFX_FloatRect CPWL_Edit::GetFocusRect() const {
diff --git a/fpdfsdk/pwl/cpwl_edit_ctrl.cpp b/fpdfsdk/pwl/cpwl_edit_ctrl.cpp
index 818e476..7e03e55 100644
--- a/fpdfsdk/pwl/cpwl_edit_ctrl.cpp
+++ b/fpdfsdk/pwl/cpwl_edit_ctrl.cpp
@@ -55,8 +55,7 @@
}
void CPWL_EditCtrl::ReplaceSelection(const WideString& text) {
- m_pEdit->ClearSelection();
- m_pEdit->InsertText(text, FX_CHARSET_Default);
+ m_pEdit->ReplaceSelection(text);
}
bool CPWL_EditCtrl::RePosChildWnd() {
diff --git a/fpdfsdk/pwl/cpwl_edit_impl.cpp b/fpdfsdk/pwl/cpwl_edit_impl.cpp
index 23eee8b..0988c1f 100644
--- a/fpdfsdk/pwl/cpwl_edit_impl.cpp
+++ b/fpdfsdk/pwl/cpwl_edit_impl.cpp
@@ -210,10 +210,13 @@
void CPWL_EditImpl_Undo::Undo() {
m_bWorking = true;
- if (CanUndo()) {
- m_UndoItemStack[m_nCurUndoPos - 1]->Undo();
+ int nUndoRemain = 1;
+ while (CanUndo() && nUndoRemain > 0) {
+ nUndoRemain += m_UndoItemStack[m_nCurUndoPos - 1]->Undo();
m_nCurUndoPos--;
+ nUndoRemain--;
}
+ ASSERT(nUndoRemain == 0);
m_bWorking = false;
}
@@ -223,10 +226,13 @@
void CPWL_EditImpl_Undo::Redo() {
m_bWorking = true;
- if (CanRedo()) {
- m_UndoItemStack[m_nCurUndoPos]->Redo();
+ int nRedoRemain = 1;
+ while (CanRedo() && nRedoRemain > 0) {
+ nRedoRemain += m_UndoItemStack[m_nCurUndoPos]->Redo();
m_nCurUndoPos++;
+ nRedoRemain--;
}
+ ASSERT(nRedoRemain == 0);
m_bWorking = false;
}
@@ -268,16 +274,18 @@
CFXEU_InsertWord::~CFXEU_InsertWord() {}
-void CFXEU_InsertWord::Redo() {
+int CFXEU_InsertWord::Redo() {
m_pEdit->SelectNone();
m_pEdit->SetCaret(m_wpOld);
m_pEdit->InsertWord(m_Word, m_nCharset, false, true);
+ return 0;
}
-void CFXEU_InsertWord::Undo() {
+int CFXEU_InsertWord::Undo() {
m_pEdit->SelectNone();
m_pEdit->SetCaret(m_wpNew);
m_pEdit->Backspace(false, true);
+ return 0;
}
CFXEU_InsertReturn::CFXEU_InsertReturn(CPWL_EditImpl* pEdit,
@@ -289,16 +297,43 @@
CFXEU_InsertReturn::~CFXEU_InsertReturn() {}
-void CFXEU_InsertReturn::Redo() {
+int CFXEU_InsertReturn::Redo() {
m_pEdit->SelectNone();
m_pEdit->SetCaret(m_wpOld);
m_pEdit->InsertReturn(false, true);
+ return 0;
}
-void CFXEU_InsertReturn::Undo() {
+int CFXEU_InsertReturn::Undo() {
m_pEdit->SelectNone();
m_pEdit->SetCaret(m_wpNew);
m_pEdit->Backspace(false, true);
+ return 0;
+}
+
+CFXEU_ReplaceSelection::CFXEU_ReplaceSelection(CPWL_EditImpl* pEdit,
+ bool bIsEnd)
+ : m_pEdit(pEdit), m_bEnd(bIsEnd) {
+ ASSERT(m_pEdit);
+}
+
+CFXEU_ReplaceSelection::~CFXEU_ReplaceSelection() {}
+
+int CFXEU_ReplaceSelection::Redo() {
+ m_pEdit->SelectNone();
+ if (IsEnd())
+ return 0;
+ // Redo ClearSelection, InsertText and ReplaceSelection's end marker
+ return 3;
+}
+
+int CFXEU_ReplaceSelection::Undo() {
+ m_pEdit->SelectNone();
+ if (!IsEnd())
+ return 0;
+ // Undo InsertText, ClearSelection and ReplaceSelection's beginning
+ // marker
+ return 3;
}
CFXEU_Backspace::CFXEU_Backspace(CPWL_EditImpl* pEdit,
@@ -316,19 +351,21 @@
CFXEU_Backspace::~CFXEU_Backspace() {}
-void CFXEU_Backspace::Redo() {
+int CFXEU_Backspace::Redo() {
m_pEdit->SelectNone();
m_pEdit->SetCaret(m_wpOld);
m_pEdit->Backspace(false, true);
+ return 0;
}
-void CFXEU_Backspace::Undo() {
+int CFXEU_Backspace::Undo() {
m_pEdit->SelectNone();
m_pEdit->SetCaret(m_wpNew);
if (m_wpNew.nSecIndex != m_wpOld.nSecIndex)
m_pEdit->InsertReturn(false, true);
else
m_pEdit->InsertWord(m_Word, m_nCharset, false, true);
+ return 0;
}
CFXEU_Delete::CFXEU_Delete(CPWL_EditImpl* pEdit,
@@ -348,19 +385,21 @@
CFXEU_Delete::~CFXEU_Delete() {}
-void CFXEU_Delete::Redo() {
+int CFXEU_Delete::Redo() {
m_pEdit->SelectNone();
m_pEdit->SetCaret(m_wpOld);
m_pEdit->Delete(false, true);
+ return 0;
}
-void CFXEU_Delete::Undo() {
+int CFXEU_Delete::Undo() {
m_pEdit->SelectNone();
m_pEdit->SetCaret(m_wpNew);
if (m_bSecEnd)
m_pEdit->InsertReturn(false, true);
else
m_pEdit->InsertWord(m_Word, m_nCharset, false, true);
+ return 0;
}
CFXEU_Clear::CFXEU_Clear(CPWL_EditImpl* pEdit,
@@ -372,17 +411,19 @@
CFXEU_Clear::~CFXEU_Clear() {}
-void CFXEU_Clear::Redo() {
+int CFXEU_Clear::Redo() {
m_pEdit->SelectNone();
m_pEdit->SetSelection(m_wrSel.BeginPos, m_wrSel.EndPos);
m_pEdit->Clear(false, true);
+ return 0;
}
-void CFXEU_Clear::Undo() {
+int CFXEU_Clear::Undo() {
m_pEdit->SelectNone();
m_pEdit->SetCaret(m_wrSel.BeginPos);
m_pEdit->InsertText(m_swText, FX_CHARSET_Default, false, true);
m_pEdit->SetSelection(m_wrSel.BeginPos, m_wrSel.EndPos);
+ return 0;
}
CFXEU_InsertText::CFXEU_InsertText(CPWL_EditImpl* pEdit,
@@ -400,16 +441,18 @@
CFXEU_InsertText::~CFXEU_InsertText() {}
-void CFXEU_InsertText::Redo() {
+int CFXEU_InsertText::Redo() {
m_pEdit->SelectNone();
m_pEdit->SetCaret(m_wpOld);
m_pEdit->InsertText(m_swText, m_nCharset, false, true);
+ return 0;
}
-void CFXEU_InsertText::Undo() {
+int CFXEU_InsertText::Undo() {
m_pEdit->SelectNone();
m_pEdit->SetSelection(m_wpOld, m_wpNew);
m_pEdit->Clear(false, true);
+ return 0;
}
// static
@@ -1685,6 +1728,13 @@
}
}
+void CPWL_EditImpl::ReplaceSelection(const WideString& text) {
+ AddEditUndoItem(pdfium::MakeUnique<CFXEU_ReplaceSelection>(this, false));
+ ClearSelection();
+ InsertText(text, FX_CHARSET_Default);
+ AddEditUndoItem(pdfium::MakeUnique<CFXEU_ReplaceSelection>(this, true));
+}
+
bool CPWL_EditImpl::Redo() {
if (m_bEnableUndo) {
if (m_Undo.CanRedo()) {
diff --git a/fpdfsdk/pwl/cpwl_edit_impl.h b/fpdfsdk/pwl/cpwl_edit_impl.h
index 0472863..889ed78 100644
--- a/fpdfsdk/pwl/cpwl_edit_impl.h
+++ b/fpdfsdk/pwl/cpwl_edit_impl.h
@@ -98,8 +98,16 @@
public:
virtual ~IFX_Edit_UndoItem() = default;
- virtual void Undo() = 0;
- virtual void Redo() = 0;
+ // Undo/Redo the current undo item and returns the number of additional items
+ // to be processed in |m_UndoItemStack| to fully undo/redo the action. (An
+ // example is CFXEU_ReplaceSelection::Undo(), if CFXEU_ReplaceSelection marks
+ // the end of a replace action, CFXEU_ReplaceSelection::Undo() returns 3
+ // because 3 more undo items need to be processed to revert the replace
+ // action: insert text, clear selection and the CFXEU_ReplaceSelection which
+ // marks the beginning of replace action.) Implementations should return 0 by
+ // default.
+ virtual int Undo() = 0;
+ virtual int Redo() = 0;
};
class CFXEU_InsertWord final : public IFX_Edit_UndoItem {
@@ -112,8 +120,8 @@
~CFXEU_InsertWord() override;
// IFX_Edit_UndoItem:
- void Redo() override;
- void Undo() override;
+ int Redo() override;
+ int Undo() override;
private:
UnownedPtr<CPWL_EditImpl> m_pEdit;
@@ -132,8 +140,8 @@
~CFXEU_InsertReturn() override;
// IFX_Edit_UndoItem:
- void Redo() override;
- void Undo() override;
+ int Redo() override;
+ int Undo() override;
private:
UnownedPtr<CPWL_EditImpl> m_pEdit;
@@ -142,6 +150,22 @@
CPVT_WordPlace m_wpNew;
};
+class CFXEU_ReplaceSelection final : public IFX_Edit_UndoItem {
+ public:
+ CFXEU_ReplaceSelection(CPWL_EditImpl* pEdit, bool bIsEnd);
+ ~CFXEU_ReplaceSelection() override;
+
+ // IFX_Edit_UndoItem:
+ int Redo() override;
+ int Undo() override;
+
+ private:
+ bool IsEnd() const { return m_bEnd; }
+
+ UnownedPtr<CPWL_EditImpl> m_pEdit;
+ const bool m_bEnd; // indicate whether this is the end of replace action
+};
+
class CFXEU_Backspace final : public IFX_Edit_UndoItem {
public:
CFXEU_Backspace(CPWL_EditImpl* pEdit,
@@ -152,8 +176,8 @@
~CFXEU_Backspace() override;
// IFX_Edit_UndoItem:
- void Redo() override;
- void Undo() override;
+ int Redo() override;
+ int Undo() override;
private:
UnownedPtr<CPWL_EditImpl> m_pEdit;
@@ -175,8 +199,8 @@
~CFXEU_Delete() override;
// IFX_Edit_UndoItem:
- void Redo() override;
- void Undo() override;
+ int Redo() override;
+ int Undo() override;
private:
UnownedPtr<CPWL_EditImpl> m_pEdit;
@@ -196,8 +220,8 @@
~CFXEU_Clear() override;
// IFX_Edit_UndoItem:
- void Redo() override;
- void Undo() override;
+ int Redo() override;
+ int Undo() override;
private:
UnownedPtr<CPWL_EditImpl> m_pEdit;
@@ -216,8 +240,8 @@
~CFXEU_InsertText() override;
// IFX_Edit_UndoItem:
- void Redo() override;
- void Undo() override;
+ int Redo() override;
+ int Undo() override;
private:
UnownedPtr<CPWL_EditImpl> m_pEdit;
@@ -289,6 +313,7 @@
bool Delete();
bool ClearSelection();
bool InsertText(const WideString& sText, int32_t charset);
+ void ReplaceSelection(const WideString& text);
bool Redo();
bool Undo();
CPVT_WordPlace WordIndexToWordPlace(int32_t index) const;
