Skip to content

Commit

Permalink
QComboBox: inform accessibility about model change before selecting
Browse files Browse the repository at this point in the history
QComboBox implicitly selects the first item that gets inserted into the
model. This happens in response to the model's rowInserted signal, at
which point the item view might not have handled the rowInserted signal
yet. Because of that, the view couldn't update the accessibility bridge,
so informing accessibility about a row being selected that doens't exist
in the bridge's representation of the table yet will result in data
being out of sync, and depending on the bridge implementation trigger
asserts.

Fix this by explicitly updating the accessibility bridge before
implicitly selecting the first row.

Fixes: QTBUG-119526
Fixes: QTBUG-118585
Pick-to: 6.6 6.5
Change-Id: I2830c00751b3f18feb5d9252b23823c80229fed1
Reviewed-by: Tor Arne Vestbø <[email protected]>
Reviewed-by: Jan Arve Sæther <[email protected]>
  • Loading branch information
vohi authored and zhaojz2308 committed Mar 14, 2024
1 parent 60e8e10 commit 3236cc3
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 0 deletions.
11 changes: 11 additions & 0 deletions src/plugins/platforms/cocoa/qcocoaaccessibilityelement.mm
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@
#include "qcocoawindow.h"
#include "qcocoascreen.h"

#include <QtCore/qlogging.h>
#include <QtGui/private/qaccessiblecache_p.h>
#include <QtGui/private/qaccessiblebridgeutils_p.h>
#include <QtGui/qaccessible.h>

QT_USE_NAMESPACE

Q_LOGGING_CATEGORY(lcAccessibilityTable, "qt.accessibility.table")

#if QT_CONFIG(accessibility)

/**
Expand Down Expand Up @@ -131,6 +134,12 @@ - (instancetype)initWithId:(QAccessible::Id)anId role:(NSAccessibilityRole)role
auto *tableElement = [QMacAccessibilityElement elementWithInterface:table];
Q_ASSERT(tableElement);
Q_ASSERT(tableElement->rows);

qCDebug(lcAccessibilityTable) << "Creating cell representation for"
<< m_rowIndex << m_columnIndex
<< "in table with"
<< tableElement->rows.count << "rows";

Q_ASSERT(int(tableElement->rows.count) > m_rowIndex);
auto *rowElement = tableElement->rows[m_rowIndex];
if (!rowElement->columns) {
Expand Down Expand Up @@ -273,6 +282,8 @@ - (void)updateTableModel
if (QAccessibleInterface *iface = self.qtInterface) {
if (QAccessibleTableInterface *table = iface->tableInterface()) {
Q_ASSERT(!self.isManagedByParent);
qCDebug(lcAccessibilityTable) << "Updating table representation with"
<< table->rowCount() << table->columnCount();
rows = [self populateTableArray:rows role:NSAccessibilityRowRole count:table->rowCount()];
columns = [self populateTableArray:columns role:NSAccessibilityColumnRole count:table->columnCount()];
}
Expand Down
11 changes: 11 additions & 0 deletions src/widgets/widgets/qcombobox.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1096,6 +1096,17 @@ void QComboBoxPrivate::_q_rowsInserted(const QModelIndex &parent, int start, int
// set current index if combo was previously empty and there is no placeholderText
if (start == 0 && (end - start + 1) == q->count() && !currentIndex.isValid() &&
placeholderText.isEmpty()) {
#if QT_CONFIG(accessibility)
// This might have been called by the model emitting rowInserted(), at which
// point the view won't have updated the accessibility bridge yet about its new
// dimensions. Do it now so that the change of the selection matches the row
// indexes of the accessibility bridge's representation.
if (container && container->itemView()) {
QAccessibleTableModelChangeEvent event(container->itemView(),
QAccessibleTableModelChangeEvent::ModelReset);
QAccessible::updateAccessibility(&event);
}
#endif
q->setCurrentIndex(0);
// need to emit changed if model updated index "silently"
} else if (currentIndex.row() != indexBeforeChange) {
Expand Down

0 comments on commit 3236cc3

Please sign in to comment.