From 089dd16fb6ea44d68201780e05cf08d0c5add580 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Tue, 30 May 2017 20:56:09 +0200 Subject: Accessibility: cleanup and fix the attached property handling Previously qmlAttachedPropertiesObject() refused to create attached properties object instances for objects that were not created in QML. QQuickControl & friends created a "fallback" instance of the accessible attached properties object, which made tst_accessible pass, but since QQuickAccessibleAttached was not aware of the instance, accessibility did not work for these controls that created the instance by hand. In qtdeclarative commit 4be38ab, qmlAttachedPropertiesObject() has been changed to allow creating attached properties for objects instantiated in C++. This allows us to remove the non-functional fallback instance. Change-Id: I8a86388dc4dee4721284d47aefc15940a75e6c8d Reviewed-by: Qt CI Bot Reviewed-by: Mitch Curtis --- src/quicktemplates2/qquickcontrol.cpp | 39 ++++++++------------------ src/quicktemplates2/qquickcontrol_p_p.h | 3 -- src/quicktemplates2/qquicklabel.cpp | 21 ++++++-------- src/quicktemplates2/qquicklabel_p_p.h | 3 -- src/quicktemplates2/qquicktextarea.cpp | 24 +++++++--------- src/quicktemplates2/qquicktextarea_p_p.h | 2 -- src/quicktemplates2/qquicktextfield.cpp | 30 +++++++++----------- src/quicktemplates2/qquicktextfield_p_p.h | 3 -- tests/auto/accessibility/tst_accessibility.cpp | 16 ++--------- 9 files changed, 46 insertions(+), 95 deletions(-) diff --git a/src/quicktemplates2/qquickcontrol.cpp b/src/quicktemplates2/qquickcontrol.cpp index ab1dd875..06f8c8a2 100644 --- a/src/quicktemplates2/qquickcontrol.cpp +++ b/src/quicktemplates2/qquickcontrol.cpp @@ -130,8 +130,7 @@ QQuickControlPrivate::QQuickControlPrivate() focusPolicy(Qt::NoFocus), focusReason(Qt::OtherFocusReason), background(nullptr), - contentItem(nullptr), - accessibleAttached(nullptr) + contentItem(nullptr) { #if QT_CONFIG(accessibility) QAccessible::installActivationObserver(this); @@ -1398,7 +1397,7 @@ void QQuickControl::componentComplete() setAcceptHoverEvents(QQuickControlPrivate::calcHoverEnabled(d->parentItem)); #endif #if QT_CONFIG(accessibility) - if (!d->accessibleAttached && QAccessible::isActive()) + if (QAccessible::isActive()) accessibilityActiveChanged(true); #endif } @@ -1610,29 +1609,20 @@ QAccessible::Role QQuickControl::accessibleRole() const void QQuickControl::accessibilityActiveChanged(bool active) { - Q_D(QQuickControl); - if (d->accessibleAttached || !active) + if (!active) return; - d->accessibleAttached = qobject_cast(qmlAttachedPropertiesObject(this, true)); - - // QQuickControl relies on the existence of a QQuickAccessibleAttached object. - // However, qmlAttachedPropertiesObject(create=true) creates an instance only - // for items that have been created by a QML engine. Therefore we create the - // object by hand for items created in C++ (QQuickPopupItem, for instance). - if (!d->accessibleAttached) - d->accessibleAttached = new QQuickAccessibleAttached(this); - - d->accessibleAttached->setRole(accessibleRole()); + QQuickAccessibleAttached *accessibleAttached = qobject_cast(qmlAttachedPropertiesObject(this, true)); + Q_ASSERT(accessibleAttached); + accessibleAttached->setRole(accessibleRole()); } #endif QString QQuickControl::accessibleName() const { #if QT_CONFIG(accessibility) - Q_D(const QQuickControl); - if (d->accessibleAttached) - return d->accessibleAttached->name(); + if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(this)) + return accessibleAttached->name(); #endif return QString(); } @@ -1640,9 +1630,8 @@ QString QQuickControl::accessibleName() const void QQuickControl::setAccessibleName(const QString &name) { #if QT_CONFIG(accessibility) - Q_D(QQuickControl); - if (d->accessibleAttached) - d->accessibleAttached->setName(name); + if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(this)) + accessibleAttached->setName(name); #else Q_UNUSED(name) #endif @@ -1651,9 +1640,7 @@ void QQuickControl::setAccessibleName(const QString &name) QVariant QQuickControl::accessibleProperty(const char *propertyName) { #if QT_CONFIG(accessibility) - Q_D(QQuickControl); - if (d->accessibleAttached) - return QQuickAccessibleAttached::property(this, propertyName); + return QQuickAccessibleAttached::property(this, propertyName); #endif Q_UNUSED(propertyName) return QVariant(); @@ -1662,9 +1649,7 @@ QVariant QQuickControl::accessibleProperty(const char *propertyName) bool QQuickControl::setAccessibleProperty(const char *propertyName, const QVariant &value) { #if QT_CONFIG(accessibility) - Q_D(QQuickControl); - if (d->accessibleAttached) - return QQuickAccessibleAttached::setProperty(this, propertyName, value); + return QQuickAccessibleAttached::setProperty(this, propertyName, value); #endif Q_UNUSED(propertyName) Q_UNUSED(value) diff --git a/src/quicktemplates2/qquickcontrol_p_p.h b/src/quicktemplates2/qquickcontrol_p_p.h index 65c4a7ec..54346016 100644 --- a/src/quicktemplates2/qquickcontrol_p_p.h +++ b/src/quicktemplates2/qquickcontrol_p_p.h @@ -60,8 +60,6 @@ QT_BEGIN_NAMESPACE -class QQuickAccessibleAttached; - class Q_QUICKTEMPLATES2_PRIVATE_EXPORT QQuickControlPrivate : public QQuickItemPrivate #if QT_CONFIG(accessibility) , public QAccessible::ActivationObserver @@ -169,7 +167,6 @@ public: Qt::FocusReason focusReason; QQuickItem *background; QQuickItem *contentItem; - QQuickAccessibleAttached *accessibleAttached; }; QT_END_NAMESPACE diff --git a/src/quicktemplates2/qquicklabel.cpp b/src/quicktemplates2/qquicklabel.cpp index d99a72a5..8d964745 100644 --- a/src/quicktemplates2/qquicklabel.cpp +++ b/src/quicktemplates2/qquicklabel.cpp @@ -79,8 +79,7 @@ QT_BEGIN_NAMESPACE */ QQuickLabelPrivate::QQuickLabelPrivate() - : background(nullptr), - accessibleAttached(nullptr) + : background(nullptr) { #if QT_CONFIG(accessibility) QAccessible::installActivationObserver(this); @@ -176,7 +175,8 @@ void QQuickLabelPrivate::updatePalette(const QPalette &palette) void QQuickLabelPrivate::textChanged(const QString &text) { #if QT_CONFIG(accessibility) - if (accessibleAttached) + Q_Q(QQuickLabel); + if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(q)) accessibleAttached->setName(text); #else Q_UNUSED(text) @@ -186,17 +186,14 @@ void QQuickLabelPrivate::textChanged(const QString &text) #if QT_CONFIG(accessibility) void QQuickLabelPrivate::accessibilityActiveChanged(bool active) { - if (accessibleAttached || !active) + if (!active) return; Q_Q(QQuickLabel); - accessibleAttached = qobject_cast(qmlAttachedPropertiesObject(q, true)); - if (accessibleAttached) { - accessibleAttached->setRole(accessibleRole()); - accessibleAttached->setName(text); - } else { - qWarning() << "QQuickLabel: " << q << " QQuickAccessibleAttached object creation failed!"; - } + QQuickAccessibleAttached *accessibleAttached = qobject_cast(qmlAttachedPropertiesObject(q, true)); + Q_ASSERT(accessibleAttached); + accessibleAttached->setRole(accessibleRole()); + accessibleAttached->setName(text); } QAccessible::Role QQuickLabelPrivate::accessibleRole() const @@ -308,7 +305,7 @@ void QQuickLabel::componentComplete() Q_D(QQuickLabel); QQuickText::componentComplete(); #if QT_CONFIG(accessibility) - if (!d->accessibleAttached && QAccessible::isActive()) + if (QAccessible::isActive()) d->accessibilityActiveChanged(true); #endif } diff --git a/src/quicktemplates2/qquicklabel_p_p.h b/src/quicktemplates2/qquicklabel_p_p.h index bc78d806..a8214d77 100644 --- a/src/quicktemplates2/qquicklabel_p_p.h +++ b/src/quicktemplates2/qquicklabel_p_p.h @@ -57,8 +57,6 @@ QT_BEGIN_NAMESPACE -class QQuickAccessibleAttached; - class QQuickLabelPrivate : public QQuickTextPrivate #if QT_CONFIG(accessibility) , public QAccessible::ActivationObserver @@ -108,7 +106,6 @@ public: QPalette resolvedPalette; QQuickItem *background; - QQuickAccessibleAttached *accessibleAttached; }; QT_END_NAMESPACE diff --git a/src/quicktemplates2/qquicktextarea.cpp b/src/quicktemplates2/qquicktextarea.cpp index 8a4fbb50..eec25848 100644 --- a/src/quicktemplates2/qquicktextarea.cpp +++ b/src/quicktemplates2/qquicktextarea.cpp @@ -138,7 +138,6 @@ QQuickTextAreaPrivate::QQuickTextAreaPrivate() #endif background(nullptr), focusReason(Qt::OtherFocusReason), - accessibleAttached(nullptr), flickable(nullptr) { #if QT_CONFIG(accessibility) @@ -410,7 +409,7 @@ void QQuickTextAreaPrivate::readOnlyChanged(bool isReadOnly) { Q_UNUSED(isReadOnly); #if QT_CONFIG(accessibility) - if (accessibleAttached) + if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(q_func())) accessibleAttached->set_readOnly(isReadOnly); #endif #if QT_CONFIG(cursor) @@ -421,18 +420,15 @@ void QQuickTextAreaPrivate::readOnlyChanged(bool isReadOnly) #if QT_CONFIG(accessibility) void QQuickTextAreaPrivate::accessibilityActiveChanged(bool active) { - if (accessibleAttached || !active) + if (!active) return; Q_Q(QQuickTextArea); - accessibleAttached = qobject_cast(qmlAttachedPropertiesObject(q, true)); - if (accessibleAttached) { - accessibleAttached->setRole(accessibleRole()); - accessibleAttached->set_readOnly(q->isReadOnly()); - accessibleAttached->setDescription(placeholder); - } else { - qWarning() << "QQuickTextArea: " << q << " QQuickAccessibleAttached object creation failed!"; - } + QQuickAccessibleAttached *accessibleAttached = qobject_cast(qmlAttachedPropertiesObject(q, true)); + Q_ASSERT(accessibleAttached); + accessibleAttached->setRole(accessibleRole()); + accessibleAttached->set_readOnly(q->isReadOnly()); + accessibleAttached->setDescription(placeholder); } QAccessible::Role QQuickTextAreaPrivate::accessibleRole() const @@ -532,8 +528,8 @@ void QQuickTextArea::setPlaceholderText(const QString &text) d->placeholder = text; #if QT_CONFIG(accessibility) - if (d->accessibleAttached) - d->accessibleAttached->setDescription(text); + if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(this)) + accessibleAttached->setDescription(text); #endif emit placeholderTextChanged(); } @@ -692,7 +688,7 @@ void QQuickTextArea::componentComplete() setAcceptHoverEvents(QQuickControlPrivate::calcHoverEnabled(d->parentItem)); #endif #if QT_CONFIG(accessibility) - if (!d->accessibleAttached && QAccessible::isActive()) + if (QAccessible::isActive()) d->accessibilityActiveChanged(true); #endif } diff --git a/src/quicktemplates2/qquicktextarea_p_p.h b/src/quicktemplates2/qquicktextarea_p_p.h index 82ea03cb..32f53c4f 100644 --- a/src/quicktemplates2/qquicktextarea_p_p.h +++ b/src/quicktemplates2/qquicktextarea_p_p.h @@ -62,7 +62,6 @@ QT_BEGIN_NAMESPACE class QQuickFlickable; -class QQuickAccessibleAttached; class QQuickTextAreaPrivate : public QQuickTextEditPrivate, public QQuickItemChangeListener #if QT_CONFIG(accessibility) @@ -141,7 +140,6 @@ public: QString placeholder; Qt::FocusReason focusReason; QQuickPressHandler pressHandler; - QQuickAccessibleAttached *accessibleAttached; QQuickFlickable *flickable; }; diff --git a/src/quicktemplates2/qquicktextfield.cpp b/src/quicktemplates2/qquicktextfield.cpp index 5197044d..1c350165 100644 --- a/src/quicktemplates2/qquicktextfield.cpp +++ b/src/quicktemplates2/qquicktextfield.cpp @@ -118,8 +118,7 @@ QQuickTextFieldPrivate::QQuickTextFieldPrivate() explicitHoverEnabled(false), #endif background(nullptr), - focusReason(Qt::OtherFocusReason), - accessibleAttached(nullptr) + focusReason(Qt::OtherFocusReason) { #if QT_CONFIG(accessibility) QAccessible::installActivationObserver(this); @@ -273,7 +272,7 @@ void QQuickTextFieldPrivate::readOnlyChanged(bool isReadOnly) { Q_UNUSED(isReadOnly); #if QT_CONFIG(accessibility) - if (accessibleAttached) + if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(q_func())) accessibleAttached->set_readOnly(isReadOnly); #endif #if QT_CONFIG(cursor) @@ -284,7 +283,7 @@ void QQuickTextFieldPrivate::readOnlyChanged(bool isReadOnly) void QQuickTextFieldPrivate::echoModeChanged(QQuickTextField::EchoMode echoMode) { #if QT_CONFIG(accessibility) - if (accessibleAttached) + if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(q_func())) accessibleAttached->set_passwordEdit((echoMode == QQuickTextField::Password || echoMode == QQuickTextField::PasswordEchoOnEdit) ? true : false); #else Q_UNUSED(echoMode) @@ -294,19 +293,16 @@ void QQuickTextFieldPrivate::echoModeChanged(QQuickTextField::EchoMode echoMode) #if QT_CONFIG(accessibility) void QQuickTextFieldPrivate::accessibilityActiveChanged(bool active) { - if (accessibleAttached || !active) + if (!active) return; Q_Q(QQuickTextField); - accessibleAttached = qobject_cast(qmlAttachedPropertiesObject(q, true)); - if (accessibleAttached) { - accessibleAttached->setRole(accessibleRole()); - accessibleAttached->set_readOnly(m_readOnly); - accessibleAttached->set_passwordEdit((m_echoMode == QQuickTextField::Password || m_echoMode == QQuickTextField::PasswordEchoOnEdit) ? true : false); - accessibleAttached->setDescription(placeholder); - } else { - qWarning() << "QQuickTextField: " << q << " QQuickAccessibleAttached object creation failed!"; - } + QQuickAccessibleAttached *accessibleAttached = qobject_cast(qmlAttachedPropertiesObject(q, true)); + Q_ASSERT(accessibleAttached); + accessibleAttached->setRole(accessibleRole()); + accessibleAttached->set_readOnly(m_readOnly); + accessibleAttached->set_passwordEdit((m_echoMode == QQuickTextField::Password || m_echoMode == QQuickTextField::PasswordEchoOnEdit) ? true : false); + accessibleAttached->setDescription(placeholder); } QAccessible::Role QQuickTextFieldPrivate::accessibleRole() const @@ -401,8 +397,8 @@ void QQuickTextField::setPlaceholderText(const QString &text) d->placeholder = text; #if QT_CONFIG(accessibility) - if (d->accessibleAttached) - d->accessibleAttached->setDescription(text); + if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(this)) + accessibleAttached->setDescription(text); #endif emit placeholderTextChanged(); } @@ -553,7 +549,7 @@ void QQuickTextField::componentComplete() setAcceptHoverEvents(QQuickControlPrivate::calcHoverEnabled(d->parentItem)); #endif #if QT_CONFIG(accessibility) - if (!d->accessibleAttached && QAccessible::isActive()) + if (QAccessible::isActive()) d->accessibilityActiveChanged(true); #endif } diff --git a/src/quicktemplates2/qquicktextfield_p_p.h b/src/quicktemplates2/qquicktextfield_p_p.h index d726419b..65767452 100644 --- a/src/quicktemplates2/qquicktextfield_p_p.h +++ b/src/quicktemplates2/qquicktextfield_p_p.h @@ -60,8 +60,6 @@ QT_BEGIN_NAMESPACE -class QQuickAccessibleAttached; - class QQuickTextFieldPrivate : public QQuickTextInputPrivate #if QT_CONFIG(accessibility) , public QAccessible::ActivationObserver @@ -130,7 +128,6 @@ public: QString placeholder; Qt::FocusReason focusReason; QQuickPressHandler pressHandler; - QQuickAccessibleAttached *accessibleAttached; }; QT_END_NAMESPACE diff --git a/tests/auto/accessibility/tst_accessibility.cpp b/tests/auto/accessibility/tst_accessibility.cpp index 4208a366..614566e6 100644 --- a/tests/auto/accessibility/tst_accessibility.cpp +++ b/tests/auto/accessibility/tst_accessibility.cpp @@ -113,16 +113,6 @@ void tst_accessibility::a11y_data() QTest::newRow("WeekNumberColumn") << "weeknumbercolumn" << 0x0 << "WeekNumberColumn"; //QAccessible::NoRole } -#if QT_CONFIG(accessibility) -static QQuickAccessibleAttached *accessibleAttached(QQuickItem *item) -{ - QQuickAccessibleAttached *acc = qobject_cast(qmlAttachedPropertiesObject(item, false)); - if (!acc) - acc = item->findChild(); - return acc; -} -#endif - void tst_accessibility::a11y() { QFETCH(QString, name); @@ -152,7 +142,7 @@ void tst_accessibility::a11y() QVERIFY(item); #if QT_CONFIG(accessibility) - QQuickAccessibleAttached *acc = accessibleAttached(item); + QQuickAccessibleAttached *acc = QQuickAccessibleAttached::attachedProperties(item); if (name != QLatin1Literal("dayofweekrow") && name != QLatin1Literal("monthgrid") && name != QLatin1Literal("weeknumbercolumn")) { @@ -161,7 +151,7 @@ void tst_accessibility::a11y() } else { QVERIFY(!acc); QAccessible::setActive(true); - acc = accessibleAttached(item); + acc = QQuickAccessibleAttached::attachedProperties(item); } } QVERIFY(acc); @@ -170,8 +160,6 @@ void tst_accessibility::a11y() #else Q_UNUSED(role) Q_UNUSED(text) - QObject *acc = qmlAttachedPropertiesObject(item, false); - QVERIFY(!acc); #endif } -- cgit v1.2.3