From e39d629ebe9044b505ac35eaae3ab9c214d452a2 Mon Sep 17 00:00:00 2001 From: Jan Arve Saether Date: Wed, 11 Sep 2013 11:09:40 +0200 Subject: [PATCH] Include hidden headers in trees and tables consistently There was a disagreement between the a11y plugin and QTreeView whether the horizontal header should have been exposed or not. When the header was hidden, this resulted in that we sent an event with a child id that was wrong, or in worst case higher than QAI::childrenCount(). This was the reason we got the warning output as described in the task. With this commit, we consistently *expose* hidden headers both for QTreeView and QTableView, but ensure that their state().invisible is set to true instead. This makes it consistent with how hidden cells are exposed. This also fixes a bug in QTableViewPrivate::accessibleTable2Index where we always added 1 to the index, which was spotted while writing the test. Task-number: QTBUG-33247 Change-Id: Ifd1f83d56296dd071424fdb81fce7628bc24fe0a Reviewed-by: Frederik Gladhorn --- src/plugins/accessible/widgets/itemviews.cpp | 29 +++++++++++-- src/plugins/accessible/widgets/itemviews.h | 2 + src/widgets/itemviews/qtableview_p.h | 5 ++- src/widgets/itemviews/qtreeview.cpp | 14 +++++-- src/widgets/itemviews/qtreeview.h | 1 + .../qaccessibility/tst_qaccessibility.cpp | 41 ++++++++++++++++--- 6 files changed, 77 insertions(+), 15 deletions(-) diff --git a/src/plugins/accessible/widgets/itemviews.cpp b/src/plugins/accessible/widgets/itemviews.cpp index 86c7812553..9dcf815cce 100644 --- a/src/plugins/accessible/widgets/itemviews.cpp +++ b/src/plugins/accessible/widgets/itemviews.cpp @@ -120,8 +120,6 @@ QHeaderView *QAccessibleTable::horizontalHeader() const #ifndef QT_NO_TREEVIEW } else if (const QTreeView *tv = qobject_cast(view())) { header = tv->header(); - if (header && header->isHidden()) - header = 0; #endif } return header; @@ -1123,7 +1121,12 @@ QAccessible::Role QAccessibleTableHeaderCell::role() const QAccessible::State QAccessibleTableHeaderCell::state() const { - return QAccessible::State(); + QAccessible::State s; + if (QHeaderView *h = headerView()) { + s.invisible = !h->testAttribute(Qt::WA_WState_Visible); + s.disabled = !h->isEnabled(); + } + return s; } QRect QAccessibleTableHeaderCell::rect() const @@ -1194,6 +1197,26 @@ QAccessibleInterface *QAccessibleTableHeaderCell::child(int) const return 0; } +QHeaderView *QAccessibleTableHeaderCell::headerView() const +{ + QHeaderView *header = 0; + if (false) { +#ifndef QT_NO_TABLEVIEW + } else if (const QTableView *tv = qobject_cast(view)) { + if (orientation == Qt::Horizontal) { + header = tv->horizontalHeader(); + } else { + header = tv->verticalHeader(); + } +#endif +#ifndef QT_NO_TREEVIEW + } else if (const QTreeView *tv = qobject_cast(view)) { + header = tv->header(); +#endif + } + return header; +} + #endif // QT_NO_ITEMVIEWS QT_END_NAMESPACE diff --git a/src/plugins/accessible/widgets/itemviews.h b/src/plugins/accessible/widgets/itemviews.h index 72b2339dd3..7692ce0e77 100644 --- a/src/plugins/accessible/widgets/itemviews.h +++ b/src/plugins/accessible/widgets/itemviews.h @@ -239,6 +239,8 @@ public: QAccessibleInterface *child(int index) const; private: + QHeaderView *headerView() const; + QPointer view; int index; Qt::Orientation orientation; diff --git a/src/widgets/itemviews/qtableview_p.h b/src/widgets/itemviews/qtableview_p.h index d36dcd2c3a..74eb7b7728 100644 --- a/src/widgets/itemviews/qtableview_p.h +++ b/src/widgets/itemviews/qtableview_p.h @@ -170,8 +170,9 @@ public: } inline int accessibleTable2Index(const QModelIndex &index) const { - return (index.row() + (horizontalHeader ? 1 : 0)) * (index.model()->columnCount() + (verticalHeader ? 1 : 0)) - + index.column() + (verticalHeader ? 1 : 0) + 1; + const int vHeader = verticalHeader ? 1 : 0; + return (index.row() + (horizontalHeader ? 1 : 0)) * (index.model()->columnCount() + vHeader) + + index.column() + vHeader; } int sectionSpanEndLogical(const QHeaderView *header, int logical, int span) const; diff --git a/src/widgets/itemviews/qtreeview.cpp b/src/widgets/itemviews/qtreeview.cpp index b5f1b12365..204a6c923f 100644 --- a/src/widgets/itemviews/qtreeview.cpp +++ b/src/widgets/itemviews/qtreeview.cpp @@ -3912,9 +3912,8 @@ void QTreeView::currentChanged(const QModelIndex ¤t, const QModelIndex &pr } #ifndef QT_NO_ACCESSIBILITY if (QAccessible::isActive() && current.isValid()) { - int entry = (visualIndex(current) + (header()?1:0))*current.model()->columnCount()+current.column(); QAccessibleEvent event(this, QAccessible::Focus); - event.setChild(entry); + event.setChild(accessibleTree2Index(current)); QAccessible::updateAccessibility(&event); } #endif @@ -3932,7 +3931,7 @@ void QTreeView::selectionChanged(const QItemSelection &selected, // ### does not work properly for selection ranges. QModelIndex sel = selected.indexes().value(0); if (sel.isValid()) { - int entry = (visualIndex(sel) + (header()?1:0))*sel.model()->columnCount()+sel.column(); + int entry = accessibleTree2Index(sel); Q_ASSERT(entry >= 0); QAccessibleEvent event(this, QAccessible::Selection); event.setChild(entry); @@ -3940,7 +3939,7 @@ void QTreeView::selectionChanged(const QItemSelection &selected, } QModelIndex desel = deselected.indexes().value(0); if (desel.isValid()) { - int entry = (visualIndex(desel) + (header()?1:0))*desel.model()->columnCount()+desel.column(); + int entry = accessibleTree2Index(desel); Q_ASSERT(entry >= 0); QAccessibleEvent event(this, QAccessible::SelectionRemove); event.setChild(entry); @@ -3957,6 +3956,13 @@ int QTreeView::visualIndex(const QModelIndex &index) const return d->viewIndex(index); } +int QTreeView::accessibleTree2Index(const QModelIndex &index) const +{ + // Note that this will include the header, even if its hidden. + return (visualIndex(index) + (header() ? 1 : 0)) * index.model()->columnCount() + index.column(); +} + + QT_END_NAMESPACE #include "moc_qtreeview.cpp" diff --git a/src/widgets/itemviews/qtreeview.h b/src/widgets/itemviews/qtreeview.h index d9c6cd9269..a29e9b64b0 100644 --- a/src/widgets/itemviews/qtreeview.h +++ b/src/widgets/itemviews/qtreeview.h @@ -224,6 +224,7 @@ private: friend class QAccessibleTree; friend class QAccessibleTableCell; int visualIndex(const QModelIndex &index) const; + int accessibleTree2Index(const QModelIndex &index) const; Q_DECLARE_PRIVATE(QTreeView) Q_DISABLE_COPY(QTreeView) diff --git a/tests/auto/other/qaccessibility/tst_qaccessibility.cpp b/tests/auto/other/qaccessibility/tst_qaccessibility.cpp index 557189f4e2..4186572be3 100644 --- a/tests/auto/other/qaccessibility/tst_qaccessibility.cpp +++ b/tests/auto/other/qaccessibility/tst_qaccessibility.cpp @@ -2600,11 +2600,23 @@ void tst_QAccessibility::treeTest() QCOMPARE(iface->indexOfChild(child2), 4); QCOMPARE(child2->text(QAccessible::Name), QString("Austria")); - treeView->setHeaderHidden(true); - QAccessibleInterface *accSpain = iface->child(0); - QCOMPARE(accSpain->role(), QAccessible::TreeItem); - QCOMPARE(iface->indexOfChild(accSpain), 0); - treeView->setHeaderHidden(false); + bool headerHidden = true; + do { + treeView->setHeaderHidden(headerHidden); + header1 = iface->child(0); + QCOMPARE(header1->role(), QAccessible::ColumnHeader); + QCOMPARE(!!header1->state().invisible, headerHidden); + QCOMPARE(header1->text(QAccessible::Name), QStringLiteral("Artist")); + header1 = iface->child(1); + QCOMPARE(header1->role(), QAccessible::ColumnHeader); + QCOMPARE(!!header1->state().invisible, headerHidden); + QCOMPARE(header1->text(QAccessible::Name), QStringLiteral("Work")); + + QAccessibleInterface *accSpain = iface->child(2); + QCOMPARE(accSpain->role(), QAccessible::TreeItem); + QCOMPARE(iface->indexOfChild(accSpain), 2); + headerHidden = !headerHidden; + } while (!headerHidden); QTestAccessibility::clearEvents(); @@ -2931,8 +2943,25 @@ void tst_QAccessibility::tableTest() cell30_new = table2->cellAt(3, 0); QCOMPARE(cell30_new, cell20); QCOMPARE(iface->indexOfChild(cell30_new), 21); - delete tableView; + + { + QTestAccessibility::clearEvents(); + QModelIndex index00 = tableView->model()->index(1, 1, tableView->rootIndex()); + tableView->setSelectionBehavior(QAbstractItemView::SelectItems); + tableView->setSelectionMode(QAbstractItemView::SingleSelection); + tableView->selectionModel()->select(index00, QItemSelectionModel::ClearAndSelect); + QAccessibleEvent event(tableView, QAccessible::Selection); + event.setChild(12); + QCOMPARE(QTestAccessibility::containsEvent(&event), true); + QTestAccessibility::clearEvents(); + tableView->setSelectionBehavior(QAbstractItemView::SelectItems); + tableView->setSelectionMode(QAbstractItemView::SingleSelection); + tableView->selectionModel()->select(index00, QItemSelectionModel::ClearAndSelect); + tableView->horizontalHeader()->setVisible(false); + + } + delete tableView; QVERIFY(!QAccessible::accessibleInterface(id00)); QTestAccessibility::clearEvents(); }