Fixes: shift+arrow key selection with MTextEdit

RevBy: Pekka Vuorela
Details: selecting just a single character with shift+arrow key didn't work
because MInputContext::update() was called due to smartMoveCursor() ->
moveCursor() -> updateMicroFocus() and at that time edit mode wasn't yet
EditModeSelect and therefore query for selected text returned nothing.
updateMicroFocus() wasn't called again after setting the mode either and just
adding that would have resulted to unnecessary update() calls, so a mechanism
to suppress actual updateMicroFocus() calls was introduced and that was used
to ensure just one updateMicroFocus() call after moving the cursor and
setting the mode.

3 files changed, 104 insertions, 14 deletions

diff --git a/src/corelib/widgets/mtextedit.cpp b/src/corelib/widgets/mtextedit.cpp
index 1d8e688d..6cfc24bc 100755
--- a/src/corelib/widgets/mtextedit.cpp
+++ b/src/corelib/widgets/mtextedit.cpp
@@ -302,7 +302,9 @@ MTextEditPrivate::MTextEditPrivate()
       completer(0),
       registeredToolbarId(-1),
       editActive(false),
-      omitInputMethodEvents(false)
+      omitInputMethodEvents(false),
+      updateMicroFocusDisabled(0),
+      pendingMicroFocusUpdate(false)
 {
 }
 
@@ -374,7 +376,7 @@ bool MTextEditPrivate::moveCursor(QTextCursor::MoveOperation moveOp,
     result = cursor()->movePosition(moveOp, moveMode, n);
     if (result) {
         q->model()->updateCursor();
-        q->updateMicroFocus();
+        updateMicroFocus();
         emit q->cursorPositionChanged();
     }
 
@@ -788,7 +790,7 @@ void MTextEditPrivate::commitPreedit()
     }
 
     setMode(MTextEditModel::EditModeBasic);
-    q->updateMicroFocus();
+    updateMicroFocus();
 
     if (q->hasFocus()) {
         // make sure the committed preedit doesn't get left on the input context
@@ -1212,7 +1214,7 @@ void MTextEditPrivate::_q_confirmCompletion(const QString &completion)
     QObject::disconnect(q, SIGNAL(textChanged()), completer, SLOT(complete()));
     emit q->textChanged();
     QObject::connect(q, SIGNAL(textChanged()), completer, SLOT(complete()));
-    q->updateMicroFocus();
+    updateMicroFocus();
 }
 
 bool MTextEditPrivate::copy()
@@ -1240,6 +1242,34 @@ QString MTextEditPrivate::replaceLineBreaks(QString text, QChar replacement)
     return text;
 }
 
+void MTextEditPrivate::disableUpdateMicroFocus()
+{
+    ++updateMicroFocusDisabled;
+}
+
+void MTextEditPrivate::enableUpdateMicroFocus(bool flush)
+{
+    Q_Q(MTextEdit);
+    Q_ASSERT(updateMicroFocusDisabled);
+    if ((--updateMicroFocusDisabled == 0) && pendingMicroFocusUpdate && flush) {
+        q->updateMicroFocus();
+    }
+    pendingMicroFocusUpdate = false;
+}
+
+void MTextEditPrivate::updateMicroFocus()
+{
+    Q_Q(MTextEdit);
+
+    if (updateMicroFocusDisabled <= 0) {
+        q->updateMicroFocus();
+        pendingMicroFocusUpdate = false;
+    } else {
+        pendingMicroFocusUpdate = true;
+    }
+}
+
+
 ///////////////////////////////////////////////
 // Actual class implementation
 
@@ -1357,7 +1387,7 @@ void MTextEdit::setSelection(int start, int length, bool useBoundaries)
 
     model()->updateCursor();
 
-    updateMicroFocus();
+    d->updateMicroFocus();
 
     emit selectionChanged();
 
@@ -1439,6 +1469,7 @@ void MTextEdit::keyPressEvent(QKeyEvent *event)
             d->smartMoveCursor(moveDirection, QTextCursor::MoveAnchor, 1);
         } else if (event->modifiers() == Qt::ShiftModifier
                    &&  (textInteractionFlags() & Qt::TextSelectableByKeyboard)) {
+            d->disableUpdateMicroFocus();
             const bool moved = d->smartMoveCursor(moveDirection, QTextCursor::KeepAnchor, 1);
             if (!wasSelecting && d->cursor()->hasSelection()) { // first character was selected
                 d->setMode(MTextEditModel::EditModeSelect);
@@ -1449,7 +1480,9 @@ void MTextEdit::keyPressEvent(QKeyEvent *event)
             }
             if (moved) {
                 emit selectionChanged();
+                d->updateMicroFocus();
             }
+            d->enableUpdateMicroFocus(true);
         }
         event->accept();
         return;
@@ -1577,7 +1610,7 @@ void MTextEdit::keyPressEvent(QKeyEvent *event)
         }
 
         model()->updateCursor();
-        updateMicroFocus();
+        d->updateMicroFocus();
         emit textChanged();
         emit cursorPositionChanged();
 
@@ -1700,7 +1733,7 @@ bool MTextEdit::insert(const QString &text)
     // either something was inserted or something was deleted, or both
     if (insertionSuccess || wasSelecting) {
         emit textChanged();
-        updateMicroFocus();
+        d->updateMicroFocus();
     }
 
     return insertionSuccess;
@@ -1751,7 +1784,7 @@ bool MTextEdit::setText(const QString &text)
 
     // only avoid signaling if empty before and after
     if (!((document()->characterCount() == 0) && wasEmpty)) {
-        updateMicroFocus();
+        d->updateMicroFocus();
         emit textChanged();
     }
 
@@ -1899,7 +1932,7 @@ void MTextEdit::handleMouseRelease(int eventCursorPosition, QGraphicsSceneMouseE
     }
 
     if (cursorPosition() != cursorPositionBefore) {
-        updateMicroFocus();
+        d->updateMicroFocus();
     }
 }
 
@@ -1988,7 +2021,7 @@ void MTextEdit::paste()
     if (changed) {
         emit textChanged();
         emit cursorPositionChanged();
-        updateMicroFocus();
+        d->updateMicroFocus();
     } else {
         mDebug("MTextEdit") << __PRETTY_FUNCTION__ << "paste failed";
         emit pasteFailed();
@@ -2021,7 +2054,7 @@ void MTextEdit::cut()
         emit selectionChanged();
         emit textChanged();
         emit cursorPositionChanged();
-        updateMicroFocus();
+        d->updateMicroFocus();
     }
 }
 
@@ -2121,7 +2154,7 @@ void MTextEdit::inputMethodEvent(QInputMethodEvent *event)
     }
 
     if (changed) {
-        updateMicroFocus();
+        d->updateMicroFocus();
         emit textChanged();
         emit cursorPositionChanged();
     }
@@ -2406,7 +2439,7 @@ void MTextEdit::deselect()
         d->setMode(MTextEditModel::EditModeBasic);
         model()->updateCursor();
         d->sendCopyAvailable(false);
-        updateMicroFocus();
+        d->updateMicroFocus();
         emit selectionChanged();
     }
 }
@@ -2581,7 +2614,7 @@ void MTextEdit::setCompleter(MCompleter *completer)
         setInputMethodAutoCapitalizationEnabled(false);
 
         if (hasFocus()) {
-            updateMicroFocus();
+            d->updateMicroFocus();
             connect(d->completer, SIGNAL(confirmed(QString, QModelIndex)),
                     this, SLOT(_q_confirmCompletion(QString)));
         }
diff --git a/src/corelib/widgets/mtextedit_p.h b/src/corelib/widgets/mtextedit_p.h
index 00ad99b8..9fb65f90 100755
--- a/src/corelib/widgets/mtextedit_p.h
+++ b/src/corelib/widgets/mtextedit_p.h
@@ -105,6 +105,19 @@ public:
     QString replaceLineBreaks(QString text, QChar replacement);
     void _q_confirmCompletion(const QString &);
 
+    //! \brief Disable MTextEdit::updateMicroFocus().
+    //!
+    //! Can be called multiple times without calling \a enableUpdateMicroFocus in between.  In such a case
+    //! \a enableUpdateMicroFocus must be called equally many times.
+    void disableUpdateMicroFocus();
+    //! \param flush call updateMicroFocus() automatically if true and updateMicroFocus was called
+    //! while being disabled
+    void enableUpdateMicroFocus(bool flush = false);
+    //! \brief updateMicroFocus() wrapper that can be disabled
+    //! \sa disableUpdateMicroFocus
+    //! \sa enableUpdateMicroFocus
+    void updateMicroFocus();
+
     bool pendingSoftwareInputPanelRequest;
 
 private:
@@ -118,6 +131,9 @@ private:
 
     // protection from input methods that send something on QInputContext::reset()
     bool omitInputMethodEvents;
+
+    int updateMicroFocusDisabled;
+    bool pendingMicroFocusUpdate;
 };
 
 #endif
diff --git a/tests/ut_mtextedit/ut_mtextedit.cpp b/tests/ut_mtextedit/ut_mtextedit.cpp
index 2983526b..8521500f 100644
--- a/tests/ut_mtextedit/ut_mtextedit.cpp
+++ b/tests/ut_mtextedit/ut_mtextedit.cpp
@@ -97,6 +97,9 @@ class SimpleInputContext: public QInputContext
 public:
     SimpleInputContext(QObject *parent = 0)
         : QInputContext(parent),
+          updateCallCount(0),
+          selectionAvailableAtLastUpdate(false),
+          edit(0),
           m_visible(false)
     {}
 
@@ -143,6 +146,20 @@ public:
         return false;
     }
 
+    void update()
+    {
+        ++updateCallCount;
+        if (edit) {
+            selectedTextAtLastUpdate = edit->selectedText();
+        }
+        selectionAvailableAtLastUpdate = !selectedTextAtLastUpdate.isEmpty();
+    }
+
+    int updateCallCount;
+    bool selectionAvailableAtLastUpdate;
+    QString selectedTextAtLastUpdate;
+    MTextEdit *edit;
+
 private:
     bool m_visible;
 };
@@ -258,6 +275,7 @@ void Ut_MTextEdit::init()
 {
     m_subject.reset(new MTextEdit(MTextEditModel::MultiLine, ""));
     qApp->setInputContext(m_sic = new SimpleInputContext);
+    m_sic->edit = m_subject.get();
 }
 
 
@@ -1946,6 +1964,7 @@ void Ut_MTextEdit::testArrowKeys()
 
 void Ut_MTextEdit::testSelectByArrowKeys()
 {
+    setupSipEnv(m_subject.get());
     QSignalSpy copyAvailableSpy(m_subject.get(), SIGNAL(copyAvailable(bool)));
     QVERIFY(copyAvailableSpy.isValid());
 
@@ -1982,9 +2001,12 @@ void Ut_MTextEdit::testSelectByArrowKeys()
     QCOMPARE(copyAvailableSpy.count(), 0);
     QCOMPARE(selectionChangedSpy.count(), 0);
 
+    QCOMPARE(m_sic->updateCallCount, 0);
+
     m_subject->setText(line);
     m_subject->setCursorPosition(1);
 
+    m_sic->updateCallCount = 0;
     m_subject->keyPressEvent(&right);
     QCOMPARE(m_subject->cursorPosition(), 2);
     QCOMPARE(m_subject->selectedText(), QString("2"));
@@ -1994,6 +2016,9 @@ void Ut_MTextEdit::testSelectByArrowKeys()
     QCOMPARE(selectionChangedSpy.count(), 1);
     copyAvailableSpy.clear();
     selectionChangedSpy.clear();
+    QCOMPARE(m_sic->updateCallCount, 1);
+    m_sic->updateCallCount = 0;
+    QVERIFY(m_sic->selectionAvailableAtLastUpdate);
 
     m_subject->keyPressEvent(&left);
     QCOMPARE(m_subject->cursorPosition(), 1);
@@ -2004,6 +2029,9 @@ void Ut_MTextEdit::testSelectByArrowKeys()
     QCOMPARE(selectionChangedSpy.count(), 1);
     copyAvailableSpy.clear();
     selectionChangedSpy.clear();
+    QCOMPARE(m_sic->updateCallCount, 1);
+    m_sic->updateCallCount = 0;
+    QVERIFY(!m_sic->selectionAvailableAtLastUpdate);
 
     m_subject->keyPressEvent(&left);
     QCOMPARE(m_subject->cursorPosition(), 0);
@@ -2014,11 +2042,14 @@ void Ut_MTextEdit::testSelectByArrowKeys()
     QCOMPARE(selectionChangedSpy.count(), 1);
     copyAvailableSpy.clear();
     selectionChangedSpy.clear();
+    QCOMPARE(m_sic->updateCallCount, 1);
+    QVERIFY(m_sic->selectionAvailableAtLastUpdate);
 
     m_subject->setCursorPosition(0);
     copyAvailableSpy.clear();
     selectionChangedSpy.clear();
 
+    m_sic->updateCallCount = 0;
     m_subject->keyPressEvent(&down);
     QVERIFY(m_subject->cursorPosition() > 0);
     QVERIFY(!m_subject->selectedText().isEmpty());
@@ -2028,6 +2059,9 @@ void Ut_MTextEdit::testSelectByArrowKeys()
     QCOMPARE(selectionChangedSpy.count(), 1);
     copyAvailableSpy.clear();
     selectionChangedSpy.clear();
+    QCOMPARE(m_sic->updateCallCount, 1);
+    m_sic->updateCallCount = 0;
+    QVERIFY(m_sic->selectionAvailableAtLastUpdate);
 
     m_subject->keyPressEvent(&up);
     QVERIFY(m_subject->cursorPosition() == 0);
@@ -2038,15 +2072,21 @@ void Ut_MTextEdit::testSelectByArrowKeys()
     QCOMPARE(selectionChangedSpy.count(), 1);
     copyAvailableSpy.clear();
     selectionChangedSpy.clear();
+    QCOMPARE(m_sic->updateCallCount, 1);
+    QVERIFY(!m_sic->selectionAvailableAtLastUpdate);
 
     //disable flag Qt::TextSelectableByKeyboard
     m_subject->setTextInteractionFlags(Qt::TextSelectableByMouse | Qt::TextEditable);
 
+    m_sic->updateCallCount = 0;
     m_subject->keyPressEvent(&right);
     QCOMPARE(m_subject->cursorPosition(), 0);
     QVERIFY(!m_subject->hasSelectedText());
     QCOMPARE(copyAvailableSpy.count(), 0);
     QCOMPARE(selectionChangedSpy.count(), 0);
+    QCOMPARE(m_sic->updateCallCount, 0);
+    m_sic->updateCallCount = 0;
+    QVERIFY(!m_sic->selectionAvailableAtLastUpdate);
 }
 
 void Ut_MTextEdit::testAutoSipEnabled()
@@ -2419,6 +2459,7 @@ void Ut_MTextEdit::testArrowKeyNavigation()
 void Ut_MTextEdit::setupSipEnv(MTextEdit *edit)
 {
     edit->setFlag(QGraphicsItem::ItemAcceptsInputMethod);
+    m_sic->edit = edit;
 }
 
 void Ut_MTextEdit::requestSip(MTextEdit *edit, Qt::FocusReason fr)