diff --git a/src/gui/accessible/qaccessiblecache_mac.mm b/src/gui/accessible/qaccessiblecache_mac.mm index 1c875391941..6b41867105f 100644 --- a/src/gui/accessible/qaccessiblecache_mac.mm +++ b/src/gui/accessible/qaccessiblecache_mac.mm @@ -2,24 +2,45 @@ // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only #include "qaccessiblecache_p.h" +#include // qcocoaaccessibilityelement.h in platform plugin -@interface QT_MANGLE_NAMESPACE(QMacAccessibilityElement) +QT_DECLARE_NAMESPACED_OBJC_INTERFACE(QMacAccessibilityElement, NSObject - (void)invalidate; -@end +) QT_BEGIN_NAMESPACE -void QAccessibleCache::insertElement(QAccessible::Id axid, QT_MANGLE_NAMESPACE(QMacAccessibilityElement) *element) const +bool QAccessibleCache::insertElement(QAccessible::Id axid, + QT_MANGLE_NAMESPACE(QMacAccessibilityElement) *element) const { - accessibleElements[axid] = element; + if (const auto it = accessibleElements.find(axid); it == accessibleElements.end()) { + accessibleElements[axid] = element; + [element retain]; + return true; + } else if (it.value() != element) { + auto *oldElement = it.value(); + // this might invalidate the iterator + [oldElement invalidate]; + [oldElement release]; + + accessibleElements[axid] = element; + [element retain]; + return true; + } + return false; } void QAccessibleCache::removeAccessibleElement(QAccessible::Id axid) { - QT_MANGLE_NAMESPACE(QMacAccessibilityElement) *element = elementForId(axid); - [element invalidate]; - accessibleElements.remove(axid); + // Some QAccessibleInterface instances in the cache didn't get created in + // response to a query, but when we emit events. So we might get called with + // and axid for which we don't have any element (yet). + if (QT_MANGLE_NAMESPACE(QMacAccessibilityElement) *element = elementForId(axid)) { + [element invalidate]; + accessibleElements.remove(axid); + [element release]; + } } QT_MANGLE_NAMESPACE(QMacAccessibilityElement) *QAccessibleCache::elementForId(QAccessible::Id axid) const diff --git a/src/gui/accessible/qaccessiblecache_p.h b/src/gui/accessible/qaccessiblecache_p.h index dcdf195dea4..903935bc660 100644 --- a/src/gui/accessible/qaccessiblecache_p.h +++ b/src/gui/accessible/qaccessiblecache_p.h @@ -43,7 +43,7 @@ public: #ifdef Q_OS_APPLE QT_MANGLE_NAMESPACE(QMacAccessibilityElement) *elementForId(QAccessible::Id axid) const; - void insertElement(QAccessible::Id axid, QT_MANGLE_NAMESPACE(QMacAccessibilityElement) *element) const; + bool insertElement(QAccessible::Id axid, QT_MANGLE_NAMESPACE(QMacAccessibilityElement) *element) const; #endif private Q_SLOTS: diff --git a/src/plugins/platforms/cocoa/qcocoaaccessibilityelement.h b/src/plugins/platforms/cocoa/qcocoaaccessibilityelement.h index a96ab55735d..b120e8bc6d1 100644 --- a/src/plugins/platforms/cocoa/qcocoaaccessibilityelement.h +++ b/src/plugins/platforms/cocoa/qcocoaaccessibilityelement.h @@ -16,6 +16,7 @@ QT_DECLARE_NAMESPACED_OBJC_INTERFACE(QMacAccessibilityElement, NSObject insertElement(anId, element); + if (cache->insertElement(anId, element)) + [element release]; } return element; } @@ -209,28 +219,54 @@ static void convertLineOffset(QAccessibleTextInterface *text, int *line, int *of return [self elementWithId:anId]; } -// called by QAccessibleCache::removeAccessibleElement ++ (void)removeElementsFromCache:(NSArray *)array { + for (uint i = 0; i < array.count; ++i) { + QMacAccessibilityElement *cell = [array objectAtIndex:i]; + if (cell->axid) { // it's a proper cell, remove from cache + QAccessibleCache::instance()->deleteInterface(cell->axid); + } + } +} + +// called by QAccessibleCache::removeAccessibleElement, which also releases - (void)invalidate { axid = 0; if (rows) { + [QMacAccessibilityElement removeElementsFromCache:rows]; [rows autorelease]; rows = nil; } if (columns) { + [QMacAccessibilityElement removeElementsFromCache:columns]; [columns autorelease]; columns = nil; } synthesizedRole = nil; NSAccessibilityPostNotification(self, NSAccessibilityUIElementDestroyedNotification); - [self release]; } +/*! + \internal + + If this element represents a table, then the rows and columns array are both + populated with elements representing the rows and columns. If this elements + represents a row, then the columns array is populated with elements + representing the cells. Not all of those synthesized elements might be in + the cache, but those that are need to be removed so that we don't end up + with stale representations of children when the higher-level element + expires. +*/ - (void)dealloc { - if (rows) + if (rows) { + [QMacAccessibilityElement removeElementsFromCache:rows]; [rows release]; // will also release all entries first - if (columns) + } + if (columns) { + [QMacAccessibilityElement removeElementsFromCache:columns]; [columns release]; // will also release all entries first + } + QAccessibleCache::instance()->deleteInterface(axid); [super dealloc]; } diff --git a/src/plugins/platforms/ios/quiaccessibilityelement.mm b/src/plugins/platforms/ios/quiaccessibilityelement.mm index 02fa0817e40..d7baba79436 100644 --- a/src/plugins/platforms/ios/quiaccessibilityelement.mm +++ b/src/plugins/platforms/ios/quiaccessibilityelement.mm @@ -47,7 +47,8 @@ QT_NAMESPACE_ALIAS_OBJC_CLASS(QMacAccessibilityElement); if (window && window->handle()) { auto *platformWindow = static_cast(window->handle()); element = [[self alloc] initWithId:anId withAccessibilityContainer:platformWindow->view()]; - cache->insertElement(anId, element); + if (cache->insertElement(anId, element)) + [element release]; } else { qWarning() << "Could not create a11y element for" << iface << "with window" << window diff --git a/tests/auto/other/qaccessibilitymac/tst_qaccessibilitymac.mm b/tests/auto/other/qaccessibilitymac/tst_qaccessibilitymac.mm index 4d9c14131fd..3a0995dc396 100644 --- a/tests/auto/other/qaccessibilitymac/tst_qaccessibilitymac.mm +++ b/tests/auto/other/qaccessibilitymac/tst_qaccessibilitymac.mm @@ -781,18 +781,20 @@ void tst_QAccessibilityMac::treeViewTest() QVERIFY(window.valid); // children of window - AXUIElementRef treeView = [window findDirectChildByRole:kAXOutlineRole]; - QVERIFY(treeView != nil); - TestAXObject *tv = [[TestAXObject alloc] initWithAXUIElementRef:treeView]; + auto accessibleTreeView = [window]() -> TestAXObject * { + AXUIElementRef treeView = [window findDirectChildByRole:kAXOutlineRole]; + Q_ASSERT(treeView != nil); + TestAXObject *tv = [[TestAXObject alloc] initWithAXUIElementRef:treeView]; + return tv; + }; + + TestAXObject *tv = accessibleTreeView(); QVERIFY(tv.valid); - [appObject release]; - [window release]; - // here start actual treeview tests. NSAccessibilityOutline is a specialization // of NSAccessibilityTable, and we represent trees as tables. - const auto cellText = [tv](int rowIndex, int columnIndex) -> QString { + const auto cellText = [&tv](int rowIndex, int columnIndex) -> QString { NSArray *rowArray = [tv tableRows]; Q_ASSERT(rowArray.count > uint(rowIndex)); TestAXObject *row = [[TestAXObject alloc] initWithAXUIElementRef:(AXUIElementRef)rowArray[rowIndex]]; @@ -846,9 +848,17 @@ void tst_QAccessibilityMac::treeViewTest() // this should not trigger any assert tw->setCurrentItem(lastChild); + // cache will have been cleared, reinitialize + [tv release]; + tv = accessibleTreeView(); + QVERIFY(tv.valid); + QCOMPARE(cellText(0, 0), root->text(0)); QCOMPARE(cellText(1, 0), users->text(0)); QCOMPARE(cellText(1, 1), users->text(1)); + + [appObject release]; + [window release]; } QTEST_MAIN(tst_QAccessibilityMac)