macOS: Fix assertion in accessibility implementation for treeviews

In QAccessible's widget implementations, trees are treated as tables,
with a rowCount implementation that is based on the view's current
item content. That item content is the view's content, not the model's,
and it changes when tree branches are expanded.

The Cocoa bridge for accessibility allocates arrays of row data
structures based on the rowCount implementation. Those data structures
need to be invalidated and recreated when the view's content changes.
To do that, emit an accessibility event for a model reset when laying
out items changes the size of the view's item array. We don't know what
changed during that layout process to makes this any more granular.

Amends 11ae55e918082e8fdfc0c88c21049e877cc47b5b, but the problem
with the data structure being stale and incorrect would have been there
before that chain of changes optimizing. It didn't trigger an assert,
but probably resulted in incorrect data being reported.

To make trees testable, we need to actually expose them as AXOutline
to the macOS accessibility framework. Until now, they have been treated
like plain QWidget, e.g. AXGroup. This made them in practice in-
accessible. With this change, VoiceOver works much better (although not
perfeclty yet).

Also remove an assert that could be triggered by an accessibility
client asking for a cell for an invalid index (which can be reproduced
by navigating around in a tree, following debug warnings from
QAccessibleTree::indexFromLogical: invalid index).

Pick-to: 6.5
Change-Id: I7650342aa0dcd7925a94ae6a36de5a0b344c467d
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>

4 files changed, 64 insertions, 1 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoaaccessibility.mm b/src/plugins/platforms/cocoa/qcocoaaccessibility.mm
index 69ec52afe7..c5e40a4087 100644
--- a/src/plugins/platforms/cocoa/qcocoaaccessibility.mm
+++ b/src/plugins/platforms/cocoa/qcocoaaccessibility.mm
@@ -136,6 +136,7 @@ static void populateRoleMap()
     roleMap[QAccessible::Note] = NSAccessibilityGroupRole;
     roleMap[QAccessible::ComplementaryContent] = NSAccessibilityGroupRole;
     roleMap[QAccessible::Graphic] = NSAccessibilityImageRole;
+    roleMap[QAccessible::Tree] = NSAccessibilityOutlineRole;
 }
 
 /*
diff --git a/src/plugins/platforms/cocoa/qcocoaaccessibilityelement.mm b/src/plugins/platforms/cocoa/qcocoaaccessibilityelement.mm
index d63c0401fd..27fd32f91f 100644
--- a/src/plugins/platforms/cocoa/qcocoaaccessibilityelement.mm
+++ b/src/plugins/platforms/cocoa/qcocoaaccessibilityelement.mm
@@ -292,7 +292,9 @@ static void convertLineOffset(QAccessibleTextInterface *text, int *line, int *of
         QAccessibleTableInterface *table = iface->tableInterface();
         Q_ASSERT(table);
         QAccessibleInterface *cell = table->cellAt(m_rowIndex, m_columnIndex);
-        Q_ASSERT(cell && cell->isValid());
+        if (!cell)
+            return nullptr;
+        Q_ASSERT(cell->isValid());
         iface = cell;
 
         // no longer a placeholder
diff --git a/src/widgets/itemviews/qtreeview.cpp b/src/widgets/itemviews/qtreeview.cpp
index 2280cdaa2e..d4c5d837a8 100644
--- a/src/widgets/itemviews/qtreeview.cpp
+++ b/src/widgets/itemviews/qtreeview.cpp
@@ -3325,6 +3325,16 @@ void QTreeViewPrivate::layout(int i, bool recursiveExpanding, bool afterIsUninit
         return;
     }
 
+    // QAccessibleTree's rowCount implementation uses viewItems.size(), so
+    // we need to invalidate any cached accessibility data structures if
+    // that value changes during the run of this function.
+    const auto resetModelIfNeeded = qScopeGuard([oldViewItemsSize = viewItems.size(), this, q]{
+        if (oldViewItemsSize != viewItems.size() && QAccessible::isActive()) {
+            QAccessibleTableModelChangeEvent event(q, QAccessibleTableModelChangeEvent::ModelReset);
+            QAccessible::updateAccessibility(&event);
+        }
+    });
+
     int count = 0;
     if (model->hasChildren(parent)) {
         if (model->canFetchMore(parent)) {
diff --git a/tests/auto/other/qaccessibilitymac/tst_qaccessibilitymac.mm b/tests/auto/other/qaccessibilitymac/tst_qaccessibilitymac.mm
index 8dc1460585..730e8f7e96 100644
--- a/tests/auto/other/qaccessibilitymac/tst_qaccessibilitymac.mm
+++ b/tests/auto/other/qaccessibilitymac/tst_qaccessibilitymac.mm
@@ -407,6 +407,7 @@ private Q_SLOTS:
     void notificationsTest();
     void checkBoxTest();
     void tableViewTest();
+    void treeViewTest();
 
 private:
     AccessibleTestWindow *m_window;
@@ -732,5 +733,54 @@ void tst_QAccessibilityMac::tableViewTest()
     }
 }
 
+void tst_QAccessibilityMac::treeViewTest()
+{
+    QTreeWidget *tw = new QTreeWidget;
+    tw->setColumnCount(2);
+    QTreeWidgetItem *root = new QTreeWidgetItem(tw, {"/", "0"});
+    root->setExpanded(false);
+    QTreeWidgetItem *users = new QTreeWidgetItem(root,{ "Users", "1"});
+    (void)new QTreeWidgetItem(root, {"Applications", "2"});
+    QTreeWidgetItem *lastChild = new QTreeWidgetItem(root, {"Libraries", "3"});
+
+    m_window->addWidget(tw);
+    QVERIFY(QTest::qWaitForWindowExposed(m_window));
+    QCoreApplication::processEvents();
+
+    TestAXObject *appObject = [TestAXObject getApplicationAXObject];
+    QVERIFY(appObject);
+
+    NSArray *windowList = [appObject windowList];
+    // one window
+    QVERIFY([windowList count] == 1);
+    AXUIElementRef windowRef = (AXUIElementRef)[windowList objectAtIndex:0];
+    QVERIFY(windowRef != nil);
+    TestAXObject *window = [[TestAXObject alloc] initWithAXUIElementRef:windowRef];
+
+    // children of window
+    AXUIElementRef treeView = [window findDirectChildByRole:kAXOutlineRole];
+    QVERIFY(treeView != nil);
+
+    TestAXObject *tv = [[TestAXObject alloc] initWithAXUIElementRef:treeView];
+
+    // here start actual treeview tests. NSAccessibilityOutline is a specialization
+    // of NSAccessibilityTable, and we represent trees as tables.
+    // Should have 2 columns
+    const unsigned int columnCount = 2;
+    NSArray *columnArray = [tv tableColumns];
+    QCOMPARE([columnArray count], columnCount);
+
+    // should have 1 row for now - as long as the root item is not expanded
+    NSArray *rowArray = [tv tableRows];
+    QCOMPARE(int([rowArray count]), 1);
+
+    root->setExpanded(true);
+    rowArray = [tv tableRows];
+    QCOMPARE(int([rowArray count]), root->childCount() + 1);
+
+    // this should not trigger any assert
+    tw->setCurrentItem(lastChild);
+}
+
 QTEST_MAIN(tst_QAccessibilityMac)
 #include "tst_qaccessibilitymac.moc"