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;