From 09548cb710741bd3801ad3713bb3498ff5376a06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ingo=20Kl=C3=B6cker?= Date: Mon, 22 Apr 2024 20:34:00 +0200 Subject: [PATCH] Fix accessibility of list views with underlying multi-column model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 (cherry picked from commit cd00ce4bea6f0386048bd267495433cffe83ab12) Reviewed-by: Qt Cherry-pick Bot --- src/widgets/accessible/itemviews.cpp | 109 ++++++++++++++++-- src/widgets/accessible/itemviews_p.h | 25 ++++ .../accessible/qaccessiblewidgetfactory.cpp | 6 +- .../qaccessibility/tst_qaccessibility.cpp | 35 +++--- 4 files changed, 152 insertions(+), 23 deletions(-) diff --git a/src/widgets/accessible/itemviews.cpp b/src/widgets/accessible/itemviews.cpp index 7b0dffcc497..15f61465985 100644 --- a/src/widgets/accessible/itemviews.cpp +++ b/src/widgets/accessible/itemviews.cpp @@ -39,16 +39,24 @@ 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(); - const int vHeader = verticalHeader() ? 1 : 0; - const int hHeader = horizontalHeader() ? 1 : 0; - return (index.row() + hHeader) * (theModel->columnCount(rootIndex) + vHeader) - + (index.column() + vHeader); +#if QT_CONFIG(listview) + if (m_role == QAccessible::List) { + if (index.column() != qobject_cast(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) * (columnCount() + vHeader) + + (index.column() + vHeader); + } } QAccessibleTable::QAccessibleTable(QWidget *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(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(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 QAccessibleList::selectedCells() const +{ + QAbstractItemView *theView = view(); + QList cells; + if (!view()->selectionModel() || columnCount() == 0) + return cells; + const QListView *listView = qobject_cast(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(); } diff --git a/src/widgets/accessible/itemviews_p.h b/src/widgets/accessible/itemviews_p.h index 077f14de1d2..9b30f36ced3 100644 --- a/src/widgets/accessible/itemviews_p.h +++ b/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 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. diff --git a/src/widgets/accessible/qaccessiblewidgetfactory.cpp b/src/widgets/accessible/qaccessiblewidgetfactory.cpp index 664e35a6e72..7d9bf9bc241 100644 --- a/src/widgets/accessible/qaccessiblewidgetfactory.cpp +++ b/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) diff --git a/tests/auto/other/qaccessibility/tst_qaccessibility.cpp b/tests/auto/other/qaccessibility/tst_qaccessibility.cpp index cab88e2fc0f..ffb9f7b7bb5 100644 --- a/tests/auto/other/qaccessibility/tst_qaccessibility.cpp +++ b/tests/auto/other/qaccessibility/tst_qaccessibility.cpp @@ -2852,11 +2852,15 @@ void tst_QAccessibility::scrollAreaTest() void tst_QAccessibility::listTest() { { - auto lvHolder = std::make_unique(); + const auto modelHolder = std::make_unique(); + 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(); auto listView = lvHolder.get(); - listView->addItem("Oslo"); - listView->addItem("Berlin"); - listView->addItem("Brisbane"); + listView->setModel(model); + listView->setModelColumn(1); 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->selectedItems().at(0)->text(), QLatin1String("Munich")); + QCOMPARE(listView->selectionModel()->selectedIndexes().size(), 1); + 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); - delete listView->takeItem(2); + for (auto item : model->takeRow(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(); }