Cursor positioning in QTextDocument after undo()

QTextDocument had no way of storing the cursor position from which
a document change was initiated in the undo stack. That means that
any undo operation would reposition the text cursor to the text
position where the actual change happened. This works in many cases,
but not always.

In Qt Creator we have standard IDE shortcuts like e.g.
Ctrl+Return for InsertLineBelowCurrentLine, which insert a newline
at the end of the current line, not at the cursor position. Using
undo there resulted in a surprisingly wrong cursor position.

The problem becomes worse with more advanced refactoring and
productivity tools (like snippets).

The patch creates a synthetic CursorMoved undo item with the position
which was current at the time of calling QTextCursor::beginEditBlock(),
but only if necesary, i.e. only in those cases where the cursor would
be positioned wrongly.

Reviewed-by: Roberto Raggi

4 files changed, 41 insertions, 1 deletions

diff --git a/src/gui/text/qtextcursor.cpp b/src/gui/text/qtextcursor.cpp
index d6ac3aa..3db66ce 100644
--- a/src/gui/text/qtextcursor.cpp
+++ b/src/gui/text/qtextcursor.cpp
@@ -2437,6 +2437,9 @@ void QTextCursor::beginEditBlock()
     if (!d || !d->priv)
         return;
 
+    if (d->priv->editBlock == 0) // we are the initial edit block, store current cursor position for undo
+        d->priv->editBlockCursorPosition = d->position;
+
     d->priv->beginEditBlock();
 }
 
diff --git a/src/gui/text/qtextdocument_p.cpp b/src/gui/text/qtextdocument_p.cpp
index e2bca04..f3cd481 100644
--- a/src/gui/text/qtextdocument_p.cpp
+++ b/src/gui/text/qtextdocument_p.cpp
@@ -192,6 +192,7 @@ QTextDocumentPrivate::QTextDocumentPrivate()
     initialBlockCharFormatIndex(-1) // set correctly later in init()
 {
     editBlock = 0;
+    editBlockCursorPosition = -1;
     docChangeFrom = -1;
 
     undoState = 0;
@@ -967,6 +968,10 @@ int QTextDocumentPrivate::undoRedo(bool undo)
             editPos = -1;
 	    break;
 	}
+        case QTextUndoCommand::CursorMoved:
+            editPos = c.pos;
+            editLength = 0;
+            break;
 	case QTextUndoCommand::Custom:
             resetBlockRevision = -1; // ## TODO
             if (undo)
@@ -1046,6 +1051,18 @@ void QTextDocumentPrivate::appendUndoItem(const QTextUndoCommand &c)
     if (undoState < undoStack.size())
         clearUndoRedoStacks(QTextDocument::RedoStack);
 
+    if (editBlock != 0 && editBlockCursorPosition >= 0) { // we had a beginEditBlock() with a cursor position
+        if (c.pos != (quint32) editBlockCursorPosition) { // and that cursor position is different from the command
+            // generate a CursorMoved undo item
+            QT_INIT_TEXTUNDOCOMMAND(cc, QTextUndoCommand::CursorMoved, true, QTextUndoCommand::MoveCursor,
+                                    0, 0, editBlockCursorPosition, 0, 0);
+            undoStack.append(cc);
+            undoState++;
+            editBlockCursorPosition = -1;
+        }
+    }
+
+
     if (!undoStack.isEmpty() && modified) {
         QTextUndoCommand &last = undoStack[undoState - 1];
 
@@ -1167,6 +1184,8 @@ void QTextDocumentPrivate::endEditBlock()
         }
     }
 
+    editBlockCursorPosition = -1;
+
     finishEdit();
 }
 
diff --git a/src/gui/text/qtextdocument_p.h b/src/gui/text/qtextdocument_p.h
index ac5ed3c..d1bd698 100644
--- a/src/gui/text/qtextdocument_p.h
+++ b/src/gui/text/qtextdocument_p.h
@@ -132,6 +132,7 @@ public:
         BlockAdded = 6,
         BlockDeleted = 7,
         GroupFormatChange = 8,
+        CursorMoved = 9,
         Custom = 256
     };
     enum Operation {
@@ -315,6 +316,7 @@ private:
     bool modified;
 
     int editBlock;
+    int editBlockCursorPosition;
     int docChangeFrom;
     int docChangeOldLength;
     int docChangeLength;
diff --git a/tests/auto/qtextcursor/tst_qtextcursor.cpp b/tests/auto/qtextcursor/tst_qtextcursor.cpp
index 99babac..41835bb 100644
--- a/tests/auto/qtextcursor/tst_qtextcursor.cpp
+++ b/tests/auto/qtextcursor/tst_qtextcursor.cpp
@@ -151,6 +151,7 @@ private slots:
 
     void cursorPositionWithBlockUndoAndRedo();
     void cursorPositionWithBlockUndoAndRedo2();
+    void cursorPositionWithBlockUndoAndRedo3();
 
 private:
     int blockCount();
@@ -1756,9 +1757,9 @@ void tst_QTextCursor::adjustCursorsOnInsert()
 void tst_QTextCursor::cursorPositionWithBlockUndoAndRedo()
 {
     cursor.insertText("AAAABBBBCCCCDDDD");
-    cursor.beginEditBlock();
     cursor.setPosition(12);
     int cursorPositionBefore = cursor.position();
+    cursor.beginEditBlock();
     cursor.insertText("*");
     cursor.setPosition(8);
     cursor.insertText("*");
@@ -1814,5 +1815,20 @@ void tst_QTextCursor::cursorPositionWithBlockUndoAndRedo2()
     QCOMPARE(cursor.position(), cursorPositionBefore);
 }
 
+void tst_QTextCursor::cursorPositionWithBlockUndoAndRedo3()
+{
+    // verify that it's the position of the beginEditBlock that counts, and not the last edit position
+    cursor.insertText("AAAABBBB");
+    int cursorPositionBefore = cursor.position();
+    cursor.beginEditBlock();
+    cursor.setPosition(4);
+    QVERIFY(cursor.position() != cursorPositionBefore);
+    cursor.insertText("*");
+    cursor.endEditBlock();
+    QCOMPARE(cursor.position(), 5);
+    doc->undo(&cursor);
+    QCOMPARE(cursor.position(), cursorPositionBefore);
+}
+
 QTEST_MAIN(tst_QTextCursor)
 #include "tst_qtextcursor.moc"