vcl/qt5/QtFrame.cxx | 53 ++++++++++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 24 deletions(-)
New commits: commit fcaa903461ec0f5d4c8ce730f4c64adb3cec2262 Author: Michael Weghorn <[email protected]> AuthorDate: Thu Sep 19 09:15:37 2024 +0200 Commit: Michael Weghorn <[email protected]> CommitDate: Thu Sep 19 15:36:15 2024 +0200 qt a11y: Defer QWindow creation until frame gets shown Calling QtFrame::windowHandle makes sure that the QWindow exists, by calling QWidget::setAttribute(Qt::WA_NativeWindow). QWindows that don't have the Qt::Popup or Qt::Desktop type, are listed as accessible children of the application, see QAccessibleApplication::child [1] in qtbase and the `topLevelObjects()` [2] helper method it uses. This resulted in various dummy "top-level" objects being reported as children of the LibreOffice a11y app object, and therefore shown in Accerciser. For Writer, there are 2 instances for each of the popups for the comboboxes in the formatting toolbar that get reported as toplevels, as can seen by printing the accessible ID of their parent objects in Accerciser (they have a parent different from the app, since they're not actually top-levels): Paragraph style combobox: In [4]: acc.parent.accessibleId Out[4]: 'applystyle' Font name combobox: In [5]: acc.parent.accessibleId Out[5]: 'fontnamecombobox' Font size comobobox: In [6]: acc.parent.accessibleId Out[6]: 'fontsizecombobox' While these *are* popups, the Qt::Popup type is currently not used for them in LO to work around another issue with these popus on Wayland, but the Qt::Tooltip type is used, see the QtFrame ctor and comments there. To prevent these wrong "top-levels" from always being part of the a11y tree due to that, defer creating the QWindow by calling QtFrame::windowHandle to when the window is actually needed, in QtFrame::Show. Pass the Qt::UniqueConnection connection type param in the call to QObject::connect [3], to avoid duplicate connections if the frame gets hidden/shown multiple times. (Other than the ctor, QtFrame::Show could potentially get called multiple times.) With this in place, Accerciser now only shows a single top-level frame after starting LO Writer with the qt6 VCL plugin, which is the actual Writer window. [1] https://code.qt.io/cgit/qt/qtbase.git/tree/src/gui/accessible/qaccessibleobject.cpp?id=0681e720a9851f1873ce5a5f99b5567d2b418261#n160] [2] https://code.qt.io/cgit/qt/qtbase.git/tree/src/gui/accessible/qaccessibleobject.cpp?id=0681e720a9851f1873ce5a5f99b5567d2b418261#n122 [3] https://doc.qt.io/qt-6/qobject.html#connect Change-Id: I7aff5c435dfa8b6aadcbbedb0d84db19bb86c8ab Reviewed-on: https://gerrit.libreoffice.org/c/core/+/173656 Tested-by: Jenkins Reviewed-by: Michael Weghorn <[email protected]> diff --git a/vcl/qt5/QtFrame.cxx b/vcl/qt5/QtFrame.cxx index fb3c5b66b0de..4d9c12891cdf 100644 --- a/vcl/qt5/QtFrame.cxx +++ b/vcl/qt5/QtFrame.cxx @@ -178,16 +178,6 @@ QtFrame::QtFrame(QtFrame* pParent, SalFrameStyleFlags nStyle, bool bUseCairo) FillSystemEnvData(m_aSystemData, reinterpret_cast<sal_IntPtr>(this), m_pQWidget); - QWindow* pChildWindow = windowHandle(); - connect(pChildWindow, &QWindow::screenChanged, this, &QtFrame::screenChanged); - - if (pParent && !(pParent->m_nStyle & SalFrameStyleFlags::PLUG)) - { - QWindow* pParentWindow = pParent->windowHandle(); - if (pParentWindow && pChildWindow && (pParentWindow != pChildWindow)) - pChildWindow->setTransientParent(pParentWindow); - } - SetIcon(SV_ICON_ID_OFFICE); } @@ -429,6 +419,17 @@ void QtFrame::Show(bool bVisible, bool bNoActivate) return; } + QWindow* pChildWindow = windowHandle(); + connect(pChildWindow, &QWindow::screenChanged, this, &QtFrame::screenChanged, + Qt::UniqueConnection); + + if (m_pParent && !(m_pParent->m_nStyle & SalFrameStyleFlags::PLUG)) + { + QWindow* pParentWindow = m_pParent->windowHandle(); + if (pParentWindow && pChildWindow && (pParentWindow != pChildWindow)) + pChildWindow->setTransientParent(pParentWindow); + } + // show SetDefaultSize(); commit b87df743b5d987e607c769e3c8f52cd3034dcaa8 Author: Michael Weghorn <[email protected]> AuthorDate: Thu Sep 19 08:57:41 2024 +0200 Commit: Michael Weghorn <[email protected]> CommitDate: Thu Sep 19 15:36:08 2024 +0200 qt: Run whole QtFrame::Show in main thread Instead of running only parts of the method explicitly in the main thread, ensure this for the whole method. Also, add a SolarMutexGuard at the beginning. Change-Id: I9c510b2d58ee5a3b05c1a16ce8e53077c7075caa Reviewed-on: https://gerrit.libreoffice.org/c/core/+/173655 Tested-by: Jenkins Reviewed-by: Michael Weghorn <[email protected]> diff --git a/vcl/qt5/QtFrame.cxx b/vcl/qt5/QtFrame.cxx index 7723967c1537..fb3c5b66b0de 100644 --- a/vcl/qt5/QtFrame.cxx +++ b/vcl/qt5/QtFrame.cxx @@ -410,32 +410,36 @@ void QtFrame::SetExtendedFrameStyle(SalExtStyle /*nExtStyle*/) { /* not needed * void QtFrame::Show(bool bVisible, bool bNoActivate) { + SolarMutexGuard g; + QtInstance* pQtInstance = GetQtInstance(); + assert(pQtInstance); + if (!pQtInstance->IsMainThread()) + { + pQtInstance->RunInMainThread([&] { Show(bVisible, bNoActivate); }); + return; + } + assert(m_pQWidget); if (bVisible == asChild()->isVisible()) return; - auto* pSalInst(GetQtInstance()); - assert(pSalInst); - if (!bVisible) // hide { - pSalInst->RunInMainThread([this]() { asChild()->setVisible(false); }); + asChild()->setVisible(false); return; } // show SetDefaultSize(); - pSalInst->RunInMainThread([this, bNoActivate]() { - QWidget* const pChild = asChild(); - pChild->setVisible(true); - pChild->raise(); - if (!bNoActivate) - { - pChild->activateWindow(); - pChild->setFocus(); - } - }); + QWidget* const pChild = asChild(); + pChild->setVisible(true); + pChild->raise(); + if (!bNoActivate) + { + pChild->activateWindow(); + pChild->setFocus(); + } } void QtFrame::SetMinClientSize(tools::Long nWidth, tools::Long nHeight)
