Fixed cursor position and undo/redo merges

Fixes two things:

1. Cursor positions are being incorrectly calculated in scenarios where keystroke actions reject some input text.
2. Undo/redo operations were buggy and the commands would not merge often leading to single character undo/redo most of the times.
This commit is contained in:
Pratham Gandhi 2024-06-15 18:26:51 +00:00 committed by Albert Astals Cid
parent 26005d457e
commit 2fe492891f
7 changed files with 82 additions and 30 deletions

View file

@ -69,15 +69,15 @@ void CalculateTextTest::testSimpleCalculate()
// Set some values and do calculation
Okular::FormFieldText *field1 = fields[QStringLiteral("field1")];
QVERIFY(field1);
m_document->editFormText(0, field1, QStringLiteral("10"), 0, 0, 0);
m_document->editFormText(0, field1, QStringLiteral("10"), 0, 0, 0, QString());
Okular::FormFieldText *field2 = fields[QStringLiteral("field2")];
QVERIFY(field2);
m_document->editFormText(0, field2, QStringLiteral("20"), 0, 0, 0);
m_document->editFormText(0, field2, QStringLiteral("20"), 0, 0, 0, QString());
Okular::FormFieldText *field3 = fields[QStringLiteral("field3")];
QVERIFY(field3);
m_document->editFormText(0, field3, QStringLiteral("30"), 0, 0, 0);
m_document->editFormText(0, field3, QStringLiteral("30"), 0, 0, 0, QString());
// Verify the results
QCOMPARE(fields[QStringLiteral("Sum")]->text(), QStringLiteral("60"));
@ -90,7 +90,7 @@ void CalculateTextTest::testSimpleCalculate()
QCOMPARE(fields[QStringLiteral("Sum")]->text(), QStringLiteral("60"));
// Test that multiplication with zero works
m_document->editFormText(0, field2, QStringLiteral("0"), 0, 0, 0);
m_document->editFormText(0, field2, QStringLiteral("0"), 0, 0, 0, QStringLiteral("20"));
QCOMPARE(fields[QStringLiteral("Prod")]->text(), QStringLiteral("0"));
// Test that updating the field also worked with sum

View file

@ -376,7 +376,7 @@ void EditFormsTest::verifyTextForm(Okular::FormFieldText *form)
QCOMPARE(form->text(), QLatin1String(""));
// Insert the string "Hello" into the form
m_document->editFormText(0, form, QStringLiteral("Hello"), 5, 0, 0);
m_document->editFormText(0, form, QStringLiteral("Hello"), 5, 0, 0, QString());
QCOMPARE(form->text(), QStringLiteral("Hello"));
QVERIFY(m_document->canUndo());
QVERIFY(!m_document->canRedo());
@ -394,12 +394,12 @@ void EditFormsTest::verifyTextForm(Okular::FormFieldText *form)
QVERIFY(!m_document->canRedo());
// Type "_World" after "Hello"
m_document->editFormText(0, form, QStringLiteral("Hello_"), 6, 5, 5);
m_document->editFormText(0, form, QStringLiteral("Hello_W"), 7, 6, 6);
m_document->editFormText(0, form, QStringLiteral("Hello_Wo"), 8, 7, 7);
m_document->editFormText(0, form, QStringLiteral("Hello_Wor"), 9, 8, 8);
m_document->editFormText(0, form, QStringLiteral("Hello_Worl"), 10, 9, 9);
m_document->editFormText(0, form, QStringLiteral("Hello_World"), 11, 10, 10);
m_document->editFormText(0, form, QStringLiteral("Hello_"), 6, 5, 5, QStringLiteral("Hello"));
m_document->editFormText(0, form, QStringLiteral("Hello_W"), 7, 6, 6, QStringLiteral("Hello_"));
m_document->editFormText(0, form, QStringLiteral("Hello_Wo"), 8, 7, 7, QStringLiteral("Hello_W"));
m_document->editFormText(0, form, QStringLiteral("Hello_Wor"), 9, 8, 8, QStringLiteral("Hello_Wo"));
m_document->editFormText(0, form, QStringLiteral("Hello_Worl"), 10, 9, 9, QStringLiteral("Hello_Wor"));
m_document->editFormText(0, form, QStringLiteral("Hello_World"), 11, 10, 10, QStringLiteral("Hello_Worl"));
// Verify that character insertion operations were merged together into a single undo command
m_document->undo();

View file

@ -1366,7 +1366,7 @@ void PartTest::testSaveAsUndoStackForms()
if (ff->id() == 65537) {
QCOMPARE(ff->type(), FormField::FormText);
FormFieldText *fft = static_cast<FormFieldText *>(ff);
part.m_document->editFormText(0, fft, QStringLiteral("BlaBla"), 6, 0, 0);
part.m_document->editFormText(0, fft, QStringLiteral("BlaBla"), 6, 0, 0, QString());
} else if (ff->id() == 65538) {
QCOMPARE(ff->type(), FormField::FormButton);
FormFieldButton *ffb = static_cast<FormFieldButton *>(ff);

View file

@ -3967,6 +3967,12 @@ void Document::editFormText(int pageNumber, Okular::FormFieldText *form, const Q
d->m_undoStack->push(uc);
}
void Document::editFormText(int pageNumber, Okular::FormFieldText *form, const QString &newContents, int newCursorPos, int prevCursorPos, int prevAnchorPos, const QString &oldContents)
{
QUndoCommand *uc = new EditFormTextCommand(this->d, form, pageNumber, newContents, newCursorPos, oldContents, prevCursorPos, prevAnchorPos);
d->m_undoStack->push(uc);
}
void Document::editFormList(int pageNumber, FormFieldChoice *form, const QList<int> &newChoices)
{
const QList<int> prevChoices = form->currentChoices();
@ -4373,7 +4379,13 @@ QString DocumentPrivate::evaluateKeystrokeEventChange(const QString &oldVal, con
return {};
}
const size_t changeLength = (selEnd - selStart) + (newUcs4.size() - oldUcs4.size());
return QString::fromUcs4(std::u32string_view {newUcs4}.substr(selStart, changeLength).data(), changeLength);
auto subview = std::u32string_view {newUcs4}.substr(selStart, changeLength);
if (subview.empty()) {
// If subview is empty (in scenarios when selStart is at end and changeLength is non-zero) fromUcs4 returns \u0000.
// This should not happen, but just a guard.
return {};
}
return QString::fromUcs4(subview.data(), changeLength);
}
void Document::processKeystrokeAction(const Action *action, Okular::FormFieldText *fft, const QVariant &newValue, int prevCursorPos, int prevAnchorPos)

View file

@ -678,9 +678,9 @@ public:
* Processes the given keystroke @p action on @p fft.
*
* @since 1.9
* @deprecated
* @deprecated use processKeystrokeAction(const Action *, Okular::FormFieldText *, const QVariant &, int, int)
*/
void processKeystrokeAction(const Action *action, Okular::FormFieldText *fft, const QVariant &newValue);
OKULARCORE_DEPRECATED void processKeystrokeAction(const Action *action, Okular::FormFieldText *fft, const QVariant &newValue);
/**
* Processes the given keystroke @p action on @p fft between the two positions @p prevCursorPos and @p prevAnchorPos
@ -1083,8 +1083,19 @@ public Q_SLOTS:
* The new text cursor position (@p newCursorPos), previous text cursor position (@p prevCursorPos),
* and previous cursor anchor position will be restored by the undo / redo commands.
* @since 0.17 (KDE 4.11)
*
* @deprecated use editFormText(int pageNumber, Okular::FormFieldText *form, const QString &newContents,
* int newCursorPos, int prevCursorPos, int prevAnchorPos, const QString &oldContents)
*/
void editFormText(int pageNumber, Okular::FormFieldText *form, const QString &newContents, int newCursorPos, int prevCursorPos, int prevAnchorPos);
OKULARCORE_DEPRECATED void editFormText(int pageNumber, Okular::FormFieldText *form, const QString &newContents, int newCursorPos, int prevCursorPos, int prevAnchorPos);
/**
* Edit the text contents of the specified @p form on page @p page to be @p newContents where
* previous contents are @p oldContents for undo/redo commands. The new text cursor position (@p newCursorPos),
* previous text cursor position (@p prevCursorPos), and previous cursor anchor position will be restored by the undo / redo commands.
* @since 24.08
*/
void editFormText(int pageNumber, Okular::FormFieldText *form, const QString &newContents, int newCursorPos, int prevCursorPos, int prevAnchorPos, const QString &oldContents);
/**
* Edit the selected list entries in @p form on page @p page to be @p newChoices.

View file

@ -42,7 +42,9 @@ FormWidgetsController::FormWidgetsController(Okular::Document *doc)
connect(this, &FormWidgetsController::formComboChangedByUndoRedo, this, &FormWidgetsController::changed);
// connect form modification signals to and from document
connect(this, &FormWidgetsController::formTextChangedByWidget, doc, &Okular::Document::editFormText);
// connect to the newer editFormText method and not the deprecated one
using EditFormTextType = void (Okular::Document::*)(int, Okular::FormFieldText *, const QString &, int, int, int, const QString &);
connect(this, &FormWidgetsController::formTextChangedByWidget, doc, static_cast<EditFormTextType>(&Okular::Document::editFormText));
connect(doc, &Okular::Document::formTextChangedByUndoRedo, this, &FormWidgetsController::formTextChangedByUndoRedo);
connect(this, &FormWidgetsController::formListChangedByWidget, doc, &Okular::Document::editFormList);
@ -599,14 +601,23 @@ void FormLineEdit::contextMenuEvent(QContextMenuEvent *event)
void FormLineEdit::slotChanged()
{
Okular::FormFieldText *form = static_cast<Okular::FormFieldText *>(m_ff);
int cursorPos = cursorPosition();
QString newWidgetValueBeforeKeystroke = text();
QString oldContents = form->text();
int cursorPos;
if (text() != form->text()) {
if (newWidgetValueBeforeKeystroke != oldContents) {
if (form->additionalAction(Okular::FormField::FieldModified) && m_editing && !form->isReadOnly()) {
m_controller->document()->processKeystrokeAction(form->additionalAction(Okular::FormField::FieldModified), form, text(), m_prevCursorPos, m_prevAnchorPos);
}
Q_EMIT m_controller->formTextChangedByWidget(pageItem()->pageNumber(), form, text(), cursorPos, m_prevCursorPos, m_prevAnchorPos);
// calculate cursor position after keystroke action since it is possible that the change was not accepted and widget was refreshed.
cursorPos = cursorPosition();
if (newWidgetValueBeforeKeystroke == text()) {
// if widget was not refreshed then emit this signal.
Q_EMIT m_controller->formTextChangedByWidget(pageItem()->pageNumber(), form, text(), cursorPos, m_prevCursorPos, m_prevAnchorPos, oldContents);
}
} else {
// we still evaluate the cursor position if no change occurs in widget contents.
cursorPos = cursorPosition();
}
m_prevCursorPos = cursorPos;
@ -633,7 +644,11 @@ void FormLineEdit::slotHandleTextChangedByUndoRedo(int pageNumber, Okular::FormF
connect(this, &QLineEdit::cursorPositionChanged, this, &FormLineEdit::slotChanged);
m_prevCursorPos = cursorPos;
m_prevAnchorPos = anchorPos;
setFocus();
// If the contents of the box have already lost focus, we need to run all the keystroke, validation, formatting scripts again.
if (!hasFocus()) { // if lineEdit already had focus, undoing/redoing was still retain the focus and these scripts would execute when focus is lost.
// TODO handle the keystroke, validate, calculate, format events here.
}
}
void FormLineEdit::slotRefresh(Okular::FormField *form)
@ -751,20 +766,33 @@ void TextAreaEdit::slotHandleTextChangedByUndoRedo(int pageNumber, Okular::FormF
m_prevCursorPos = cursorPos;
m_prevAnchorPos = anchorPos;
setTextCursor(c);
setFocus();
// If the contents of the box have already lost focus, we need to run all the keystroke, validation, formatting scripts again.
if (!hasFocus()) { // if lineEdit already had focus, undoing/redoing was still retain the focus and these scripts would execute when focus is lost.
// TODO handle the keystroke, validate, calculate, format events here.
}
}
void TextAreaEdit::slotChanged()
{
Okular::FormFieldText *form = static_cast<Okular::FormFieldText *>(m_ff);
int cursorPos = textCursor().position();
QString newWidgetValueBeforeKeystroke = toPlainText();
QString oldContents = form->text();
int cursorPos;
if (toPlainText() != form->text()) {
if (newWidgetValueBeforeKeystroke != oldContents) {
if (form->additionalAction(Okular::FormField::FieldModified) && m_editing && !form->isReadOnly()) {
m_controller->document()->processKeystrokeAction(form->additionalAction(Okular::FormField::FieldModified), form, toPlainText(), m_prevCursorPos, m_prevAnchorPos);
}
Q_EMIT m_controller->formTextChangedByWidget(pageItem()->pageNumber(), form, toPlainText(), cursorPos, m_prevCursorPos, m_prevAnchorPos);
// calculate cursor position after keystroke action since it is possible that the change was not accepted and widget was refreshed.
cursorPos = textCursor().position();
if (newWidgetValueBeforeKeystroke == toPlainText()) {
// if widget was not refreshed then emit this signal.
Q_EMIT m_controller->formTextChangedByWidget(pageItem()->pageNumber(), form, toPlainText(), cursorPos, m_prevCursorPos, m_prevAnchorPos, oldContents);
}
} else {
// we still evaluate the cursor position if no change occurs in widget contents.
cursorPos = textCursor().position();
}
m_prevCursorPos = cursorPos;
m_prevAnchorPos = textCursor().anchor();
@ -859,9 +887,10 @@ void FileEdit::slotChanged()
Okular::FormFieldText *form = static_cast<Okular::FormFieldText *>(m_ff);
QString contents = text();
QString oldContents = form->text();
int cursorPos = lineEdit()->cursorPosition();
if (contents != form->text()) {
Q_EMIT m_controller->formTextChangedByWidget(pageItem()->pageNumber(), form, contents, cursorPos, m_prevCursorPos, m_prevAnchorPos);
if (contents != oldContents) {
Q_EMIT m_controller->formTextChangedByWidget(pageItem()->pageNumber(), form, contents, cursorPos, m_prevCursorPos, m_prevAnchorPos, oldContents);
}
m_prevCursorPos = cursorPos;

View file

@ -79,7 +79,7 @@ Q_SIGNALS:
void requestRedo();
void canUndoChanged(bool undoAvailable);
void canRedoChanged(bool redoAvailable);
void formTextChangedByWidget(int pageNumber, Okular::FormFieldText *form, const QString &newContents, int newCursorPos, int prevCursorPos, int prevAnchorPos);
void formTextChangedByWidget(int pageNumber, Okular::FormFieldText *form, const QString &newContents, int newCursorPos, int prevCursorPos, int prevAnchorPos, const QString &oldContents);
void formTextChangedByUndoRedo(int pageNumber, Okular::FormFieldText *form, const QString &contents, int cursorPos, int anchorPos);