keepassxreboot / keepassxc

KeePassXC is a cross-platform community-driven port of the Windows application “Keepass Password Safe”.
https://keepassxc.org/
Other
21.43k stars 1.48k forks source link

Add Qt::WA_X11NetWmWindowTypeDialog attribute for the database unlock dialogue window #5623

Open selurvedu opened 4 years ago

selurvedu commented 4 years ago

Summary

Add Qt::WA_X11NetWmWindowTypeDialog attribute for the database unlock dialogue window.

This feature is available since Qt 5.6.2, see: https://bugreports.qt.io/browse/QTBUG-50190 https://codereview.qt-project.org/c/qt/qtbase/+/111936 https://doc.qt.io/archives/qt-5.6/qt.html

Examples

Implementing this feature would be as simple as applying a one-line patch:

diff --git a/src/gui/DatabaseOpenDialog.cpp b/src/gui/DatabaseOpenDialog.cpp
index e7194b7e..118c77af 100644
--- a/src/gui/DatabaseOpenDialog.cpp
+++ b/src/gui/DatabaseOpenDialog.cpp
@@ -24,6 +24,7 @@ DatabaseOpenDialog::DatabaseOpenDialog(QWidget* parent)
     : QDialog(parent)
     , m_view(new DatabaseOpenWidget(this))
 {
+    setAttribute(Qt::WA_X11NetWmWindowTypeDialog);
     setWindowTitle(tr("Unlock Database - KeePassXC"));
     setWindowFlags(Qt::Dialog | Qt::WindowStaysOnTopHint);
     connect(m_view, SIGNAL(dialogFinished(bool)), this, SLOT(complete(bool)));

I applied this patch on top of KeePassXC 2.6.2 and built it with Qt 5.15.1 on GNU/Linux. xprop outputs _NET_WM_WINDOW_TYPE(ATOM) = _NET_WM_WINDOW_TYPE_DIALOG, _NET_WM_WINDOW_TYPE_NORMAL. Everything else works as usual.

Context

Some window managers on X11 have higher priorities and special placement rules for dialogue windows. This change would let them treat the unlock database window appropriately.

Precautions

This was not tested on Windows, macOS and Wayland.

phoerious commented 4 years ago

This would have to be ifdef-guarded, since our lowest supported Qt version is 5.5 (Ubuntu Xenial). Would you be willing to make a pull request of this?

selurvedu commented 4 years ago

According to the Qt patch and ticket I mentioned, the attribute was there for ages, but it simply did nothing. See here: https://github.com/qt/qtbase/blame/5.5/src/widgets/kernel/qwidget.cpp#L11185 It's also in the docs for Qt 5.5: https://doc.qt.io/archives/qt-5.5/qt.html

I just got a successful build on Xenial without ifdefs. If you're okay with that, I'm not gonna add them, if not, just let me know.

Would you be willing to make a pull request of this?

Yes, I can make a PR. I simply have to follow the guidelines in .github/CONTRIBUTING.md, right?

phoerious commented 4 years ago

Yes.