vcl/Library_vclplug_qt5.mk           |    1 
 vcl/Library_vclplug_qt6.mk           |    1 
 vcl/inc/qt5/QtAccessibleRegistry.hxx |   41 +++++++++++++++++++++++++++++++
 vcl/inc/qt6/QtAccessibleRegistry.hxx |   12 +++++++++
 vcl/qt5/QtAccessibleRegistry.cxx     |   45 +++++++++++++++++++++++++++++++++++
 vcl/qt5/QtAccessibleWidget.cxx       |   43 ++++++++++++++++++++++-----------
 vcl/qt6/QtAccessibleRegistry.cxx     |   12 +++++++++
 7 files changed, 141 insertions(+), 14 deletions(-)

New commits:
commit 61c0c1286dbd9015809ba8ee5ee687b612438bef
Author:     Michael Weghorn <m.wegh...@posteo.de>
AuthorDate: Wed Aug 24 13:47:25 2022 +0200
Commit:     Michael Weghorn <m.wegh...@posteo.de>
CommitDate: Wed Aug 24 19:05:54 2022 +0200

    qt a11y: Remember associated QObject also for non-QtXAcccessible case
    
    This is similar to what
    Change-Id Ic890a387ff016e889f25dba70c82d0d81ae7a9e3
    ("qt a11y: Remember and reuse existing QObject for XAccessible")
    does to avoid creating another `QtXAccessible` for an `XAccessible`
    if one has been created earlier.
    
    However, there is an additional case that needs to be
    covered to avoid creating multiple `QObjects` for
    a single `XAccessible`:
    
    `QtAccessibleWidget::customFactory` not only gets called
    by explicitly calling `QAccessible::queryAccessibleInterface`
    from within LibreOffice code, but the Qt library itself
    also calls this method, in which case no entry associating
    the `XAccessible` with its `QObject` had been inserted into
    the map handled by our `QtAccessibleRegistry` previously.
    
    This would result in a "new" `QtXAccessible` object
    being created later when a `QObject` would be needed
    for that `XAccessible`, rather than using the
    `QtWidget` that is the actual `QObject` associated
    with the object.
    
    Prevent that from happening by inserting an entry
    into the map for this case as well.
    
    With this and two Accerciser fixes [1] [2] in place, jumping
    to bookmarks in Accerciser's treeview of the LO a11y
    hierarchy now generally works with the qt6 VCL plugin.
    
    It previously failed due to the fact that a new object
    was created and navigating the tree up to the root application
    object from the bookmarked object would then fail.
    
    The fact that there were two objects could be seen e.g. by
    using the following commands in Accerciser's IPython console
    with the root "soffice.bin" application a11y object
    selected in the treeview after starting Calc:
    
        In [25]: acc[1][0][0].get_parent().path
        Out[25]: '/org/a11y/atspi/accessible/2147483672'
        In [26]: acc[1][0].path
        Out[26]: '/org/a11y/atspi/accessible/2147483648'
    
    -> Two different IDs/paths for what should be the same object.
    (The parent of the first child of the object at tree path 1,0
    should be the object itself, but here it wasn't.)
    
    With this change in place, this now works as expected:
    
        In [28]: acc[1][0][0].get_parent().path
        Out[28]: '/org/a11y/atspi/accessible/2147483648'
        In [29]: acc[1][0].path
        Out[29]: '/org/a11y/atspi/accessible/2147483648'
    
    Together with
    Change-Id Ic890a387ff016e889f25dba70c82d0d81ae7a9e3
    ("qt a11y: Remember and reuse existing QObject for XAccessible"),
    this also addresses the remaining issue mentioned in
    
        commit 99640693d28ca11b31a1d3855e104d2d8c5122d7
        Author: Michael Weghorn <m.wegh...@posteo.de>
        Date:   Wed Aug 3 16:49:48 2022 +0200
    
    > Note however that this change alone is not yet sufficient
    > for a window to actually be returned for any arbitrary a11y
    > object deeper down the hierarchy. This is because
    > walking up the a11y hierarchy currently results in new
    > Qt a11y objects being created for the parents instead of
    > using existing ones, and the newly created ones lack
    > the association to the widgets.
    > (This works in a WIP branch that remembers/caches
    > a11y objects, but that needs some additional work before
    > it can be merged.)
    
    Note that there are still cases where navigation
    to bookmarks in Accerciser's tree view doesn't work
    (reliably), but those would need to be looked at
    separately and might not be specific to the qt6
    VCL plugin. (At least I have come across such
    cases with gtk3 as well.)
    
    [1] 
https://gitlab.gnome.org/GNOME/accerciser/-/commit/c2a3e9f1eb1fcd6eb059f1f2fe6e629b86521335
    [2] 
https://gitlab.gnome.org/GNOME/accerciser/-/commit/a092dc933985fafd5b1e2cc3374c7bbc0fb2d12e
    
    Change-Id: I02262014a45a4b024cdc1bbd385da8a35a2c304a
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/138764
    Tested-by: Jenkins
    Reviewed-by: Michael Weghorn <m.wegh...@posteo.de>

diff --git a/vcl/inc/qt5/QtAccessibleRegistry.hxx 
b/vcl/inc/qt5/QtAccessibleRegistry.hxx
index 00acf80fd68c..87781752f704 100644
--- a/vcl/inc/qt5/QtAccessibleRegistry.hxx
+++ b/vcl/inc/qt5/QtAccessibleRegistry.hxx
@@ -33,6 +33,7 @@ private:
 public:
     /** Returns the related QObject* for the XAccessible. Creates a new one if 
none exists yet. */
     static QObject* getQObject(css::uno::Reference<XAccessible> xAcc);
+    static void insert(css::uno::Reference<XAccessible> xAcc, QObject* 
pQObject);
     /** Removes the entry for the given XAccessible. */
     static void remove(css::uno::Reference<XAccessible> xAcc);
 };
diff --git a/vcl/qt5/QtAccessibleRegistry.cxx b/vcl/qt5/QtAccessibleRegistry.cxx
index e64f8ae03868..88f9abcfd17e 100644
--- a/vcl/qt5/QtAccessibleRegistry.cxx
+++ b/vcl/qt5/QtAccessibleRegistry.cxx
@@ -10,6 +10,8 @@
 #include <QtAccessibleRegistry.hxx>
 #include <QtXAccessible.hxx>
 
+#include <cassert>
+
 std::map<XAccessible*, QObject*> QtAccessibleRegistry::m_aMapping = {};
 
 QObject* QtAccessibleRegistry::getQObject(css::uno::Reference<XAccessible> 
xAcc)
@@ -28,6 +30,12 @@ QObject* 
QtAccessibleRegistry::getQObject(css::uno::Reference<XAccessible> xAcc)
     return pQtAcc;
 }
 
+void QtAccessibleRegistry::insert(css::uno::Reference<XAccessible> xAcc, 
QObject* pQObject)
+{
+    assert(pQObject);
+    m_aMapping.emplace(xAcc.get(), pQObject);
+}
+
 void QtAccessibleRegistry::remove(css::uno::Reference<XAccessible> xAcc)
 {
     assert(xAcc.is());
diff --git a/vcl/qt5/QtAccessibleWidget.cxx b/vcl/qt5/QtAccessibleWidget.cxx
index fc81332465e7..e2e99e6de208 100644
--- a/vcl/qt5/QtAccessibleWidget.cxx
+++ b/vcl/qt5/QtAccessibleWidget.cxx
@@ -755,7 +755,13 @@ QAccessibleInterface* 
QtAccessibleWidget::customFactory(const QString& classname
         vcl::Window* pWindow = pWidget->frame().GetWindow();
 
         if (pWindow)
-            return new QtAccessibleWidget(pWindow->GetAccessible(), object);
+        {
+            css::uno::Reference<XAccessible> xAcc = pWindow->GetAccessible();
+            // insert into registry so the association between the XAccessible 
and the QtWidget
+            // is remembered rather than creating a different QtXAccessible 
when a QObject is needed later
+            QtAccessibleRegistry::insert(xAcc, object);
+            return new QtAccessibleWidget(xAcc, object);
+        }
     }
     if (classname == QLatin1String("QtXAccessible") && object)
     {
commit 812fe185fba48b439fb1229517d62aa67c209016
Author:     Michael Weghorn <m.wegh...@posteo.de>
AuthorDate: Wed Aug 24 11:42:04 2022 +0200
Commit:     Michael Weghorn <m.wegh...@posteo.de>
CommitDate: Wed Aug 24 19:05:35 2022 +0200

    qt a11y: Remember and reuse existing QObject for XAccessible
    
    Previously, a new `QtXAccessible` object was created
    for an `XAccessible` each time before
    `QAccessible::queryAccessibleInterface` was called,
    which is not only unnecessary but also causes
    various issues, e.g. it breaks walking the a11y hierarchy
    upwards (i.e. from children to parents), since a new
    object is created for the parent.
    
    This introduces `QtAccessibleRegistry` that keeps
    a mapping between the `XAccessible` and
    the associated `QObject`. That mapping is used
    to reuse already created objects instead of creating
    new ones for the same `XAccessible`.
    
    The entry for an `XAccessible` is removed again from the
    map in `QtAccessibleWidget::invalidate`, which gets called
    when the `XAccessible` gets disposed,
    s. `QtAccessibleEventListener::disposing`.
    
    With this in place, Orca now also nicely announces
    only the text of the push buttons themselves in the "Save Document?"
    dialog when switching between the buttons using the
    Tab key, rather than announcing the whole widget hierarchy
    every time (probably because creating a new object every time
    prevented Orca from recognizing that the previously selected
    pushbutton and the newly selected one are siblings, i.e.
    have the same parent object.)
    
    Change-Id: Ic890a387ff016e889f25dba70c82d0d81ae7a9e3
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/138757
    Tested-by: Jenkins
    Reviewed-by: Michael Weghorn <m.wegh...@posteo.de>

diff --git a/vcl/Library_vclplug_qt5.mk b/vcl/Library_vclplug_qt5.mk
index 25a8864c83ad..2a72693e0e85 100644
--- a/vcl/Library_vclplug_qt5.mk
+++ b/vcl/Library_vclplug_qt5.mk
@@ -76,6 +76,7 @@ endif
 
 $(eval $(call gb_Library_add_exception_objects,vclplug_qt5,\
     vcl/qt5/QtAccessibleEventListener \
+    vcl/qt5/QtAccessibleRegistry \
     vcl/qt5/QtAccessibleWidget \
     vcl/qt5/QtBitmap \
     vcl/qt5/QtClipboard \
diff --git a/vcl/Library_vclplug_qt6.mk b/vcl/Library_vclplug_qt6.mk
index 36da06abb294..a6b14da23f5c 100644
--- a/vcl/Library_vclplug_qt6.mk
+++ b/vcl/Library_vclplug_qt6.mk
@@ -75,6 +75,7 @@ endif
 
 $(eval $(call gb_Library_add_exception_objects,vclplug_qt6,\
     vcl/qt6/QtAccessibleEventListener \
+    vcl/qt6/QtAccessibleRegistry \
     vcl/qt6/QtAccessibleWidget \
     vcl/qt6/QtBitmap \
     vcl/qt6/QtClipboard \
diff --git a/vcl/inc/qt5/QtAccessibleRegistry.hxx 
b/vcl/inc/qt5/QtAccessibleRegistry.hxx
new file mode 100644
index 000000000000..00acf80fd68c
--- /dev/null
+++ b/vcl/inc/qt5/QtAccessibleRegistry.hxx
@@ -0,0 +1,40 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#pragma once
+
+#include <map>
+
+#include <QtCore/QObject>
+
+#include <com/sun/star/accessibility/XAccessible.hpp>
+
+using namespace css::accessibility;
+
+/**
+ * Maintains a mapping between XAccessible objects and the
+ * associated QObjects. The corresponding QObject can be
+ * passed to the QAccessible::queryAccessibleInterface method in
+ * order to retrieve the QAccessibleInterface for the
+ * XAccessible object.
+ */
+class QtAccessibleRegistry
+{
+private:
+    static std::map<css::accessibility::XAccessible*, QObject*> m_aMapping;
+    QtAccessibleRegistry() = delete;
+
+public:
+    /** Returns the related QObject* for the XAccessible. Creates a new one if 
none exists yet. */
+    static QObject* getQObject(css::uno::Reference<XAccessible> xAcc);
+    /** Removes the entry for the given XAccessible. */
+    static void remove(css::uno::Reference<XAccessible> xAcc);
+};
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/inc/qt6/QtAccessibleRegistry.hxx 
b/vcl/inc/qt6/QtAccessibleRegistry.hxx
new file mode 100644
index 000000000000..b29fbb32633e
--- /dev/null
+++ b/vcl/inc/qt6/QtAccessibleRegistry.hxx
@@ -0,0 +1,12 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include "../qt5/QtAccessibleRegistry.hxx"
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/qt5/QtAccessibleRegistry.cxx b/vcl/qt5/QtAccessibleRegistry.cxx
new file mode 100644
index 000000000000..e64f8ae03868
--- /dev/null
+++ b/vcl/qt5/QtAccessibleRegistry.cxx
@@ -0,0 +1,37 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <QtAccessibleRegistry.hxx>
+#include <QtXAccessible.hxx>
+
+std::map<XAccessible*, QObject*> QtAccessibleRegistry::m_aMapping = {};
+
+QObject* QtAccessibleRegistry::getQObject(css::uno::Reference<XAccessible> 
xAcc)
+{
+    if (!xAcc.is())
+        return nullptr;
+
+    // look for existing entry in the map
+    auto entry = m_aMapping.find(xAcc.get());
+    if (entry != m_aMapping.end())
+        return entry->second;
+
+    // create a new object and remember it in the map
+    QtXAccessible* pQtAcc = new QtXAccessible(xAcc);
+    m_aMapping.emplace(xAcc.get(), pQtAcc);
+    return pQtAcc;
+}
+
+void QtAccessibleRegistry::remove(css::uno::Reference<XAccessible> xAcc)
+{
+    assert(xAcc.is());
+    m_aMapping.erase(xAcc.get());
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/qt5/QtAccessibleWidget.cxx b/vcl/qt5/QtAccessibleWidget.cxx
index b23a8c9a2844..fc81332465e7 100644
--- a/vcl/qt5/QtAccessibleWidget.cxx
+++ b/vcl/qt5/QtAccessibleWidget.cxx
@@ -22,6 +22,7 @@
 #include <QtGui/QAccessibleInterface>
 
 #include <QtAccessibleEventListener.hxx>
+#include <QtAccessibleRegistry.hxx>
 #include <QtFrame.hxx>
 #include <QtTools.hxx>
 #include <QtWidget.hxx>
@@ -74,7 +75,11 @@ QtAccessibleWidget::QtAccessibleWidget(const 
Reference<XAccessible> xAccessible,
     }
 }
 
-void QtAccessibleWidget::invalidate() { m_xAccessible.clear(); }
+void QtAccessibleWidget::invalidate()
+{
+    QtAccessibleRegistry::remove(m_xAccessible);
+    m_xAccessible.clear();
+}
 
 Reference<XAccessibleContext> QtAccessibleWidget::getAccessibleContextImpl() 
const
 {
@@ -251,7 +256,8 @@ void 
lcl_appendRelation(QVector<QPair<QAccessibleInterface*, QAccessible::Relati
     {
         Reference<XAccessible> xAccessible(aRelation.TargetSet[i], 
uno::UNO_QUERY);
         relations->append(
-            { QAccessible::queryAccessibleInterface(new 
QtXAccessible(xAccessible)), aQRelation });
+            { 
QAccessible::queryAccessibleInterface(QtAccessibleRegistry::getQObject(xAccessible)),
+              aQRelation });
     }
 }
 }
@@ -315,7 +321,8 @@ QAccessibleInterface* QtAccessibleWidget::parent() const
         return nullptr;
 
     if (xAc->getAccessibleParent().is())
-        return QAccessible::queryAccessibleInterface(new 
QtXAccessible(xAc->getAccessibleParent()));
+        return QAccessible::queryAccessibleInterface(
+            QtAccessibleRegistry::getQObject(xAc->getAccessibleParent()));
 
     // go via the QObject hierarchy; some a11y objects like the application
     // (at the root of the a11y hierarchy) are handled solely by Qt and have
@@ -332,8 +339,8 @@ QAccessibleInterface* QtAccessibleWidget::child(int index) 
const
     Reference<XAccessibleContext> xAc = getAccessibleContextImpl();
     if (!xAc.is())
         return nullptr;
-
-    return QAccessible::queryAccessibleInterface(new 
QtXAccessible(xAc->getAccessibleChild(index)));
+    return QAccessible::queryAccessibleInterface(
+        QtAccessibleRegistry::getQObject(xAc->getAccessibleChild(index)));
 }
 
 QString QtAccessibleWidget::text(QAccessible::Text text) const
@@ -736,7 +743,7 @@ QAccessibleInterface* QtAccessibleWidget::childAt(int x, 
int y) const
     // convert from screen to local coordinates
     QPoint aLocalCoords = QPoint(x, y) - rect().topLeft();
     return QAccessible::queryAccessibleInterface(
-        new QtXAccessible(xAccessibleComponent->getAccessibleAtPoint(
+        
QtAccessibleRegistry::getQObject(xAccessibleComponent->getAccessibleAtPoint(
             awt::Point(aLocalCoords.x(), aLocalCoords.y()))));
 }
 
@@ -1458,7 +1465,8 @@ QAccessibleInterface* QtAccessibleWidget::caption() const
     Reference<XAccessibleTable> xTable(xAc, UNO_QUERY);
     if (!xTable.is())
         return nullptr;
-    return QAccessible::queryAccessibleInterface(new 
QtXAccessible(xTable->getAccessibleCaption()));
+    return QAccessible::queryAccessibleInterface(
+        QtAccessibleRegistry::getQObject(xTable->getAccessibleCaption()));
 }
 
 QAccessibleInterface* QtAccessibleWidget::cellAt(int row, int column) const
@@ -1471,7 +1479,7 @@ QAccessibleInterface* QtAccessibleWidget::cellAt(int row, 
int column) const
     if (!xTable.is())
         return nullptr;
     return QAccessible::queryAccessibleInterface(
-        new QtXAccessible(xTable->getAccessibleCellAt(row, column)));
+        QtAccessibleRegistry::getQObject(xTable->getAccessibleCellAt(row, 
column)));
 }
 
 int QtAccessibleWidget::columnCount() const
@@ -1603,7 +1611,7 @@ QList<QAccessibleInterface*> 
QtAccessibleWidget::selectedCells() const
     {
         Reference<XAccessible> xChild = 
xSelection->getSelectedAccessibleChild(i);
         QAccessibleInterface* pInterface
-            = QAccessible::queryAccessibleInterface(new QtXAccessible(xChild));
+            = 
QAccessible::queryAccessibleInterface(QtAccessibleRegistry::getQObject(xChild));
         aSelectedCells.push_back(pInterface);
     }
     return aSelectedCells;
@@ -1666,7 +1674,8 @@ QAccessibleInterface* QtAccessibleWidget::summary() const
     Reference<XAccessibleTable> xTable(xAc, UNO_QUERY);
     if (!xTable.is())
         return nullptr;
-    return QAccessible::queryAccessibleInterface(new 
QtXAccessible(xTable->getAccessibleSummary()));
+    return QAccessible::queryAccessibleInterface(
+        QtAccessibleRegistry::getQObject(xTable->getAccessibleSummary()));
 }
 
 bool QtAccessibleWidget::unselectColumn(int column)
@@ -1710,7 +1719,7 @@ QList<QAccessibleInterface*> 
QtAccessibleWidget::columnHeaderCells() const
     {
         Reference<XAccessible> xCell = xHeaders->getAccessibleCellAt(nRow, 
nCol);
         QAccessibleInterface* pInterface
-            = QAccessible::queryAccessibleInterface(new QtXAccessible(xCell));
+            = 
QAccessible::queryAccessibleInterface(QtAccessibleRegistry::getQObject(xCell));
         aHeaderCells.push_back(pInterface);
     }
     return aHeaderCells;
@@ -1776,7 +1785,7 @@ QList<QAccessibleInterface*> 
QtAccessibleWidget::rowHeaderCells() const
     {
         Reference<XAccessible> xCell = xHeaders->getAccessibleCellAt(nRow, 
nCol);
         QAccessibleInterface* pInterface
-            = QAccessible::queryAccessibleInterface(new QtXAccessible(xCell));
+            = 
QAccessible::queryAccessibleInterface(QtAccessibleRegistry::getQObject(xCell));
         aHeaderCells.push_back(pInterface);
     }
     return aHeaderCells;
@@ -1821,7 +1830,7 @@ QAccessibleInterface* QtAccessibleWidget::table() const
     if (!xTableAcc.is())
         return nullptr;
 
-    return QAccessible::queryAccessibleInterface(new QtXAccessible(xTableAcc));
+    return 
QAccessible::queryAccessibleInterface(QtAccessibleRegistry::getQObject(xTableAcc));
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/qt6/QtAccessibleRegistry.cxx b/vcl/qt6/QtAccessibleRegistry.cxx
new file mode 100644
index 000000000000..782393e71353
--- /dev/null
+++ b/vcl/qt6/QtAccessibleRegistry.cxx
@@ -0,0 +1,12 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include "../qt5/QtAccessibleRegistry.cxx"
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */

Reply via email to