Bug#1033995: qtbase-opensource-src: Fix accessibility of qt5 applications run as root
Lisandro Damián Nicanor Pérez Meyer, le mer. 12 avril 2023 21:54:03 -0300, a ecrit: > Samuel: do you know if this bug is also applicable to Qt 6? Yes it is. The patch I have sent upstream should apply fine on debian's qt6. Samuel
Bug#1033995: qtbase-opensource-src: Fix accessibility of qt5 applications run as root
Samuel: do you know if this bug is also applicable to Qt 6? -- Lisandro Damián Nicanor Pérez Meyer https://perezmeyer.com.ar/
Bug#1033995: qtbase-opensource-src: Fix accessibility of qt5 applications run as root
On Wed, 12 Apr 2023 at 19:56, Lisandro Damián Nicanor Pérez Meyer wrote: > > Uups, I wrongly tested this without re enabling the patch, my bad! > Building and testing again! Patch tested, everything looks fine! I am now pushing the package to the archive.
Bug#1033995: qtbase-opensource-src: Fix accessibility of qt5 applications run as root
Uups, I wrongly tested this without re enabling the patch, my bad! Building and testing again! On Wed, 12 Apr 2023 at 19:50, Samuel Thibault wrote: > > Lisandro Damián Nicanor Pérez Meyer, le mer. 12 avril 2023 19:49:00 -0300, a > ecrit: > > In the meantime I'll be preparing the upload, if everything is OK I'll > > push to the repo and upload the packages. > > Thanks for your patience on this! > > Samuel -- Lisandro Damián Nicanor Pérez Meyer https://perezmeyer.com.ar/
Bug#1033995: qtbase-opensource-src: Fix accessibility of qt5 applications run as root
Hi again, On Wed, 12 Apr 2023 at 19:19, Lisandro Damián Nicanor Pérez Meyer wrote: > > Hi, > > On Wed, 12 Apr 2023 at 19:03, Samuel Thibault wrote: > > > > And again it's posing problem. As advised by svuorela on irc, here is a > > version that defers the trigger. This is probably even safer since > > that's what is committed upstream for the AT_SPI_BUS_ADDRESS environment > > variable case. > > It makes sense, let me try the patch. krunner works as expected. I am now waiting for the automatic screen locker to pop in and check it works as expected too (this were two symptoms of his issue). In the meantime I'll be preparing the upload, if everything is OK I'll push to the repo and upload the packages. -- Lisandro Damián Nicanor Pérez Meyer https://perezmeyer.com.ar/
Bug#1033995: qtbase-opensource-src: Fix accessibility of qt5 applications run as root
Lisandro Damián Nicanor Pérez Meyer, le mer. 12 avril 2023 19:49:00 -0300, a ecrit: > In the meantime I'll be preparing the upload, if everything is OK I'll > push to the repo and upload the packages. Thanks for your patience on this! Samuel
Bug#1033995: qtbase-opensource-src: Fix accessibility of qt5 applications run as root
Hi, On Wed, 12 Apr 2023 at 19:03, Samuel Thibault wrote: > > And again it's posing problem. As advised by svuorela on irc, here is a > version that defers the trigger. This is probably even safer since > that's what is committed upstream for the AT_SPI_BUS_ADDRESS environment > variable case. It makes sense, let me try the patch. -- Lisandro Damián Nicanor Pérez Meyer https://perezmeyer.com.ar/
Bug#1033995: qtbase-opensource-src: Fix accessibility of qt5 applications run as root
And again it's posing problem. As advised by svuorela on irc, here is a
version that defers the trigger. This is probably even safer since
that's what is committed upstream for the AT_SPI_BUS_ADDRESS environment
variable case.
Samuel
--- a/src/platformsupport/linuxaccessibility/bridge.cpp
+++ b/src/platformsupport/linuxaccessibility/bridge.cpp
@@ -65,6 +65,16 @@ QSpiAccessibleBridge::QSpiAccessibleBrid
{
dbusConnection = new DBusConnection();
connect(dbusConnection, SIGNAL(enabledChanged(bool)), this,
SLOT(enabledChanged(bool)));
+// Now that we have connected the signal, make sure we didn't miss a
change,
+// e.g. when running as root or when AT_SPI_BUS_ADDRESS is set by hand.
+// But do that only on next loop, once dbus is really settled.
+QMetaObject::invokeMethod(
+this,
+[this] {
+if (dbusConnection->isEnabled())
+enabledChanged(true);
+},
+Qt::QueuedConnection);
}
void QSpiAccessibleBridge::enabledChanged(bool enabled)
Bug#1033995: qtbase-opensource-src: Fix accessibility of qt5 applications run as root
Control: reopen -1
Control: found -1 5.15.8+dfsg-5
Hello,
So the patch that was submitted upstream is indeed posing
problems: #1034160, #1034169, #1034191. AIUI, I guess that connecting
the enabledChanged signal too early is problematic because the code is
not actually ready to handle it because initialization is not finished.
I however came up with another way to fix the issue, that is way simpler
and should really not pose any problem since that's the way it's
happening in the normal case. I have submitted it upstream, and here is
the change.
Samuel
Description: fix accessibility on XCB when running as root
Accessibility actually works when running applications as root, but we
would never properly connect, since the enabledChanged signal would be
emitted from the constructor in this case. So after connecting the
signal, check the value by hand to make sure not to miss the
notification.
Only applications running as root would be affected, because all other
applications would go through the asynchronous pattern of getting the
bus address from dbus instead.
Origin: https://codereview.qt-project.org/c/qt/qtbase/+/205196
Bug: https://bugs.debian.org/1033995
Last-Update: 2023-04-09
--- a/src/platformsupport/linuxaccessibility/bridge.cpp
+++ b/src/platformsupport/linuxaccessibility/bridge.cpp
@@ -65,6 +65,10 @@ QSpiAccessibleBridge::QSpiAccessibleBrid
{
dbusConnection = new DBusConnection();
connect(dbusConnection, SIGNAL(enabledChanged(bool)), this,
SLOT(enabledChanged(bool)));
+// Now that we have connected the signal, make sure we didn't miss a
change,
+// e.g. when running as root or when AT_SPI_BUS_ADDRESS is set by hand.
+if (dbusConnection->isEnabled())
+enabledChanged(true);
}
void QSpiAccessibleBridge::enabledChanged(bool enabled)
Bug#1033995: qtbase-opensource-src: Fix accessibility of qt5 applications run as root
Hello, Dmitry Shachnev, le jeu. 06 avril 2023 22:15:01 +0300, a ecrit: > On Thu, Apr 06, 2023 at 06:20:04PM +0200, Samuel Thibault wrote: > > Oh, I didn't notice that there was a recorded test failure. > > > > I don't see how the failure can be related to the change... > > > > Cc-ing Frederik, in case he remembers / realizes something. > > I just want to add some context. We are talking about this change: > > https://codereview.qt-project.org/c/qt/qtbase/+/205196 > > Frederik, perhaps you can rebase and see if the tests still fail? Not sure Frederik is still working for KDE, I have pushed the rebase, let's see. Samuel
Bug#1033995: qtbase-opensource-src: Fix accessibility of qt5 applications run as root
Hi Frederik and Samuel, On Thu, Apr 06, 2023 at 06:20:04PM +0200, Samuel Thibault wrote: > Oh, I didn't notice that there was a recorded test failure. > > I don't see how the failure can be related to the change... > > Cc-ing Frederik, in case he remembers / realizes something. I just want to add some context. We are talking about this change: https://codereview.qt-project.org/c/qt/qtbase/+/205196 Frederik, perhaps you can rebase and see if the tests still fail? -- Dmitry Shachnev signature.asc Description: PGP signature
Bug#1033995: qtbase-opensource-src: Fix accessibility of qt5 applications run as root
Dmitry Shachnev, le jeu. 06 avril 2023 19:10:28 +0300, a ecrit:
> I don't like that this change has not been merged upstream because of
> test failures,
Oh, I didn't notice that there was a recorded test failure.
I don't see how the failure can be related to the change...
Cc-ing Frederik, in case he remembers / realizes something.
> But I looked at the KDE branch, and the last three commits they have there [1]
> seem to be related to the same problem. At least, the comment talks about the
> the same thing as your description:
>
> // Only connect on next loop run, connections to our enabled signal are
> // only established after the ctor returns.
>
> But these changes depend on AT_SPI_BUS_ADDRESS environment variable. Can we
> rely on it in Debian?
No, that variable is not usually set. root-owned applications get the
bus address from the X11 root window property.
It's unfortunate that this fix didn't see Frederik's patch which should
be way more straightforward to fix things.
Samuel
> --
> Dmitry Shachnev
> diff --git a/src/platformsupport/linuxaccessibility/dbusconnection.cpp
> b/src/platformsupport/linuxaccessibility/dbusconnection.cpp
> index 45ddc8e496..cc734abc63 100644
> --- a/src/platformsupport/linuxaccessibility/dbusconnection.cpp
> +++ b/src/platformsupport/linuxaccessibility/dbusconnection.cpp
> @@ -69,6 +69,21 @@ QT_BEGIN_NAMESPACE
> DBusConnection::DBusConnection(QObject *parent)
> : QObject(parent), m_a11yConnection(QString()), m_enabled(false)
> {
> +// If the bus is explicitly set via env var it overrides everything else.
> +QByteArray addressEnv = qgetenv("AT_SPI_BUS_ADDRESS");
> +if (!addressEnv.isEmpty()) {
> +// Only connect on next loop run, connections to our enabled signal
> are
> +// only established after the ctor returns.
> +QMetaObject::invokeMethod(
> +this,
> +[this, addressEnv] {
> +m_enabled = true;
> +connectA11yBus(QString::fromLocal8Bit(addressEnv));
> +},
> +Qt::QueuedConnection);
> +return;
> +}
> +
> // Start monitoring if "org.a11y.Bus" is registered as DBus service.
> QDBusConnection c = QDBusConnection::sessionBus();
> if (!c.isConnected()) {
Bug#1033995: qtbase-opensource-src: Fix accessibility of qt5 applications run as root
Hi Samuel!
On Thu, Apr 06, 2023 at 01:57:25AM +0200, Samuel Thibault wrote:
> Hello,
>
> Currently, qt5 applications, when run in sudo, are not accessible to
> screen readers. This is because the accessibility layer does not manage
> to connect to the accessibility bus to export the application content:
>
> https://bugreports.qt.io/browse/QTBUG-43674
>
> [...]
>
> This is particularly important because the calamares installer is based
> on qt5 and runs as root, and it currently is completely inaccessible to
> blind users, and this fix makes it possible for blind users to use it.
>
> I have confirmed that this fixes the issue for bookworm, would it be
> possible to upload to unstable? I'll then handle requesting the unblock
> from the release team.
I understand the importance of this patch, but I don't like that this change
has not been merged upstream because of test failures, and did not have any
activity for the last 5 years.
But I looked at the KDE branch, and the last three commits they have there [1]
seem to be related to the same problem. At least, the comment talks about the
the same thing as your description:
// Only connect on next loop run, connections to our enabled signal are
// only established after the ctor returns.
But these changes depend on AT_SPI_BUS_ADDRESS environment variable. Can we
rely on it in Debian? If yes, can you please check if those commits fix the
issue with calamares? I have attached them as a single patch for convenience.
[1]:
https://invent.kde.org/qt/qt/qtbase/-/commits/kde/5.15/src/platformsupport/linuxaccessibility/dbusconnection.cpp
--
Dmitry Shachnev
diff --git a/src/platformsupport/linuxaccessibility/dbusconnection.cpp b/src/platformsupport/linuxaccessibility/dbusconnection.cpp
index 45ddc8e496..cc734abc63 100644
--- a/src/platformsupport/linuxaccessibility/dbusconnection.cpp
+++ b/src/platformsupport/linuxaccessibility/dbusconnection.cpp
@@ -69,6 +69,21 @@ QT_BEGIN_NAMESPACE
DBusConnection::DBusConnection(QObject *parent)
: QObject(parent), m_a11yConnection(QString()), m_enabled(false)
{
+// If the bus is explicitly set via env var it overrides everything else.
+QByteArray addressEnv = qgetenv("AT_SPI_BUS_ADDRESS");
+if (!addressEnv.isEmpty()) {
+// Only connect on next loop run, connections to our enabled signal are
+// only established after the ctor returns.
+QMetaObject::invokeMethod(
+this,
+[this, addressEnv] {
+m_enabled = true;
+connectA11yBus(QString::fromLocal8Bit(addressEnv));
+},
+Qt::QueuedConnection);
+return;
+}
+
// Start monitoring if "org.a11y.Bus" is registered as DBus service.
QDBusConnection c = QDBusConnection::sessionBus();
if (!c.isConnected()) {
signature.asc
Description: PGP signature
Bug#1033995: qtbase-opensource-src: Fix accessibility of qt5 applications run as root
Source: qtbase-opensource-src
Version: 5.15.8+dfsg-3
Severity: important
Tags: patch upstream
Forwarded: https://bugreports.qt.io/browse/QTBUG-43674
Hello,
Currently, qt5 applications, when run in sudo, are not accessible to
screen readers. This is because the accessibility layer does not manage
to connect to the accessibility bus to export the application content:
https://bugreports.qt.io/browse/QTBUG-43674
Most of the support was merged into qt5, but there is a little fix
missing, that was missed by upstream. I have attached the fix, it is
very simple: the ordering in QSpiAccessibleBridge::QSpiAccessibleBridge
used to be
- new DBusConnection() creates the dbusConnection object
- the DBusConnection::DBusConnection constructor connects to the atspi
bus
- connect the enabledChanged signal
and this patch changes it to:
- new DBusConnection() creates the dbusConnection object
- connect the enabledChanged signal
- the DBusConnection::init method connects to the atspi bus
This is needed in the root case because since in that case it
cannot access the user session dbus, it uses a synchronous method,
in which case the enabledChanged signal is emitted from the
DBusConnection::DBusConnection constructor, and thus lost forever since
it was not connected yet at that time. So we need to connect the signal
before connecting to the atspi bus (and get the enabledChanged event).
This is particularly important because the calamares installer is based
on qt5 and runs as root, and it currently is completely inaccessible to
blind users, and this fix makes it possible for blind users to use it.
I have confirmed that this fixes the issue for bookworm, would it be
possible to upload to unstable? I'll then handle requesting the unblock
from the release team.
Samuel
-- System Information:
Debian Release: 12.0
APT prefers testing
APT policy: (990, 'testing'), (500, 'unstable-debug'), (500,
'testing-debug'), (500, 'stable-security'), (500, 'stable-debug'), (500,
'proposed-updates-debug'), (500, 'proposed-updates'), (500,
'oldstable-proposed-updates'), (500, 'oldoldstable'), (500, 'buildd-unstable'),
(500, 'unstable'), (500, 'stable'), (500, 'oldstable'), (1,
'experimental-debug'), (1, 'buildd-experimental'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386, arm64
Kernel: Linux 6.2.0 (SMP w/8 CPU threads; PREEMPT)
Locale: LANG=fr_FR.UTF-8, LC_CTYPE=fr_FR.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled
From: Frederik Gladhorn
Date: Tue, 12 Sep 2017 09:22:30 + (+0200)
Subject: Fix accessibility on XCB when running as root
X-Git-Url:
https://codereview.qt-project.org/gitweb?p=qt%2Fqtbase.git;a=commitdiff_plain;h=4ee3703ffaf063047285247016ee9e5c07ef3b53;hp=689606de91faecf91f1f92e8d355789d9be62d2f
Forwarded: https://bugreports.qt.io/browse/QTBUG-43674
Fix accessibility on XCB when running as root
Accessibility actually works when running applications as root, but we
would never properly connect, since the enabledChanged signal would be
emitted from the constructor in this case.
Only applications running as root would be affected, because all other
applications would go through the asynchronous pattern of getting the
bus address from dbus instead.
Since running apps as root won't let them access the session bus, the
xatom is the way to go.
[ChangeLog][QtGui][Accessibility] On XCB applications running as root are
now accessible.
Task-number: QTBUG-43674
Change-Id: I82cdc35f00693a8366dfcdab2f2c3c6dc5f5b783
---
---
src/platformsupport/linuxaccessibility/bridge.cpp |1 +
src/platformsupport/linuxaccessibility/dbusconnection.cpp |8
src/platformsupport/linuxaccessibility/dbusconnection_p.h |1 +
3 files changed, 10 insertions(+)
--- a/src/platformsupport/linuxaccessibility/bridge.cpp
+++ b/src/platformsupport/linuxaccessibility/bridge.cpp
@@ -65,6 +65,7 @@ QSpiAccessibleBridge::QSpiAccessibleBrid
{
dbusConnection = new DBusConnection();
connect(dbusConnection, SIGNAL(enabledChanged(bool)), this,
SLOT(enabledChanged(bool)));
+dbusConnection->init();
}
void QSpiAccessibleBridge::enabledChanged(bool enabled)
--- a/src/platformsupport/linuxaccessibility/dbusconnection.cpp
+++ b/src/platformsupport/linuxaccessibility/dbusconnection.cpp
@@ -69,6 +69,14 @@ QT_BEGIN_NAMESPACE
DBusConnection::DBusConnection(QObject *parent)
: QObject(parent), m_a11yConnection(QString()), m_enabled(false)
{
+}
+
+/**
+\internal
+Connect to the accessibility dbus service.
+*/
+void DBusConnection::init()
+{
// Start monitoring if "org.a11y.Bus" is registered as DBus service.
QDBusConnection c = QDBusConnection::sessionBus();
if (!c.isConnected()) {
--- a/src/platformsupport/linuxaccessibility/dbusconnection_p.h
+++ b/src/platformsupport/linuxaccessibility/dbusconnection_p.h
@@ -67,6 +67,7 @@ class DBusConnection : public QObject
