Accessibility: respect value in attached Accessible in controls

QQuickItemPrivate::accessibleRole is virtual and called by the framework
to determine the role of an item. The default implementation checks and
respects a possible Accessible attached object. However, subclasses that
override the virtual don't, so the attached properties are ignored, and
the class-specific implementation wins. This makes it impossible to
change the role of e.g. a checkable button.

To fix that, move the code respecting the attached object into a non-
virtual function that the framework calls instead, and only call the
virtual member if there is no attached object, or if that object is not
initialized with a role. Replace calls to the virtual from the
framework with calls to the non-virtual wrapper.

Do this for both QQuickItem and for QQuickPopup, and adjust the logic
in QQuickControl types that create an attached object and initialize
it's role when accessibility becomes active. Use the non-overridable
effective role value for that as well.

Add a test case, and to avoid any new framework calls to the virtual,
make it private.

Fixes: QTBUG-110114
Pick-to: 6.5 6.2
Change-Id: Ia709cecbd181b6d8ee3297a4af60c1e7db9a2c51
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Jan Arve Sæther <jan-arve.saether@qt.io>

src/quick/accessible/qaccessiblequickitem.cpp

@@ -456,7 +456,7 @@ QAccessible::Role QAccessibleQuickItem::role() const
 
     QAccessible::Role role = QAccessible::NoRole;
     if (item())
-        role = QQuickItemPrivate::get(item())->accessibleRole();
+        role = QQuickItemPrivate::get(item())->effectiveAccessibleRole();
     if (role == QAccessible::NoRole) {
         if (qobject_cast<QQuickText*>(const_cast<QQuickItem *>(item())))
             role = QAccessible::StaticText;

src/quick/items/qquickitem.cpp

@@ -2376,7 +2376,7 @@ bool QQuickItemPrivate::canAcceptTabFocus(QQuickItem *item)
         return true;
 
 #if QT_CONFIG(accessibility)
-    QAccessible::Role role = QQuickItemPrivate::get(item)->accessibleRole();
+    QAccessible::Role role = QQuickItemPrivate::get(item)->effectiveAccessibleRole();
     if (role == QAccessible::EditableText || role == QAccessible::Table || role == QAccessible::List) {
         return true;
     } else if (role == QAccessible::ComboBox || role == QAccessible::SpinBox) {
@@ -9731,13 +9731,20 @@ QQuickItemPrivate::ExtraData::ExtraData()
 
 
 #if QT_CONFIG(accessibility)
-QAccessible::Role QQuickItemPrivate::accessibleRole() const
+QAccessible::Role QQuickItemPrivate::effectiveAccessibleRole() const
 {
     Q_Q(const QQuickItem);
-    QQuickAccessibleAttached *accessibleAttached = qobject_cast<QQuickAccessibleAttached *>(qmlAttachedPropertiesObject<QQuickAccessibleAttached>(q, false));
-    if (accessibleAttached)
-        return accessibleAttached->role();
+    auto *attached = qmlAttachedPropertiesObject<QQuickAccessibleAttached>(q, false);
+    auto role = QAccessible::NoRole;
+    if (auto *accessibleAttached = qobject_cast<QQuickAccessibleAttached *>(attached))
+        role = accessibleAttached->role();
+    if (role == QAccessible::NoRole)
+        role = accessibleRole();
+    return role;
+}
 
+QAccessible::Role QQuickItemPrivate::accessibleRole() const
+{
     return QAccessible::NoRole;
 }
 #endif

src/quick/items/qquickitem_p.h

@@ -601,7 +601,10 @@ public:
     virtual void implicitHeightChanged();
 
 #if QT_CONFIG(accessibility)
+    QAccessible::Role effectiveAccessibleRole() const;
+private:
     virtual QAccessible::Role accessibleRole() const;
+public:
 #endif
 
     void setImplicitAntialiasing(bool antialiasing);

src/quicktemplates/qquickcontrol.cpp

@@ -2185,12 +2185,13 @@ QAccessible::Role QQuickControl::accessibleRole() const
 
 void QQuickControl::accessibilityActiveChanged(bool active)
 {
+    Q_D(QQuickControl);
     if (!active)
         return;
 
     QQuickAccessibleAttached *accessibleAttached = qobject_cast<QQuickAccessibleAttached *>(qmlAttachedPropertiesObject<QQuickAccessibleAttached>(this, true));
     Q_ASSERT(accessibleAttached);
-    accessibleAttached->setRole(accessibleRole());
+    accessibleAttached->setRole(d->effectiveAccessibleRole());
 }
 #endif
 

src/quicktemplates/qquicklabel.cpp

@@ -190,7 +190,7 @@ void QQuickLabelPrivate::accessibilityActiveChanged(bool active)
     Q_Q(QQuickLabel);
     QQuickAccessibleAttached *accessibleAttached = qobject_cast<QQuickAccessibleAttached *>(qmlAttachedPropertiesObject<QQuickAccessibleAttached>(q, true));
     Q_ASSERT(accessibleAttached);
-    accessibleAttached->setRole(accessibleRole());
+    accessibleAttached->setRole(effectiveAccessibleRole());
     maybeSetAccessibleName(text);
 }
 

src/quicktemplates/qquickpopup.cpp

@@ -14,6 +14,7 @@
 #include <QtCore/qloggingcategory.h>
 #include <QtQml/qqmlinfo.h>
 #include <QtQuick/qquickitem.h>
+#include <QtQuick/private/qquickaccessibleattached_p.h>
 #include <QtQuick/private/qquicktransition_p.h>
 #include <QtQuick/private/qquickitem_p.h>
 
@@ -2769,6 +2770,19 @@ QFont QQuickPopup::defaultFont() const
 }
 
 #if QT_CONFIG(accessibility)
+QAccessible::Role QQuickPopup::effectiveAccessibleRole() const
+{
+    auto *attached = qmlAttachedPropertiesObject<QQuickAccessibleAttached>(this, false);
+
+    auto role = QAccessible::NoRole;
+    if (auto *accessibleAttached = qobject_cast<QQuickAccessibleAttached *>(attached))
+        role = accessibleAttached->role();
+    if (role == QAccessible::NoRole)
+        role = accessibleRole();
+
+    return role;
+}
+
 QAccessible::Role QQuickPopup::accessibleRole() const
 {
     return QAccessible::Dialog;

src/quicktemplates/qquickpopup_p.h

@@ -419,7 +419,10 @@ protected:
     virtual QFont defaultFont() const;
 
 #if QT_CONFIG(accessibility)
+    QAccessible::Role effectiveAccessibleRole() const;
+private:
     virtual QAccessible::Role accessibleRole() const;
+protected:
     virtual void accessibilityActiveChanged(bool active);
 #endif
 

src/quicktemplates/qquickpopupitem.cpp

@@ -308,7 +308,7 @@ QFont QQuickPopupItem::defaultFont() const
 QAccessible::Role QQuickPopupItem::accessibleRole() const
 {
     Q_D(const QQuickPopupItem);
-    return d->popup->accessibleRole();
+    return d->popup->effectiveAccessibleRole();
 }
 
 void QQuickPopupItem::accessibilityActiveChanged(bool active)

src/quicktemplates/qquicktextarea.cpp

@@ -438,7 +438,7 @@ void QQuickTextAreaPrivate::accessibilityActiveChanged(bool active)
     Q_Q(QQuickTextArea);
     QQuickAccessibleAttached *accessibleAttached = qobject_cast<QQuickAccessibleAttached *>(qmlAttachedPropertiesObject<QQuickAccessibleAttached>(q, true));
     Q_ASSERT(accessibleAttached);
-    accessibleAttached->setRole(accessibleRole());
+    accessibleAttached->setRole(effectiveAccessibleRole());
     accessibleAttached->set_readOnly(q->isReadOnly());
     accessibleAttached->setDescription(placeholder);
 }

src/quicktemplates/qquicktextfield.cpp

@@ -288,7 +288,7 @@ void QQuickTextFieldPrivate::accessibilityActiveChanged(bool active)
     Q_Q(QQuickTextField);
     QQuickAccessibleAttached *accessibleAttached = qobject_cast<QQuickAccessibleAttached *>(qmlAttachedPropertiesObject<QQuickAccessibleAttached>(q, true));
     Q_ASSERT(accessibleAttached);
-    accessibleAttached->setRole(accessibleRole());
+    accessibleAttached->setRole(effectiveAccessibleRole());
     accessibleAttached->set_readOnly(m_readOnly);
     accessibleAttached->set_passwordEdit((m_echoMode == QQuickTextField::Password || m_echoMode == QQuickTextField::PasswordEchoOnEdit) ? true : false);
     accessibleAttached->setDescription(placeholder);

tests/auto/quick/qquickaccessible/tst_qquickaccessible.cpp

@@ -56,6 +56,7 @@ private slots:
     void commonTests();
 
     void quickAttachedProperties();
+    void attachedWins();
     void basicPropertiesTest();
     void hitTest();
     void checkableTest();
@@ -322,6 +323,31 @@ void tst_QQuickAccessible::quickAttachedProperties()
     QTestAccessibility::clearEvents();
 }
 
+// Verify that a role can be explicitly set, and that the values from the
+// attached object are used even if the item has a default role - QTBUG-110114
+void tst_QQuickAccessible::attachedWins()
+{
+    QQmlEngine engine;
+    QQmlComponent component(&engine);
+    component.setData(R"(
+        import QtQuick
+        import QtQuick.Controls
+        Button {
+            text: "Button"
+            objectName: "button"
+            Accessible.role: Accessible.RadioButton
+            Accessible.description: "Radio Button"
+        })", QUrl());
+    auto button = std::unique_ptr<QObject>(component.create());
+    QVERIFY(button);
+
+    QAccessibleInterface *iface = QAccessible::queryAccessibleInterface(button.get());
+    QVERIFY(iface);
+
+    QCOMPARE(iface->role(), QAccessible::RadioButton);
+    QTestAccessibility::clearEvents();
+}
+
 
 void tst_QQuickAccessible::basicPropertiesTest()
 {