Fix accessibility of list views with underlying multi-column model

A list view should always expose a table with a single column to
accessibility tools even if the underlying model has multiple columns.
Several functions need to be changed so that they only consider the
model column that was set on the list view.

For a list view logicalIndex() must only consider indexes for the
model column. For valid indexes the logical index is simply the row
because list views have neither row headers nor column headers.

The column count for list views is always 1 (unless the model has no
columns). The child count needs to use the column count of the
accessible table instead of the column count of the underlying model.

child(), cellAt(), selectedCellCount(), and selectedCells() get
separate implementation for list views.

Fixes: QTBUG-33786
Pick-to: 6.7 6.6
Change-Id: I18c604efa2014267bb6e3b68e403e436bdcbc4ce
Reviewed-by: Jan Arve Sæther <jan-arve.saether@qt.io>
(cherry picked from commit cd00ce4bea6f0386048bd267495433cffe83ab12)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

src/widgets/accessible/itemviews.cpp

@@ -39,17 +39,25 @@ QAbstractItemView *QAccessibleTable::view() const
 
 int QAccessibleTable::logicalIndex(const QModelIndex &index) const
 {
-    const QAbstractItemView *theView = view();
     const QAbstractItemModel *theModel = index.model();
     if (!theModel || !index.isValid())
         return -1;
 
-    const QModelIndex rootIndex = theView->rootIndex();
+#if QT_CONFIG(listview)
+    if (m_role == QAccessible::List) {
+        if (index.column() != qobject_cast<const QListView*>(view())->modelColumn())
+            return -1;
+        else
+            return index.row();
+    } else
+#endif
+    {
         const int vHeader = verticalHeader() ? 1 : 0;
         const int hHeader = horizontalHeader() ? 1 : 0;
-    return (index.row() + hHeader) * (theModel->columnCount(rootIndex) + vHeader)
+        return (index.row() + hHeader) * (columnCount() + vHeader)
              + (index.column() + vHeader);
     }
+}
 
 QAccessibleTable::QAccessibleTable(QWidget *w)
     : QAccessibleObject(w)
@@ -122,6 +130,7 @@ QAccessibleInterface *QAccessibleTable::cellAt(int row, int column) const
     const QAbstractItemModel *theModel = theView->model();
     if (!theModel)
         return nullptr;
+    Q_ASSERT(role() != QAccessible::List);
     Q_ASSERT(role() != QAccessible::Tree);
     QModelIndex index = theModel->index(row, column, theView->rootIndex());
     if (Q_UNLIKELY(!index.isValid())) {
@@ -151,7 +160,8 @@ int QAccessibleTable::columnCount() const
     const QAbstractItemModel *theModel = theView->model();
     if (!theModel)
         return 0;
-    return theModel->columnCount(theView->rootIndex());
+    const int modelColumnCount = theModel->columnCount(theView->rootIndex());
+    return m_role == QAccessible::List ? qMin(1, modelColumnCount) : modelColumnCount;
 }
 
 int QAccessibleTable::rowCount() const
@@ -541,7 +551,7 @@ int QAccessibleTable::childCount() const
     const QModelIndex rootIndex = theView->rootIndex();
     int vHeader = verticalHeader() ? 1 : 0;
     int hHeader = horizontalHeader() ? 1 : 0;
-    return (theModel->rowCount(rootIndex) + hHeader) * (theModel->columnCount(rootIndex) + vHeader);
+    return (theModel->rowCount(rootIndex) + hHeader) * (columnCount() + vHeader);
 }
 
 int QAccessibleTable::indexOfChild(const QAccessibleInterface *iface) const
@@ -969,6 +979,84 @@ bool QAccessibleTree::selectRow(int row)
 
 #endif // QT_CONFIG(treeview)
 
+#if QT_CONFIG(listview)
+
+// LIST VIEW
+
+QAccessibleInterface *QAccessibleList::child(int logicalIndex) const
+{
+    QAbstractItemView *theView = view();
+    const QAbstractItemModel *theModel = theView->model();
+    if (!theModel)
+        return nullptr;
+
+    if (columnCount() == 0) {
+        return nullptr;
+    }
+
+    const auto id = childToId.constFind(logicalIndex);
+    if (id != childToId.constEnd())
+        return QAccessible::accessibleInterface(id.value());
+
+    const QListView *listView = qobject_cast<const QListView*>(theView);
+    Q_ASSERT(listView);
+    int row = logicalIndex;
+    int column = listView->modelColumn();
+
+    const QModelIndex rootIndex = theView->rootIndex();
+    const QModelIndex index = theModel->index(row, column, rootIndex);
+    if (Q_UNLIKELY(!index.isValid())) {
+        qWarning("QAccessibleList::child: Invalid index at: %d %d", row, column);
+        return nullptr;
+    }
+    const auto iface = new QAccessibleTableCell(theView, index, cellRole());
+
+    QAccessible::registerAccessibleInterface(iface);
+    childToId.insert(logicalIndex, QAccessible::uniqueId(iface));
+    return iface;
+}
+
+QAccessibleInterface *QAccessibleList::cellAt(int row, int column) const
+{
+    if (column != 0)
+        return nullptr;
+
+    return child(row);
+}
+
+int QAccessibleList::selectedCellCount() const
+{
+    QAbstractItemView *theView = view();
+    if (!theView->selectionModel())
+        return 0;
+    const QListView *listView = qobject_cast<const QListView*>(theView);
+    const int modelColumn = listView->modelColumn();
+    const QModelIndexList selectedIndexes = theView->selectionModel()->selectedIndexes();
+    return std::count_if(selectedIndexes.cbegin(), selectedIndexes.cend(),
+                         [modelColumn](const auto &index) {
+                             return index.column() == modelColumn;
+                         });
+}
+
+QList<QAccessibleInterface *> QAccessibleList::selectedCells() const
+{
+    QAbstractItemView *theView = view();
+    QList<QAccessibleInterface*> cells;
+    if (!view()->selectionModel() || columnCount() == 0)
+        return cells;
+    const QListView *listView = qobject_cast<const QListView*>(theView);
+    const int modelColumn = listView->modelColumn();
+    const QModelIndexList selectedIndexes = theView->selectionModel()->selectedIndexes();
+    cells.reserve(qMin(selectedIndexes.size(), rowCount()));
+    for (const QModelIndex &index : selectedIndexes)
+        if (index.column() == modelColumn) {
+            cells.append(child(index.row()));
+        }
+    return cells;
+}
+
+#endif // QT_CONFIG(listview)
+
 // TABLE CELL
 
 QAccessibleTableCell::QAccessibleTableCell(QAbstractItemView *view_, const QModelIndex &index_, QAccessible::Role role_)
@@ -1042,6 +1130,11 @@ int QAccessibleTableCell::columnIndex() const
 {
     if (!isValid())
         return -1;
+#if QT_CONFIG(listview)
+    if (role() == QAccessible::ListItem) {
+        return 0;
+    }
+#endif
     return m_index.column();
 }
 

src/widgets/accessible/itemviews_p.h

@@ -147,6 +147,25 @@ private:
 };
 #endif
 
+#if QT_CONFIG(listview)
+class QAccessibleList :public QAccessibleTable
+{
+public:
+    explicit QAccessibleList(QWidget *w)
+        : QAccessibleTable(w)
+    {}
+
+    QAccessibleInterface *child(int index) const override;
+
+    // table interface
+    QAccessibleInterface *cellAt(int row, int column) const override;
+
+    // selection
+    int selectedCellCount() const override;
+    QList<QAccessibleInterface*> selectedCells() const override;
+};
+#endif
+
 class QAccessibleTableCell: public QAccessibleInterface, public QAccessibleTableCellInterface, public QAccessibleActionInterface
 {
 public:
@@ -198,6 +217,9 @@ friend class QAccessibleTable;
 #if QT_CONFIG(treeview)
 friend class QAccessibleTree;
 #endif
+#if QT_CONFIG(listview)
+friend class QAccessibleList;
+#endif
 };
 
 
@@ -234,6 +256,9 @@ friend class QAccessibleTable;
 #if QT_CONFIG(treeview)
 friend class QAccessibleTree;
 #endif
+#if QT_CONFIG(listview)
+friend class QAccessibleList;
+#endif
 };
 
 // This is the corner button on the top left of a table.

src/widgets/accessible/qaccessiblewidgetfactory.cpp

@@ -118,8 +118,12 @@ QAccessibleInterface *qAccessibleFactory(const QString &classname, QObject *obje
     } else if (classname == "QTreeView"_L1) {
         iface = new QAccessibleTree(widget);
 #endif // QT_CONFIG(treeview)
+#if QT_CONFIG(listview)
+    } else if (classname == "QListView"_L1) {
+        iface = new QAccessibleList(widget);
+#endif
 #if QT_CONFIG(itemviews)
-    } else if (classname == "QTableView"_L1 || classname == "QListView"_L1) {
+    } else if (classname == "QTableView"_L1) {
         iface = new QAccessibleTable(widget);
 #endif // QT_CONFIG(itemviews)
 #if QT_CONFIG(tabbar)

tests/auto/other/qaccessibility/tst_qaccessibility.cpp

@@ -2852,11 +2852,15 @@ void tst_QAccessibility::scrollAreaTest()
 void tst_QAccessibility::listTest()
 {
     {
-    auto lvHolder = std::make_unique<QListWidget>();
+    const auto modelHolder = std::make_unique<QStandardItemModel>();
+    auto model = modelHolder.get();
+    model->appendRow({new QStandardItem("Norway"), new QStandardItem("Oslo"), new QStandardItem("NOK")});
+    model->appendRow({new QStandardItem("Germany"), new QStandardItem("Berlin"), new QStandardItem("EUR")});
+    model->appendRow({new QStandardItem("Australia"), new QStandardItem("Brisbane"), new QStandardItem("AUD")});
+    auto lvHolder = std::make_unique<QListView>();
     auto listView = lvHolder.get();
-    listView->addItem("Oslo");
+    listView->setModel(model);
-    listView->addItem("Berlin");
+    listView->setModelColumn(1);
-    listView->addItem("Brisbane");
     listView->resize(400,400);
     listView->show();
     QTest::qWait(1); // Need this for indexOfchild to work.
@@ -2895,14 +2899,14 @@ void tst_QAccessibility::listTest()
     QTestAccessibility::clearEvents();
 
     // Check for events
-    QTest::mouseClick(listView->viewport(), Qt::LeftButton, { }, listView->visualItemRect(listView->item(1)).center());
+    QTest::mouseClick(listView->viewport(), Qt::LeftButton, { }, listView->visualRect(model->index(1, listView->modelColumn())).center());
     QAccessibleEvent selectionEvent(listView, QAccessible::SelectionAdd);
     selectionEvent.setChild(1);
     QAccessibleEvent focusEvent(listView, QAccessible::Focus);
     focusEvent.setChild(1);
     QVERIFY(QTestAccessibility::containsEvent(&selectionEvent));
     QVERIFY(QTestAccessibility::containsEvent(&focusEvent));
-    QTest::mouseClick(listView->viewport(), Qt::LeftButton, { }, listView->visualItemRect(listView->item(2)).center());
+    QTest::mouseClick(listView->viewport(), Qt::LeftButton, { }, listView->visualRect(model->index(2, listView->modelColumn())).center());
 
     QAccessibleEvent selectionEvent2(listView, QAccessible::SelectionAdd);
     selectionEvent2.setChild(2);
@@ -2911,7 +2915,7 @@ void tst_QAccessibility::listTest()
     QVERIFY(QTestAccessibility::containsEvent(&selectionEvent2));
     QVERIFY(QTestAccessibility::containsEvent(&focusEvent2));
 
-    listView->addItem("Munich");
+    model->appendRow({new QStandardItem("Germany"), new QStandardItem("Munich"), new QStandardItem("EUR")});
     QCOMPARE(iface->childCount(), 4);
 
     // table 2
@@ -2945,8 +2949,8 @@ void tst_QAccessibility::listTest()
     QVERIFY(!(cell4->state().selected));
     QAccessibleSelectionInterface *selection2 = iface->selectionInterface();
     selection2->select(cell4);
-    QCOMPARE(listView->selectedItems().size(), 1);
+    QCOMPARE(listView->selectionModel()->selectedIndexes().size(), 1);
-    QCOMPARE(listView->selectedItems().at(0)->text(), QLatin1String("Munich"));
+    QCOMPARE(model->itemFromIndex(listView->selectionModel()->selectedIndexes().at(0))->text(), QLatin1String("Munich"));
     QVERIFY(cell4->state().selected);
     QVERIFY(cellInterface->isSelected());
 
@@ -2958,21 +2962,24 @@ void tst_QAccessibility::listTest()
     // verify that unique id stays the same
     QAccessible::Id axidMunich = QAccessible::uniqueId(cell4);
     // insertion and deletion of items
-    listView->insertItem(1, "Helsinki");
+    model->insertRow(1, {new QStandardItem("Finland"), new QStandardItem("Helsinki"), new QStandardItem("EUR")});
     // list: Oslo, Helsinki, Berlin, Brisbane, Munich
 
     QAccessibleInterface *cellMunich2 = table2->cellAt(4,0);
     QCOMPARE(cell4, cellMunich2);
     QCOMPARE(axidMunich, QAccessible::uniqueId(cellMunich2));
 
-    delete listView->takeItem(2);
+    for (auto item : model->takeRow(2))
-    delete listView->takeItem(2);
+        delete item;
+    for (auto item : model->takeRow(2))
+        delete item;
     // list: Oslo, Helsinki, Munich
 
     QAccessibleInterface *cellMunich3 = table2->cellAt(2,0);
     QCOMPARE(cell4, cellMunich3);
     QCOMPARE(axidMunich, QAccessible::uniqueId(cellMunich3));
-    delete listView->takeItem(2);
+    for (auto item : model->takeRow(2))
+        delete item;
     // list: Oslo, Helsinki
     // verify that it doesn't return an invalid item from the cache
     QVERIFY(table2->cellAt(2,0) == 0);
@@ -3455,7 +3462,7 @@ void tst_QAccessibility::rootIndexView()
 
     view.setRootIndex(model.index(1, 0));
     QCOMPARE(accTable->rowCount(), 10);
-    QCOMPARE(accTable->columnCount(), 2);
+    QCOMPARE(accTable->columnCount(), 1);
 
     QTestAccessibility::clearEvents();
 }