Fix misalignment between Redo and Undo after consecutive text pasting.
When inputting the text 'aaa' 'b' 'ccc' and performing Undo.
Before the fix, the text would be 'aaa'.
After the fix, the text would be 'aaa' 'b'.
Bug: chromium:1448416
Change-Id: I4c0175e40aa485d66b6344d16ed3dd7b488d1230
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/113350
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/AUTHORS b/AUTHORS
index d3c30ba..0689774 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -14,6 +14,7 @@
Abdelkarim Sellamna <abdelkarim.se@gmail.com>
Aleksei Skotnikov <fineaskotnikov@gmail.com>
Antonio Gomes <tonikitoo@igalia.com>
+Chenguang Shao <chenguangshao1@gmail.com>
Chery Cherian <cherycherian@gmail.com>
Claudio DeSouza <claudiomdsjr@gmail.com>
Dan Ilan <danilan@gmail.com>
diff --git a/fpdfsdk/fpdf_formfill_embeddertest.cpp b/fpdfsdk/fpdf_formfill_embeddertest.cpp
index 2842ec3..71bede8 100644
--- a/fpdfsdk/fpdf_formfill_embeddertest.cpp
+++ b/fpdfsdk/fpdf_formfill_embeddertest.cpp
@@ -3292,6 +3292,41 @@
CheckCanRedo(false);
}
+TEST_F(FPDFFormFillTextFormEmbedderTest, ContinuouslyReplaceAndKeepSelection) {
+ ScopedFPDFWideString text_to_insert1 = GetFPDFWideString(L"UVW");
+
+ ClickOnFormFieldAtPoint(RegularFormBegin());
+ CheckFocusedFieldText(L"");
+ CheckCanUndo(false);
+ CheckCanRedo(false);
+
+ FORM_ReplaceAndKeepSelection(form_handle(), page(), text_to_insert1.get());
+ CheckFocusedFieldText(L"UVW");
+ CheckSelection(L"UVW");
+
+ CheckCanUndo(true);
+ CheckCanRedo(false);
+
+ PerformUndo();
+ CheckFocusedFieldText(L"");
+
+ CheckCanUndo(false);
+ CheckCanRedo(true);
+ PerformRedo();
+ CheckFocusedFieldText(L"UVW");
+ CheckSelection(L"");
+
+ ScopedFPDFWideString text_to_insert2 = GetFPDFWideString(L"XYZ");
+ FORM_ReplaceAndKeepSelection(form_handle(), page(), text_to_insert2.get());
+ CheckFocusedFieldText(L"UVWXYZ");
+ CheckSelection(L"XYZ");
+
+ CheckCanUndo(true);
+ PerformUndo();
+ CheckFocusedFieldText(L"UVW");
+ CheckSelection(L"");
+}
+
TEST_F(FPDFFormFillTextFormEmbedderTest, ReplaceSelection) {
ScopedFPDFWideString text_to_insert = GetFPDFWideString(L"XYZ");
ClickOnFormFieldAtPoint(RegularFormBegin());
@@ -3340,6 +3375,42 @@
CheckCanRedo(false);
}
+TEST_F(FPDFFormFillTextFormEmbedderTest, ContinuouslyReplaceSelection) {
+ ScopedFPDFWideString text_to_insert1 = GetFPDFWideString(L"UVW");
+
+ ClickOnFormFieldAtPoint(RegularFormBegin());
+ CheckFocusedFieldText(L"");
+ CheckCanUndo(false);
+ CheckCanRedo(false);
+
+ FORM_ReplaceSelection(form_handle(), page(), text_to_insert1.get());
+ CheckFocusedFieldText(L"UVW");
+ CheckSelection(L"");
+
+ CheckCanUndo(true);
+ CheckCanRedo(false);
+
+ PerformUndo();
+ CheckFocusedFieldText(L"");
+
+ CheckCanUndo(false);
+ CheckCanRedo(true);
+ PerformRedo();
+ CheckFocusedFieldText(L"UVW");
+ CheckSelection(L"");
+
+ ScopedFPDFWideString text_to_insert2 = GetFPDFWideString(L"XYZ");
+ FORM_ReplaceSelection(form_handle(), page(), text_to_insert2.get());
+ CheckFocusedFieldText(L"UVWXYZ");
+
+ CheckCanUndo(true);
+ CheckCanRedo(false);
+
+ PerformUndo();
+ CheckFocusedFieldText(L"UVW");
+ CheckSelection(L"");
+}
+
TEST_F(FPDFFormFillTextFormEmbedderTest, SelectAllWithKeyboardShortcut) {
// Start with a couple of letters in the text form.
TypeTextIntoTextField(2, RegularFormBegin());
diff --git a/fpdfsdk/pwl/cpwl_edit_impl.cpp b/fpdfsdk/pwl/cpwl_edit_impl.cpp
index ba4f43d..92c35f8 100644
--- a/fpdfsdk/pwl/cpwl_edit_impl.cpp
+++ b/fpdfsdk/pwl/cpwl_edit_impl.cpp
@@ -196,6 +196,11 @@
return m_nCurUndoPos < m_UndoItemStack.size();
}
+CPWL_EditImpl::UndoItemIface* CPWL_EditImpl::UndoStack::GetLastAddItem() {
+ CHECK(!m_UndoItemStack.empty());
+ return m_UndoItemStack.back().get();
+}
+
void CPWL_EditImpl::UndoStack::Redo() {
DCHECK(!m_bWorking);
m_bWorking = true;
@@ -349,25 +354,32 @@
bool bIsEnd)
: m_pEdit(pEdit), m_bEnd(bIsEnd) {
DCHECK(m_pEdit);
+ // Redo ClearSelection, InsertText and ReplaceSelection's end marker
+ // Undo InsertText, ClearSelection and ReplaceSelection's beginning
+ // marker
+ set_undo_remaining(3);
}
CPWL_EditImpl::UndoReplaceSelection::~UndoReplaceSelection() = default;
int CPWL_EditImpl::UndoReplaceSelection::Redo() {
m_pEdit->SelectNone();
- if (IsEnd())
+ if (IsEnd()) {
return 0;
- // Redo ClearSelection, InsertText and ReplaceSelection's end marker
- return 3;
+ }
+ // Redo ClearSelection, InsertText and ReplaceSelection's end
+ // marker. (ClearSelection may not exist)
+ return undo_remaining();
}
int CPWL_EditImpl::UndoReplaceSelection::Undo() {
m_pEdit->SelectNone();
- if (!IsEnd())
+ if (!IsEnd()) {
return 0;
+ }
// Undo InsertText, ClearSelection and ReplaceSelection's beginning
- // marker
- return 3;
+ // marker. (ClearSelection may not exist)
+ return undo_remaining();
}
class CPWL_EditImpl::UndoBackspace final : public CPWL_EditImpl::UndoItemIface {
@@ -1770,8 +1782,12 @@
void CPWL_EditImpl::ReplaceAndKeepSelection(const WideString& text) {
AddEditUndoItem(std::make_unique<UndoReplaceSelection>(this, false));
- ClearSelection();
-
+ bool is_insert_undo_clear = ClearSelection();
+ // It is necessary to determine whether the value of `m_nUndoRemain` is 2 or 3
+ // based on ClearSelection().
+ if (!is_insert_undo_clear) {
+ m_Undo.GetLastAddItem()->set_undo_remaining(2);
+ }
// Select the inserted text.
CPVT_WordPlace caret_before_insert = m_wpCaret;
InsertText(text, FX_Charset::kDefault);
@@ -1779,13 +1795,24 @@
m_SelState.Set(caret_before_insert, caret_after_insert);
AddEditUndoItem(std::make_unique<UndoReplaceSelection>(this, true));
+ if (!is_insert_undo_clear) {
+ m_Undo.GetLastAddItem()->set_undo_remaining(2);
+ }
}
void CPWL_EditImpl::ReplaceSelection(const WideString& text) {
AddEditUndoItem(std::make_unique<UndoReplaceSelection>(this, false));
- ClearSelection();
+ bool is_insert_undo_clear = ClearSelection();
+ // It is necessary to determine whether the value of `m_nUndoRemain` is 2 or 3
+ // based on ClearSelection().
+ if (!is_insert_undo_clear) {
+ m_Undo.GetLastAddItem()->set_undo_remaining(2);
+ }
InsertText(text, FX_Charset::kDefault);
AddEditUndoItem(std::make_unique<UndoReplaceSelection>(this, true));
+ if (!is_insert_undo_clear) {
+ m_Undo.GetLastAddItem()->set_undo_remaining(2);
+ }
}
bool CPWL_EditImpl::Redo() {
diff --git a/fpdfsdk/pwl/cpwl_edit_impl.h b/fpdfsdk/pwl/cpwl_edit_impl.h
index 0b3bc76..245bc6c 100644
--- a/fpdfsdk/pwl/cpwl_edit_impl.h
+++ b/fpdfsdk/pwl/cpwl_edit_impl.h
@@ -186,13 +186,23 @@
// 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 UndoReplaceSelection::Undo(), if UndoReplaceSelection
- // marks the end of a replace action, UndoReplaceSelection::Undo() returns 3
- // because 3 more undo items need to be processed to revert the replace
- // action: insert text, clear selection and the UndoReplaceSelection which
- // marks the beginning of replace action.) Implementations should return 0
- // by default.
+ // marks the end of a replace action, UndoReplaceSelection::Undo() returns
+ // |undo_remaining_|. The default value of |undo_remaining_| in
+ // UndoReplaceSelection is 3. because 3 more undo items need to be processed
+ // to revert the replace action: insert text, clear selection and the
+ // UndoReplaceSelection which marks the beginning of replace action. If
+ // CPWL_EditImpl::ClearSelection() returns false, the value of
+ // |undo_remaining_| in UndoReplaceSelection needs to be set to 2)
+ // Implementations should return 0 by default.
virtual int Undo() = 0;
virtual int Redo() = 0;
+ void set_undo_remaining(int undo_remaining) {
+ undo_remaining_ = undo_remaining;
+ }
+ int undo_remaining() const { return undo_remaining_; }
+
+ private:
+ int undo_remaining_ = 0;
};
class UndoStack {
@@ -205,6 +215,9 @@
void Redo();
bool CanUndo() const;
bool CanRedo() const;
+ // GetLastAddItem() will never return null, so it can only be called after
+ // calling AddItem().
+ UndoItemIface* GetLastAddItem();
private:
void RemoveHeads();
