From ffdf7eceed26c99c81accaf1529024bd0bf40f44 Mon Sep 17 00:00:00 2001 From: Peter Varga Date: Tue, 12 Nov 2019 10:57:12 +0100 Subject: Fix widget accessibility on macOS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit macOS Accessibility queries the window for the focused accessibility element. The window forwards the query to the widget with active focus. This widget is the RWHVQtDelegateWidget if a web element is focused in QWebEngineView. Therefore, a QAccessibleWidget interface has been implemented for the RWHVQtDelegateWidget to forward the request to the QWebEngineView. The focused accessibility element expected to be returned by the QAccessibleInterface::focusChild() method. In case of the macOS accessibility backend, it is called by the accessibilityFocusedUIElement() NSAccessibility API function. It expects the focused web accessibility element otherwise VoiceOver won't focus properly. The focused web accessiblity element is looked up by the new BrowserAccessibilityQt::focusChild() method. RenderWidgetHostviewQtDelegateWidget::focusChild() and QWebengineViewAccessible::focusChild() methods have been also implemented to forward it. This patch depends on a focusChild() fix in qtbase: a132e02540 Fix QAccessibleWidget::focusChild() to return focused descendant Microsoft Narrator also uses focusChild() to query the current focused element when it starts but it is still functional without this fix. Task-number: QTBUG-78284 Task-number: QTBUG-81539 Change-Id: I3c4861e58622ccbb5046c60c4efcc19842400a88 Reviewed-by: Michael Brüning --- src/core/browser_accessibility_qt.cpp | 13 ++++ src/core/browser_accessibility_qt.h | 1 + src/webenginewidgets/api/qwebengineview.cpp | 22 ++++-- src/webenginewidgets/api/qwebengineview_p.h | 1 + .../render_widget_host_view_qt_delegate_widget.cpp | 28 ++++++++ .../render_widget_host_view_qt_delegate_widget.h | 17 +++++ .../widgets/accessibility/tst_accessibility.cpp | 80 ++++++++++++++++++++++ 7 files changed, 158 insertions(+), 4 deletions(-) diff --git a/src/core/browser_accessibility_qt.cpp b/src/core/browser_accessibility_qt.cpp index db6f371d3..816a46041 100644 --- a/src/core/browser_accessibility_qt.cpp +++ b/src/core/browser_accessibility_qt.cpp @@ -148,6 +148,19 @@ QAccessibleInterface *BrowserAccessibilityQt::child(int index) const return static_cast(BrowserAccessibility::PlatformGetChild(index)); } +QAccessibleInterface *BrowserAccessibilityQt::focusChild() const +{ + if (state().focused) + return const_cast(this); + + for (int i = 0; i < childCount(); ++i) { + if (QAccessibleInterface *iface = child(i)->focusChild()) + return iface; + } + + return nullptr; +} + int BrowserAccessibilityQt::childCount() const { return PlatformChildCount(); diff --git a/src/core/browser_accessibility_qt.h b/src/core/browser_accessibility_qt.h index decfc1e9d..4acac6aa7 100644 --- a/src/core/browser_accessibility_qt.h +++ b/src/core/browser_accessibility_qt.h @@ -68,6 +68,7 @@ public: // navigation, hierarchy QAccessibleInterface *parent() const override; QAccessibleInterface *child(int index) const override; + QAccessibleInterface *focusChild() const override; int childCount() const override; int indexOfChild(const QAccessibleInterface *) const override; diff --git a/src/webenginewidgets/api/qwebengineview.cpp b/src/webenginewidgets/api/qwebengineview.cpp index de81448a9..a51f9b7a5 100644 --- a/src/webenginewidgets/api/qwebengineview.cpp +++ b/src/webenginewidgets/api/qwebengineview.cpp @@ -107,9 +107,18 @@ void QWebEngineViewPrivate::widgetChanged(QtWebEngineCore::RenderWidgetHostViewQ if (oldWidget) { q->layout()->removeWidget(oldWidget); oldWidget->hide(); +#if QT_CONFIG(accessibility) + QAccessible::deleteAccessibleInterface(QAccessible::uniqueId(QAccessible::queryAccessibleInterface(oldWidget))); +#endif } if (newWidget) { +#if QT_CONFIG(accessibility) + // An earlier QAccessible::queryAccessibleInterface() call may have already registered a default + // QAccessibleInterface for newWidget: remove it first to avoid assert in QAccessibleCache::insert(). + QAccessible::deleteAccessibleInterface(QAccessible::uniqueId(QAccessible::queryAccessibleInterface(newWidget))); + QAccessible::registerAccessibleInterface(new QtWebEngineCore::RenderWidgetHostViewQtDelegateWidgetAccessible(newWidget, q)); +#endif q->layout()->addWidget(newWidget); q->setFocusProxy(newWidget); newWidget->show(); @@ -462,11 +471,16 @@ void QWebEngineView::dropEvent(QDropEvent *e) #endif // QT_CONFIG(draganddrop) #ifndef QT_NO_ACCESSIBILITY +QAccessibleInterface *QWebEngineViewAccessible::focusChild() const +{ + if (child(0) && child(0)->focusChild()) + return child(0)->focusChild(); + return const_cast(this); +} + int QWebEngineViewAccessible::childCount() const { - if (view() && child(0)) - return 1; - return 0; + return child(0) ? 1 : 0; } QAccessibleInterface *QWebEngineViewAccessible::child(int index) const @@ -478,7 +492,7 @@ QAccessibleInterface *QWebEngineViewAccessible::child(int index) const int QWebEngineViewAccessible::indexOfChild(const QAccessibleInterface *c) const { - if (c == child(0)) + if (child(0) && c == child(0)) return 0; return -1; } diff --git a/src/webenginewidgets/api/qwebengineview_p.h b/src/webenginewidgets/api/qwebengineview_p.h index 7848e0cf3..dd0a5bedf 100644 --- a/src/webenginewidgets/api/qwebengineview_p.h +++ b/src/webenginewidgets/api/qwebengineview_p.h @@ -87,6 +87,7 @@ public: QWebEngineViewAccessible(QWebEngineView *o) : QAccessibleWidget(o) {} + QAccessibleInterface *focusChild() const override; int childCount() const override; QAccessibleInterface *child(int index) const override; int indexOfChild(const QAccessibleInterface *child) const override; diff --git a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp index 894dca4fa..8ba312822 100644 --- a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp +++ b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp @@ -483,4 +483,32 @@ void RenderWidgetHostViewQtDelegateWidget::onWindowPosChanged() m_client->visualPropertiesChanged(); } +#if QT_CONFIG(accessibility) +RenderWidgetHostViewQtDelegateWidgetAccessible::RenderWidgetHostViewQtDelegateWidgetAccessible(RenderWidgetHostViewQtDelegateWidget *o, QWebEngineView *view) + : QAccessibleWidget(o) + , m_view(view) +{ +} + +QAccessibleInterface *RenderWidgetHostViewQtDelegateWidgetAccessible::focusChild() const +{ + return QAccessible::queryAccessibleInterface(m_view)->focusChild(); +} + +int RenderWidgetHostViewQtDelegateWidgetAccessible::childCount() const +{ + return QAccessible::queryAccessibleInterface(m_view)->childCount(); +} + +QAccessibleInterface *RenderWidgetHostViewQtDelegateWidgetAccessible::child(int index) const +{ + return QAccessible::queryAccessibleInterface(m_view)->child(index); +} + +int RenderWidgetHostViewQtDelegateWidgetAccessible::indexOfChild(const QAccessibleInterface *c) const +{ + return QAccessible::queryAccessibleInterface(m_view)->indexOfChild(c); +} +#endif // QT_CONFIG(accessibility) + } // namespace QtWebEngineCore diff --git a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h index 18f848da5..df1806b6f 100644 --- a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h +++ b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h @@ -43,11 +43,13 @@ #include "render_widget_host_view_qt_delegate.h" #include "web_contents_adapter_client.h" +#include #include #include QT_BEGIN_NAMESPACE class QWebEnginePage; +class QWebEngineView; class QWebEnginePagePrivate; QT_END_NAMESPACE @@ -115,6 +117,21 @@ private: QMetaObject::Connection m_parentDestroyedConnection; }; +#if QT_CONFIG(accessibility) +class RenderWidgetHostViewQtDelegateWidgetAccessible : public QAccessibleWidget +{ +public: + RenderWidgetHostViewQtDelegateWidgetAccessible(RenderWidgetHostViewQtDelegateWidget *o, QWebEngineView *view); + + QAccessibleInterface *focusChild() const override; + int childCount() const override; + QAccessibleInterface *child(int index) const override; + int indexOfChild(const QAccessibleInterface *child) const override; +private: + QWebEngineView *m_view; +}; +#endif // QT_CONFIG(accessibility) + } // namespace QtWebEngineCore #endif diff --git a/tests/auto/widgets/accessibility/tst_accessibility.cpp b/tests/auto/widgets/accessibility/tst_accessibility.cpp index b5def4cd7..117a9e573 100644 --- a/tests/auto/widgets/accessibility/tst_accessibility.cpp +++ b/tests/auto/widgets/accessibility/tst_accessibility.cpp @@ -20,9 +20,13 @@ #include #include "../util.h" +#include +#include + #include #include #include +#include #include class tst_Accessibility : public QObject @@ -38,6 +42,8 @@ public Q_SLOTS: private Q_SLOTS: void noPage(); void hierarchy(); + void focusChild(); + void focusChild_data(); void text(); void value(); void roles_data(); @@ -142,6 +148,80 @@ void tst_Accessibility::hierarchy() QCOMPARE(input, child); } +void tst_Accessibility::focusChild_data() +{ + QTest::addColumn("interfaceName"); + QTest::addColumn>("ancestorRoles"); + + QTest::newRow("QWebEngineView") << QString("QWebEngineView") << QVector({QAccessible::Client}); + QTest::newRow("RenderWidgetHostViewQtDelegate") << QString("RenderWidgetHostViewQtDelegate") << QVector({QAccessible::Client}); + QTest::newRow("QMainWindow") << QString("QMainWindow") << QVector({QAccessible::Window, QAccessible::Client /* central widget */, QAccessible::Client /* view */}); +} + +void tst_Accessibility::focusChild() +{ + auto traverseToWebDocumentAccessibleInterface = [](QAccessibleInterface *iface) -> QAccessibleInterface * { + QFETCH(QVector, ancestorRoles); + for (int i = 0; i < ancestorRoles.size(); ++i) { + if (iface->childCount() == 0 || iface->role() != ancestorRoles[i]) + return nullptr; + iface = iface->child(0); + } + + if (iface->role() != QAccessible::WebDocument) + return nullptr; + + return iface; + }; + + QMainWindow mainWindow; + QWebEngineView *webView = new QWebEngineView; + QWidget *centralWidget = new QWidget; + QHBoxLayout *centralLayout = new QHBoxLayout; + centralWidget->setLayout(centralLayout); + mainWindow.setCentralWidget(centralWidget); + centralLayout->addWidget(webView); + + mainWindow.show(); + QVERIFY(QTest::qWaitForWindowExposed(&mainWindow)); + + webView->settings()->setAttribute(QWebEngineSettings::FocusOnNavigationEnabled, true); + webView->setHtml("" \ + "" \ + ""); + webView->show(); + QSignalSpy spyFinished(webView, &QWebEngineView::loadFinished); + QVERIFY(spyFinished.wait()); + + QVERIFY(webView->focusWidget()); + QAccessibleInterface *iface = nullptr; + QFETCH(QString, interfaceName); + if (interfaceName == "QWebEngineView") + iface = QAccessible::queryAccessibleInterface(webView); + else if (interfaceName == "RenderWidgetHostViewQtDelegate") + iface = QAccessible::queryAccessibleInterface(webView->focusWidget()); + else if (interfaceName == "QMainWindow") + iface = QAccessible::queryAccessibleInterface(&mainWindow); + QVERIFY(iface); + + // Make sure the input field does not have the focus. + evaluateJavaScriptSync(webView->page(), "document.getElementById('input1').blur()"); + QTRY_VERIFY(evaluateJavaScriptSync(webView->page(), "document.activeElement.id").toString().isEmpty()); + + QVERIFY(iface->focusChild()); + QTRY_COMPARE(iface->focusChild()->role(), QAccessible::WebDocument); + QCOMPARE(traverseToWebDocumentAccessibleInterface(iface), iface->focusChild()); + + // Set active focus on the input field. + evaluateJavaScriptSync(webView->page(), "document.getElementById('input1').focus()"); + QTRY_COMPARE(evaluateJavaScriptSync(webView->page(), "document.activeElement.id").toString(), QStringLiteral("input1")); + + QVERIFY(iface->focusChild()); + QTRY_COMPARE(iface->focusChild()->role(), QAccessible::EditableText); + // -> -> + QCOMPARE(traverseToWebDocumentAccessibleInterface(iface)->child(0)->child(0), iface->focusChild()); +} + void tst_Accessibility::text() { QWebEngineView webView; -- cgit v1.2.3