Fixed keyboard navigation for QTableView

Keyboard navigation didn't work in the following cases:
  - The last column was disabled and we pressed the tab key when at the 2nd
  last column. (See ref. task).
  - Spans with their anchor column or row hidden or disabled.
  - Navigation would not preserve the original row/column when traversing a
  span horizontally/vertically.

Auto-tests submitted with this commit.

Task-number: QTBUG-3878
Reviewed-by: Olivier

4 files changed, 304 insertions, 73 deletions

diff --git a/src/gui/itemviews/qabstractitemview.cpp b/src/gui/itemviews/qabstractitemview.cpp
index 37f4184..268e78e 100644
--- a/src/gui/itemviews/qabstractitemview.cpp
+++ b/src/gui/itemviews/qabstractitemview.cpp
@@ -2185,6 +2185,9 @@ void QAbstractItemView::keyPressEvent(QKeyEvent *event)
             } else {
                 d->selectionModel->setCurrentIndex(newCurrent, command);
                 d->pressedPosition = visualRect(newCurrent).center() + d->offset();
+                // We copy the same behaviour as for mousePressEvent().
+                QRect rect(d->pressedPosition - d->offset(), QSize(1, 1));
+                setSelection(rect, command);
             }
             return;
         }
diff --git a/src/gui/itemviews/qtableview.cpp b/src/gui/itemviews/qtableview.cpp
index 15bd445..2d98258 100644
--- a/src/gui/itemviews/qtableview.cpp
+++ b/src/gui/itemviews/qtableview.cpp
@@ -779,8 +779,6 @@ void QTableViewPrivate::drawAndClipSpans(const QRegion &area, QPainter *painter,
     foreach (QSpanCollection::Span *span, visibleSpans) {
         int row = span->top();
         int col = span->left();
-        if (isHidden(row, col))
-            continue;
         QModelIndex index = model->index(row, col, root);
         if (!index.isValid())
             continue;
@@ -1480,12 +1478,30 @@ QModelIndex QTableView::moveCursor(CursorAction cursorAction, Qt::KeyboardModifi
             ++column;
         while (isRowHidden(d->logicalRow(row)) && row < bottom)
             ++row;
+        d->visualCursor = QPoint(column, row);
         return d->model->index(d->logicalRow(row), d->logicalColumn(column), d->root);
     }
 
-    int visualRow = d->visualRow(current.row());
+    // Update visual cursor if current index has changed.
+    QPoint visualCurrent(d->visualColumn(current.column()), d->visualRow(current.row()));
+    if (visualCurrent != d->visualCursor) {
+        if (d->hasSpans()) {
+            QSpanCollection::Span span = d->span(current.row(), current.column());
+            if (span.top() > d->visualCursor.y() || d->visualCursor.y() > span.bottom()
+                || span.left() > d->visualCursor.x() || d->visualCursor.x() > span.right())
+                d->visualCursor = visualCurrent;
+        } else {
+            d->visualCursor = visualCurrent;
+        }
+    }
+
+    int visualRow = d->visualCursor.y();
+    if (visualRow > bottom)
+        visualRow = bottom;
     Q_ASSERT(visualRow != -1);
-    int visualColumn = d->visualColumn(current.column());
+    int visualColumn = d->visualCursor.x();
+    if (visualColumn > right)
+        visualColumn = right;
     Q_ASSERT(visualColumn != -1);
 
     if (isRightToLeft()) {
@@ -1496,22 +1512,33 @@ QModelIndex QTableView::moveCursor(CursorAction cursorAction, Qt::KeyboardModifi
     }
 
     switch (cursorAction) {
-    case MoveUp:
+    case MoveUp: {
+        int originalRow = visualRow;
 #ifdef QT_KEYPAD_NAVIGATION
         if (QApplication::keypadNavigationEnabled() && visualRow == 0)
             visualRow = d->visualRow(model()->rowCount() - 1) + 1;
+            // FIXME? visualRow = bottom + 1;
 #endif
-        --visualRow;
-        while (visualRow > 0 && d->isVisualRowHiddenOrDisabled(visualRow, visualColumn))
+        int r = d->logicalRow(visualRow);
+        int c = d->logicalColumn(visualColumn);
+        if (r != -1 && d->hasSpans()) {
+            QSpanCollection::Span span = d->span(r, c);
+            if (span.width() > 1 || span.height() > 1)
+                visualRow = d->visualRow(span.top());
+        }
+        while (visualRow >= 0) {
             --visualRow;
-        if (d->hasSpans()) {
-            int row = d->logicalRow(visualRow);
-            QSpanCollection::Span span = d->span(row, current.column());
-            visualRow = d->visualRow(span.top());
-            visualColumn = d->visualColumn(span.left());
+            r = d->logicalRow(visualRow);
+            c = d->logicalColumn(visualColumn);
+            if (r == -1 || (!isRowHidden(r) && d->isCellEnabled(r, c)))
+                break;
         }
+        if (visualRow < 0)
+            visualRow = originalRow;
         break;
-    case MoveDown:
+    }
+    case MoveDown: {
+        int originalRow = visualRow;
         if (d->hasSpans()) {
             QSpanCollection::Span span = d->span(current.row(), current.column());
             visualRow = d->visualRow(d->rowSpanEndLogical(span.top(), span.height()));
@@ -1520,71 +1547,106 @@ QModelIndex QTableView::moveCursor(CursorAction cursorAction, Qt::KeyboardModifi
         if (QApplication::keypadNavigationEnabled() && visualRow >= bottom)
             visualRow = -1;
 #endif
-        ++visualRow;
-        while (visualRow < bottom && d->isVisualRowHiddenOrDisabled(visualRow, visualColumn))
+        int r = d->logicalRow(visualRow);
+        int c = d->logicalColumn(visualColumn);
+        if (r != -1 && d->hasSpans()) {
+            QSpanCollection::Span span = d->span(r, c);
+            if (span.width() > 1 || span.height() > 1)
+                visualRow = d->visualRow(d->rowSpanEndLogical(span.top(), span.height()));
+        }
+        while (visualRow <= bottom) {
             ++visualRow;
-        if (d->hasSpans()) {
-            int row = d->logicalRow(visualRow);
-            QSpanCollection::Span span = d->span(row, current.column());
-            visualColumn = d->visualColumn(span.left());
+            r = d->logicalRow(visualRow);
+            c = d->logicalColumn(visualColumn);
+            if (r == -1 || (!isRowHidden(r) && d->isCellEnabled(r, c)))
+                break;
         }
+        if (visualRow > bottom)
+            visualRow = originalRow;
         break;
-    case MovePrevious: {
-        int left = 0;
-        while (d->isVisualColumnHiddenOrDisabled(visualRow, left) && left < right)
-            ++left;
-        if (visualColumn == left) {
-            visualColumn = right;
-            int top = 0;
-            while (top < bottom && d->isVisualRowHiddenOrDisabled(top, visualColumn))
-                ++top;
-            if (visualRow == top)
+    }
+    case MovePrevious:
+    case MoveLeft: {
+        int originalRow = visualRow;
+        int originalColumn = visualColumn;
+        bool firstTime = true;
+        bool looped = false;
+        bool wrapped = false;
+        do {
+            int r = d->logicalRow(visualRow);
+            int c = d->logicalColumn(visualColumn);
+            if (firstTime && c != -1 && d->hasSpans()) {
+                firstTime = false;
+                QSpanCollection::Span span = d->span(r, c);
+                if (span.width() > 1 || span.height() > 1)
+                    visualColumn = d->visualColumn(span.left());
+            }
+            while (visualColumn >= 0) {
+                --visualColumn;
+                r = d->logicalRow(visualRow);
+                c = d->logicalColumn(visualColumn);
+                if (r == -1 || c == -1 || (!isRowHidden(r) && !isColumnHidden(c) && d->isCellEnabled(r, c)))
+                    break;
+                if (wrapped && (originalRow < visualRow || (originalRow == visualRow && originalColumn <= visualColumn))) {
+                    looped = true;
+                    break;
+                }
+            }
+            if (cursorAction == MoveLeft || visualColumn >= 0)
+                break;
+            visualColumn = right + 1;
+            if (visualRow == 0) {
+                wrapped == true;
                 visualRow = bottom;
-            else
-                --visualRow;
-            while (visualRow > 0 && d->isVisualRowHiddenOrDisabled(visualRow, visualColumn))
+            } else {
                 --visualRow;
-            break;
-        } // else MoveLeft
-    }
-    case MoveLeft:
-        --visualColumn;
-        while (visualColumn > 0 && d->isVisualColumnHiddenOrDisabled(visualRow, visualColumn))
-            --visualColumn;
-        if (d->hasSpans()) {
-            int column = d->logicalColumn(visualColumn);
-            QSpanCollection::Span span = d->span(current.row(), column);
-            visualRow = d->visualRow(span.top());
-            visualColumn = d->visualColumn(span.left());
-        }
+            }
+        } while (!looped);
+        if (visualColumn < 0)
+            visualColumn = originalColumn;
         break;
+    }
     case MoveNext:
-        if (visualColumn == right) {
-            visualColumn = 0;
-            while (visualColumn < right && d->isVisualColumnHiddenOrDisabled(visualRow, visualColumn))
+    case MoveRight: {
+        int originalRow = visualRow;
+        int originalColumn = visualColumn;
+        bool firstTime = true;
+        bool looped = false;
+        bool wrapped = false;
+        do {
+            int r = d->logicalRow(visualRow);
+            int c = d->logicalColumn(visualColumn);
+            if (firstTime && c != -1 && d->hasSpans()) {
+                firstTime = false;
+                QSpanCollection::Span span = d->span(r, c);
+                if (span.width() > 1 || span.height() > 1)
+                    visualColumn = d->visualColumn(d->columnSpanEndLogical(span.left(), span.width()));
+            }
+            while (visualColumn <= right) {
                 ++visualColumn;
-            if (visualRow == bottom)
+                r = d->logicalRow(visualRow);
+                c = d->logicalColumn(visualColumn);
+                if (r == -1 || c == -1 || (!isRowHidden(r) && !isColumnHidden(c) && d->isCellEnabled(r, c)))
+                    break;
+                if (wrapped && (originalRow > visualRow || (originalRow == visualRow && originalColumn >= visualColumn))) {
+                    looped = true;
+                    break;
+                }
+            }
+            if (cursorAction == MoveRight || visualColumn <= right)
+                break;
+            visualColumn = -1;
+            if (visualRow == bottom) {
+                wrapped = true;
                 visualRow = 0;
-            else
-                ++visualRow;
-            while (visualRow < bottom && d->isVisualRowHiddenOrDisabled(visualRow, visualColumn))
+            } else {
                 ++visualRow;
-            break;
-        } // else MoveRight
-    case MoveRight:
-        if (d->hasSpans()) {
-            QSpanCollection::Span span = d->span(current.row(), current.column());
-            visualColumn = d->visualColumn(d->columnSpanEndLogical(span.left(), span.width()));
-        }
-        ++visualColumn;
-        while (visualColumn < right && d->isVisualColumnHiddenOrDisabled(visualRow, visualColumn))
-            ++visualColumn;
-        if (d->hasSpans()) {
-            int column = d->logicalColumn(visualColumn);
-            QSpanCollection::Span span = d->span(current.row(), column);
-            visualRow = d->visualRow(span.top());
-        }
+            }
+        } while (!looped);
+        if (visualColumn > right)
+            visualColumn = originalColumn;
         break;
+    }
     case MoveHome:
         visualColumn = 0;
         while (visualColumn < right && d->isVisualColumnHiddenOrDisabled(visualRow, visualColumn))
@@ -1613,14 +1675,15 @@ QModelIndex QTableView::moveCursor(CursorAction cursorAction, Qt::KeyboardModifi
         return d->model->index(newRow, current.column(), d->root);
     }}
 
+    d->visualCursor = QPoint(visualColumn, visualRow);
     int logicalRow = d->logicalRow(visualRow);
     int logicalColumn = d->logicalColumn(visualColumn);
     if (!d->model->hasIndex(logicalRow, logicalColumn, d->root))
         return QModelIndex();
 
     QModelIndex result = d->model->index(logicalRow, logicalColumn, d->root);
-    if (!isIndexHidden(result) && d->isIndexEnabled(result))
-        return d->model->index(logicalRow, logicalColumn, d->root);
+    if (!d->isRowHidden(logicalRow) && !d->isColumnHidden(logicalColumn) && d->isIndexEnabled(result))
+        return result;
 
     return QModelIndex();
 }
@@ -2375,7 +2438,8 @@ bool QTableView::isCornerButtonEnabled() const
 QRect QTableView::visualRect(const QModelIndex &index) const
 {
     Q_D(const QTableView);
-    if (!d->isIndexValid(index) || index.parent() != d->root || isIndexHidden(index) )
+    if (!d->isIndexValid(index) || index.parent() != d->root
+        || (!d->hasSpans() && isIndexHidden(index)))
         return QRect();
 
     d->executePostedLayout();
diff --git a/src/gui/itemviews/qtableview_p.h b/src/gui/itemviews/qtableview_p.h
index c785bd7..9fa14c2 100644
--- a/src/gui/itemviews/qtableview_p.h
+++ b/src/gui/itemviews/qtableview_p.h
@@ -135,7 +135,8 @@ public:
           rowSectionAnchor(-1), columnSectionAnchor(-1),
           columnResizeTimerID(0), rowResizeTimerID(0),
           horizontalHeader(0), verticalHeader(0),
-          sortingEnabled(false), geometryRecursionBlock(false)
+          sortingEnabled(false), geometryRecursionBlock(false),
+          visualCursor(QPoint())
  {
     wrapItemText = true;
 #ifndef QT_NO_DRAGANDDROP
@@ -183,6 +184,7 @@ public:
     QWidget *cornerWidget;
     bool sortingEnabled;
     bool geometryRecursionBlock;
+    QPoint visualCursor;  // (Row,column) cell coordinates to track through span navigation.
 
     QSpanCollection spans;
 
diff --git a/tests/auto/qtableview/tst_qtableview.cpp b/tests/auto/qtableview/tst_qtableview.cpp
index bb0e226..4ea1b19 100644
--- a/tests/auto/qtableview/tst_qtableview.cpp
+++ b/tests/auto/qtableview/tst_qtableview.cpp
@@ -100,6 +100,9 @@ private slots:
     void moveCursor_data();
     void moveCursor();
 
+    void moveCursorStrikesBack_data();
+    void moveCursorStrikesBack();
+
     void hideRows_data();
     void hideRows();
 
@@ -252,12 +255,43 @@ public:
           row_count(rows),
           column_count(columns),
           can_fetch_more(false),
-          fetch_more_count(0) {}
+          fetch_more_count(0),
+          disabled_rows(),
+          disabled_columns() {}
 
     int rowCount(const QModelIndex& = QModelIndex()) const { return row_count; }
     int columnCount(const QModelIndex& = QModelIndex()) const { return column_count; }
     bool isEditable(const QModelIndex &) const { return true; }
 
+    Qt::ItemFlags flags(const QModelIndex &index) const
+    {
+        Qt::ItemFlags index_flags = QAbstractTableModel::flags(index);
+        if (disabled_rows.contains(index.row())
+            || disabled_columns.contains(index.column()))
+            index_flags &= ~Qt::ItemIsEnabled;
+        return index_flags;
+    }
+
+    void disableRow(int row)
+    {
+        disabled_rows.insert(row);
+    }
+
+    void enableRow(int row)
+    {
+        disabled_rows.remove(row);
+    }
+
+    void disableColumn(int column)
+    {
+        disabled_columns.insert(column);
+    }
+
+    void enableColumn(int column)
+    {
+        disabled_columns.remove(column);
+    }
+
     QVariant data(const QModelIndex &idx, int role) const
     {
         if (!idx.isValid() || idx.row() >= row_count || idx.column() >= column_count) {
@@ -363,6 +397,8 @@ public:
     int column_count;
     bool can_fetch_more;
     int fetch_more_count;
+    QSet<int> disabled_rows;
+    QSet<int> disabled_columns;
 };
 
 class QtTestTableView : public QTableView
@@ -834,7 +870,7 @@ void tst_QTableView::moveCursor_data()
         << 4 << 4 << -1 << 2
         << 0 << 2
         << int(QtTestTableView::MoveNext) << int(Qt::NoModifier)
-        << 1 << 0 << IntPair(0,0) << IntPair(3,0);
+        << 1 << 3 << IntPair(0,0) << IntPair(3,0);
 
     // MoveLeft
     QTest::newRow("MoveLeft (0,0)")
@@ -1137,6 +1173,132 @@ void tst_QTableView::moveCursor()
     QCOMPARE(newIndex.column(), expectedColumn);
 }
 
+void tst_QTableView::moveCursorStrikesBack_data()
+{
+    QTest::addColumn<int>("hideRow");
+    QTest::addColumn<int>("hideColumn");
+    QTest::addColumn<IntList>("disableRows");
+    QTest::addColumn<IntList>("disableColumns");
+    QTest::addColumn<QRect>("span");
+
+    QTest::addColumn<int>("startRow");
+    QTest::addColumn<int>("startColumn");
+    QTest::addColumn<IntList>("cursorMoveActions");
+    QTest::addColumn<int>("expectedRow");
+    QTest::addColumn<int>("expectedColumn");
+
+    QTest::newRow("Last column disabled. Task QTBUG-3878") << -1 << -1
+            << IntList()
+            << (IntList() << 6)
+            << QRect()
+            << 0 << 5 << (IntList() << int(QtTestTableView::MoveNext))
+            << 1 << 0;
+
+    QTest::newRow("Span, anchor column hidden") << -1 << 1
+            << IntList()
+            << IntList()
+            << QRect(1, 2, 2, 3)
+            << 2 << 0 << (IntList() << int(QtTestTableView::MoveNext))
+            << 2 << 2;
+
+    QTest::newRow("Span, anchor column disabled") << -1 << -1
+            << IntList()
+            << (IntList() << 1)
+            << QRect(1, 2, 2, 3)
+            << 2 << 0 << (IntList() << int(QtTestTableView::MoveNext))
+            << 2 << 2;
+
+    QTest::newRow("Span, anchor row hidden") << 2 << -1
+            << IntList()
+            << IntList()
+            << QRect(1, 2, 2, 3)
+            << 1 << 2 << (IntList() << int(QtTestTableView::MoveDown))
+            << 3 << 2;
+
+    QTest::newRow("Span, anchor row disabled") << -1 << -1
+            << (IntList() << 2)
+            << IntList()
+            << QRect(1, 2, 2, 3)
+            << 1 << 2 << (IntList() << int(QtTestTableView::MoveDown))
+            << 3 << 2;
+
+    QTest::newRow("Move through span right") << -1 << -1
+            << IntList()
+            << IntList()
+            << QRect(1, 2, 2, 3)
+            << 3 << 0 << (IntList() << int(QtTestTableView::MoveRight) << int(QtTestTableView::MoveRight))
+            << 3 << 3;
+
+    QTest::newRow("Move through span left") << -1 << -1
+            << IntList()
+            << IntList()
+            << QRect(1, 2, 2, 3)
+            << 3 << 3 << (IntList() << int(QtTestTableView::MoveLeft) << int(QtTestTableView::MoveLeft))
+            << 3 << 0;
+
+    QTest::newRow("Move through span down") << -1 << -1
+            << IntList()
+            << IntList()
+            << QRect(1, 2, 2, 3)
+            << 1 << 2 << (IntList() << int(QtTestTableView::MoveDown) << int(QtTestTableView::MoveDown))
+            << 5 << 2;
+
+    QTest::newRow("Move through span up") << -1 << -1
+            << IntList()
+            << IntList()
+            << QRect(1, 2, 2, 3)
+            << 5 << 2 << (IntList() << int(QtTestTableView::MoveUp) << int(QtTestTableView::MoveUp))
+            << 1 << 2;
+}
+
+void tst_QTableView::moveCursorStrikesBack()
+{
+    QFETCH(int, hideRow);
+    QFETCH(int, hideColumn);
+    QFETCH(IntList, disableRows);
+    QFETCH(IntList, disableColumns);
+    QFETCH(QRect, span);
+
+    QFETCH(int, startRow);
+    QFETCH(int, startColumn);
+    QFETCH(IntList, cursorMoveActions);
+    QFETCH(int, expectedRow);
+    QFETCH(int, expectedColumn);
+
+    QtTestTableModel model(7, 7);
+    QtTestTableView view;
+    view.setModel(&model);
+    view.hideRow(hideRow);
+    view.hideColumn(hideColumn);
+
+    foreach (int row, disableRows)
+        model.disableRow(row);
+    foreach (int column, disableColumns)
+        model.disableColumn(column);
+
+    if (span.height() && span.width())
+        view.setSpan(span.top(), span.left(), span.height(), span.width());
+    view.show();
+
+    QModelIndex index = model.index(startRow, startColumn);
+    view.setCurrentIndex(index);
+
+    int newRow = -1;
+    int newColumn = -1;
+    foreach (int cursorMoveAction, cursorMoveActions) {
+        QModelIndex newIndex = view.moveCursor((QtTestTableView::CursorAction)cursorMoveAction, 0);
+        view.setCurrentIndex(newIndex);
+        newRow = newIndex.row();
+        newColumn = newIndex.column();
+    }
+
+    // expected fails, task 119433
+    if(newRow == -1)
+        return;
+    QCOMPARE(newRow, expectedRow);
+    QCOMPARE(newColumn, expectedColumn);
+}
+
 void tst_QTableView::hideRows_data()
 {
     QTest::addColumn<int>("rowCount");