From 0866e48ae25f53eb9b27b0b86d333700f912459f Mon Sep 17 00:00:00 2001
From: Ivan Komissarov <ABBAPOH@me.com>
Date: Sat, 14 Apr 2012 12:14:22 +0400
Subject: [PATCH] Fixed applying invalid data from QLineEdit to model

Task-number: QTBUG-2216

Change-Id: I5d1c18aadb380667dedc2bb9f9b7e3dd8178a24c
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@digia.com>
Reviewed-by: Stephen Kelly <stephen.kelly@kdab.com>
Reviewed-by: David Faure <david.faure@kdab.com>
---
 src/widgets/itemviews/qitemdelegate.cpp       |  44 ++++--
 src/widgets/itemviews/qstyleditemdelegate.cpp |  44 ++++--
 .../qitemdelegate/tst_qitemdelegate.cpp       | 131 ++++++++++++++++++
 3 files changed, 199 insertions(+), 20 deletions(-)

diff --git a/src/widgets/itemviews/qitemdelegate.cpp b/src/widgets/itemviews/qitemdelegate.cpp
index 7d8512adc44..c040322ba15 100644
--- a/src/widgets/itemviews/qitemdelegate.cpp
+++ b/src/widgets/itemviews/qitemdelegate.cpp
@@ -113,6 +113,7 @@ public:
 
     static QString valueToText(const QVariant &value, const QStyleOptionViewItem &option);
 
+    bool tryFixup(QWidget *editor);
     void _q_commitDataAndCloseEditor(QWidget *editor);
 
     QItemEditorFactory *f;
@@ -386,6 +387,24 @@ QString QItemDelegatePrivate::valueToText(const QVariant &value, const QStyleOpt
     return text;
 }
 
+bool QItemDelegatePrivate::tryFixup(QWidget *editor)
+{
+#ifndef QT_NO_LINEEDIT
+    if (QLineEdit *e = qobject_cast<QLineEdit*>(editor)) {
+        if (!e->hasAcceptableInput()) {
+            if (const QValidator *validator = e->validator()) {
+                QString text = e->text();
+                validator->fixup(text);
+                e->setText(text);
+            }
+            return e->hasAcceptableInput();
+        }
+    }
+#endif // QT_NO_LINEEDIT
+
+    return true;
+}
+
 /*!
     Renders the delegate using the given \a painter and style \a option for
     the item specified by \a index.
@@ -1154,18 +1173,24 @@ QRect QItemDelegate::textRectangle(QPainter * /*painter*/, const QRect &rect,
 
 bool QItemDelegate::eventFilter(QObject *object, QEvent *event)
 {
+    Q_D(QItemDelegate);
+
     QWidget *editor = qobject_cast<QWidget*>(object);
     if (!editor)
         return false;
     if (event->type() == QEvent::KeyPress) {
         switch (static_cast<QKeyEvent *>(event)->key()) {
         case Qt::Key_Tab:
-            emit commitData(editor);
-            emit closeEditor(editor, QAbstractItemDelegate::EditNextItem);
+            if (d->tryFixup(editor)) {
+                emit commitData(editor);
+                emit closeEditor(editor, EditNextItem);
+            }
             return true;
         case Qt::Key_Backtab:
-            emit commitData(editor);
-            emit closeEditor(editor, QAbstractItemDelegate::EditPreviousItem);
+            if (d->tryFixup(editor)) {
+                emit commitData(editor);
+                emit closeEditor(editor, EditPreviousItem);
+            }
             return true;
         case Qt::Key_Enter:
         case Qt::Key_Return:
@@ -1176,11 +1201,9 @@ bool QItemDelegate::eventFilter(QObject *object, QEvent *event)
             // before committing the data (e.g. so it can do
             // validation/fixup of the input).
 #endif // QT_NO_TEXTEDIT
-#ifndef QT_NO_LINEEDIT
-            if (QLineEdit *e = qobject_cast<QLineEdit*>(editor))
-                if (!e->hasAcceptableInput())
-                    return false;
-#endif // QT_NO_LINEEDIT
+            if (!d->tryFixup(editor))
+                return true;
+
             QMetaObject::invokeMethod(this, "_q_commitDataAndCloseEditor",
                                       Qt::QueuedConnection, Q_ARG(QWidget*, editor));
             return false;
@@ -1211,8 +1234,9 @@ bool QItemDelegate::eventFilter(QObject *object, QEvent *event)
                 return false;
             }
 #endif
+            if (d->tryFixup(editor))
+                emit commitData(editor);
 
-            emit commitData(editor);
             emit closeEditor(editor, NoHint);
         }
     } else if (event->type() == QEvent::ShortcutOverride) {
diff --git a/src/widgets/itemviews/qstyleditemdelegate.cpp b/src/widgets/itemviews/qstyleditemdelegate.cpp
index b121800c31c..7e1933ad1e7 100644
--- a/src/widgets/itemviews/qstyleditemdelegate.cpp
+++ b/src/widgets/itemviews/qstyleditemdelegate.cpp
@@ -97,6 +97,7 @@ public:
         return factory ? factory : QItemEditorFactory::defaultFactory();
     }
 
+    bool tryFixup(QWidget *editor);
     void _q_commitDataAndCloseEditor(QWidget *editor)
     {
         Q_Q(QStyledItemDelegate);
@@ -106,6 +107,24 @@ public:
     QItemEditorFactory *factory;
 };
 
+bool QStyledItemDelegatePrivate::tryFixup(QWidget *editor)
+{
+#ifndef QT_NO_LINEEDIT
+    if (QLineEdit *e = qobject_cast<QLineEdit*>(editor)) {
+        if (!e->hasAcceptableInput()) {
+            if (const QValidator *validator = e->validator()) {
+                QString text = e->text();
+                validator->fixup(text);
+                e->setText(text);
+            }
+            return e->hasAcceptableInput();
+        }
+    }
+#endif // QT_NO_LINEEDIT
+
+    return true;
+}
+
 /*!
     \class QStyledItemDelegate
 
@@ -622,18 +641,24 @@ void QStyledItemDelegate::setItemEditorFactory(QItemEditorFactory *factory)
 */
 bool QStyledItemDelegate::eventFilter(QObject *object, QEvent *event)
 {
+    Q_D(QStyledItemDelegate);
+
     QWidget *editor = qobject_cast<QWidget*>(object);
     if (!editor)
         return false;
     if (event->type() == QEvent::KeyPress) {
         switch (static_cast<QKeyEvent *>(event)->key()) {
         case Qt::Key_Tab:
-            emit commitData(editor);
-            emit closeEditor(editor, QAbstractItemDelegate::EditNextItem);
+            if (d->tryFixup(editor)) {
+                emit commitData(editor);
+                emit closeEditor(editor, EditNextItem);
+            }
             return true;
         case Qt::Key_Backtab:
-            emit commitData(editor);
-            emit closeEditor(editor, QAbstractItemDelegate::EditPreviousItem);
+            if (d->tryFixup(editor)) {
+                emit commitData(editor);
+                emit closeEditor(editor, EditPreviousItem);
+            }
             return true;
         case Qt::Key_Enter:
         case Qt::Key_Return:
@@ -644,11 +669,9 @@ bool QStyledItemDelegate::eventFilter(QObject *object, QEvent *event)
             // before committing the data (e.g. so it can do
             // validation/fixup of the input).
 #endif // QT_NO_TEXTEDIT
-#ifndef QT_NO_LINEEDIT
-            if (QLineEdit *e = qobject_cast<QLineEdit*>(editor))
-                if (!e->hasAcceptableInput())
-                    return false;
-#endif // QT_NO_LINEEDIT
+            if (!d->tryFixup(editor))
+                return true;
+
             QMetaObject::invokeMethod(this, "_q_commitDataAndCloseEditor",
                                       Qt::QueuedConnection, Q_ARG(QWidget*, editor));
             return false;
@@ -679,8 +702,9 @@ bool QStyledItemDelegate::eventFilter(QObject *object, QEvent *event)
                 return false;
             }
 #endif
+            if (d->tryFixup(editor))
+                emit commitData(editor);
 
-            emit commitData(editor);
             emit closeEditor(editor, NoHint);
         }
     } else if (event->type() == QEvent::ShortcutOverride) {
diff --git a/tests/auto/widgets/itemviews/qitemdelegate/tst_qitemdelegate.cpp b/tests/auto/widgets/itemviews/qitemdelegate/tst_qitemdelegate.cpp
index d47eebe03a6..538cc7bb4c1 100644
--- a/tests/auto/widgets/itemviews/qitemdelegate/tst_qitemdelegate.cpp
+++ b/tests/auto/widgets/itemviews/qitemdelegate/tst_qitemdelegate.cpp
@@ -232,6 +232,8 @@ private slots:
     void enterKey_data();
     void enterKey();
     void comboBox();
+    void testLineEditValidation_data();
+    void testLineEditValidation();
 
     void task257859_finalizeEdit();
     void QTBUG4435_keepSelectionOnCheck();
@@ -1413,6 +1415,135 @@ void tst_QItemDelegate::comboBox()
     QCOMPARE(data.toBool(), false);
 }
 
+void tst_QItemDelegate::testLineEditValidation_data()
+{
+    QTest::addColumn<int>("key");
+
+    QTest::newRow("enter") << int(Qt::Key_Enter);
+    QTest::newRow("return") << int(Qt::Key_Return);
+    QTest::newRow("tab") << int(Qt::Key_Tab);
+    QTest::newRow("backtab") << int(Qt::Key_Backtab);
+    QTest::newRow("escape") << int(Qt::Key_Escape);
+}
+
+void tst_QItemDelegate::testLineEditValidation()
+{
+    QFETCH(int, key);
+
+    struct TestDelegate : public QItemDelegate
+    {
+        virtual QWidget *createEditor(QWidget *parent, const QStyleOptionViewItem &option, const QModelIndex &index) const
+        {
+            Q_UNUSED(option);
+            Q_UNUSED(index);
+
+            QLineEdit *editor = new QLineEdit(parent);
+            QRegularExpression re("\\w+,\\w+"); // two words separated by a comma
+            editor->setValidator(new QRegularExpressionValidator(re, editor));
+            editor->setObjectName(QStringLiteral("TheEditor"));
+            return editor;
+        }
+    } delegate;
+
+    QStandardItemModel model;
+    // need a couple of dummy items to test tab and back tab
+    model.appendRow(new QStandardItem(QStringLiteral("dummy")));
+    QStandardItem *item = new QStandardItem(QStringLiteral("abc,def"));
+    model.appendRow(item);
+    model.appendRow(new QStandardItem(QStringLiteral("dummy")));
+
+    QListView view;
+    view.setModel(&model);
+    view.setItemDelegate(&delegate);
+    view.show();
+    view.setFocus();
+    QApplication::setActiveWindow(&view);
+    QVERIFY(QTest::qWaitForWindowActive(&view));
+
+    QList<QLineEdit *> lineEditors;
+    QPointer<QLineEdit> editor;
+    QPersistentModelIndex index = model.indexFromItem(item);
+
+    view.setCurrentIndex(index);
+    view.edit(index);
+    QTest::qWait(30);
+
+    lineEditors = view.findChildren<QLineEdit *>(QStringLiteral("TheEditor"));
+    QCOMPARE(lineEditors.count(), 1);
+    editor = lineEditors.at(0);
+    editor->clear();
+
+    // first try to set a valid text
+    QTest::keyClicks(editor, QStringLiteral("foo,bar"));
+    QTest::qWait(30);
+
+    // close the editor
+    QTest::keyClick(editor, Qt::Key(key));
+    QTest::qWait(30);
+
+    QVERIFY(editor.isNull());
+    if (key != Qt::Key_Escape)
+        QCOMPARE(item->data(Qt::DisplayRole).toString(), QStringLiteral("foo,bar"));
+    else
+        QCOMPARE(item->data(Qt::DisplayRole).toString(), QStringLiteral("abc,def"));
+
+    // now an invalid (but partially matching) text
+    view.setCurrentIndex(index);
+    view.edit(index);
+    QTest::qWait(30);
+
+    lineEditors = view.findChildren<QLineEdit *>(QStringLiteral("TheEditor"));
+    QCOMPARE(lineEditors.count(), 1);
+    editor = lineEditors.at(0);
+    editor->clear();
+
+    // edit
+    QTest::keyClicks(editor, QStringLiteral("foobar"));
+    QTest::qWait(30);
+
+    // try to close the editor
+    QTest::keyClick(editor, Qt::Key(key));
+    QTest::qWait(30);
+
+    if (key != Qt::Key_Escape) {
+        QVERIFY(!editor.isNull());
+        QCOMPARE(qApp->focusWidget(), editor.data());
+        QCOMPARE(editor->text(), QStringLiteral("foobar"));
+        QCOMPARE(item->data(Qt::DisplayRole).toString(), QStringLiteral("foo,bar"));
+    } else {
+        QVERIFY(editor.isNull());
+        QCOMPARE(item->data(Qt::DisplayRole).toString(), QStringLiteral("abc,def"));
+    }
+
+    // reset the view to forcibly close the editor
+    view.reset();
+    QTest::qWait(30);
+
+    // set a valid text again
+    view.setCurrentIndex(index);
+    view.edit(index);
+    QTest::qWait(30);
+
+    lineEditors = view.findChildren<QLineEdit *>(QStringLiteral("TheEditor"));
+    QCOMPARE(lineEditors.count(), 1);
+    editor = lineEditors.at(0);
+    editor->clear();
+
+    // set a valid text
+    QTest::keyClicks(editor, QStringLiteral("gender,bender"));
+    QTest::qWait(30);
+
+    // close the editor
+    QTest::keyClick(editor, Qt::Key(key));
+    QTest::qWait(30);
+
+    QVERIFY(editor.isNull());
+    if (key != Qt::Key_Escape)
+        QCOMPARE(item->data(Qt::DisplayRole).toString(), QStringLiteral("gender,bender"));
+    else
+        QCOMPARE(item->data(Qt::DisplayRole).toString(), QStringLiteral("abc,def"));
+}
+
 
 // ### _not_ covered:
 
-- 
GitLab