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 <qt_ci_bot@qt-project.org>
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

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<QQuickAccessibleAttached *>(qmlAttachedPropertiesObject<QQuickAccessibleAttached>(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<QQuickAccessibleAttached *>(qmlAttachedPropertiesObject<QQuickAccessibleAttached>(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<QQuickAccessibleAttached *>(qmlAttachedPropertiesObject<QQuickAccessibleAttached>(q, true));
-    if (accessibleAttached) {
-        accessibleAttached->setRole(accessibleRole());
-        accessibleAttached->setName(text);
-    } else {
-        qWarning() << "QQuickLabel: " << q << " QQuickAccessibleAttached object creation failed!";
-    }
+    QQuickAccessibleAttached *accessibleAttached = qobject_cast<QQuickAccessibleAttached *>(qmlAttachedPropertiesObject<QQuickAccessibleAttached>(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<QQuickAccessibleAttached *>(qmlAttachedPropertiesObject<QQuickAccessibleAttached>(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<QQuickAccessibleAttached *>(qmlAttachedPropertiesObject<QQuickAccessibleAttached>(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<QQuickAccessibleAttached *>(qmlAttachedPropertiesObject<QQuickAccessibleAttached>(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<QQuickAccessibleAttached *>(qmlAttachedPropertiesObject<QQuickAccessibleAttached>(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<QQuickAccessibleAttached *>(qmlAttachedPropertiesObject<QQuickAccessibleAttached>(item, false));
-    if (!acc)
-        acc = item->findChild<QQuickAccessibleAttached *>();
-    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<QObject>(item, false);
-    QVERIFY(!acc);
 #endif
 }