From 0b28abeb01073fad3eef6893d6ea11d5206ff762 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Fl=C3=B6ser?= Date: Sun, 23 Dec 2018 08:56:15 +0100 Subject: [PATCH] Port window specific rules dialog to DBus Summary: The dialog invoked through user actions menu takes the internal uuid as command line argument which allows to query the required information from KWin instead of using X11. This allows to enable the system for Wayland windows. In order to replace the usage of ClientMachine in the rules dialog the dbus interface is extended by a value whether the window is on the localhost. This is exposed through a virtual method on toplevel which is overridden in ShellClient and there always returning true. Test Plan: Run a nested Wayland and opened the dialog on a wayland window Reviewers: #kwin Subscribers: kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D17750 --- autotests/integration/dbus_interface_test.cpp | 6 +- dbusinterface.cpp | 1 + kcmkwin/kwinrules/CMakeLists.txt | 2 +- kcmkwin/kwinrules/kwinsrc.cpp | 1 - kcmkwin/kwinrules/main.cpp | 87 ++++++++++--------- kcmkwin/kwinrules/ruleslist.cpp | 4 +- kcmkwin/kwinrules/ruleswidget.cpp | 52 ++--------- kcmkwin/kwinrules/ruleswidget.h | 6 +- rules.cpp | 3 +- shell_client.h | 5 ++ toplevel.cpp | 8 ++ toplevel.h | 1 + useractions.cpp | 10 +-- 13 files changed, 82 insertions(+), 104 deletions(-) diff --git a/autotests/integration/dbus_interface_test.cpp b/autotests/integration/dbus_interface_test.cpp index cde2319a19..c58b7baa87 100644 --- a/autotests/integration/dbus_interface_test.cpp +++ b/autotests/integration/dbus_interface_test.cpp @@ -151,7 +151,7 @@ void TestDbusInterface::testGetWindowInfoShellClient() QVERIFY(!reply.isError()); auto windowData = reply.value(); QVERIFY(!windowData.isEmpty()); - QCOMPARE(windowData.size(), 23); + QCOMPARE(windowData.size(), 24); QCOMPARE(windowData.value(QStringLiteral("type")).toInt(), NET::Normal); QCOMPARE(windowData.value(QStringLiteral("x")).toInt(), client->x()); QCOMPARE(windowData.value(QStringLiteral("y")).toInt(), client->y()); @@ -170,6 +170,7 @@ void TestDbusInterface::testGetWindowInfoShellClient() QCOMPARE(windowData.value(QStringLiteral("maximizeVertical")).toBool(), false); QCOMPARE(windowData.value(QStringLiteral("noBorder")).toBool(), true); QCOMPARE(windowData.value(QStringLiteral("clientMachine")).toString(), QString()); + QCOMPARE(windowData.value(QStringLiteral("localhost")).toBool(), true); QCOMPARE(windowData.value(QStringLiteral("role")).toString(), QString()); QCOMPARE(windowData.value(QStringLiteral("resourceName")).toString(), QStringLiteral("testDbusInterface")); if (type == Test::ShellSurfaceType::WlShell) { @@ -298,7 +299,7 @@ void TestDbusInterface::testGetWindowInfoX11Client() QVERIFY(!reply.isError()); auto windowData = reply.value(); QVERIFY(!windowData.isEmpty()); - QCOMPARE(windowData.size(), 23); + QCOMPARE(windowData.size(), 24); QCOMPARE(windowData.value(QStringLiteral("type")).toInt(), NET::Normal); QCOMPARE(windowData.value(QStringLiteral("x")).toInt(), client->x()); QCOMPARE(windowData.value(QStringLiteral("y")).toInt(), client->y()); @@ -322,6 +323,7 @@ void TestDbusInterface::testGetWindowInfoX11Client() QCOMPARE(windowData.value(QStringLiteral("desktopFile")).toString(), QStringLiteral("org.kde.foo")); QCOMPARE(windowData.value(QStringLiteral("caption")).toString(), QStringLiteral("Some caption")); // not testing clientmachine as that is system dependent + // due to that also not testing localhost auto verifyProperty = [client] (const QString &name) { QDBusPendingReply reply{getWindowInfo(client->internalId())}; diff --git a/dbusinterface.cpp b/dbusinterface.cpp index 76667db6df..2a1da8b912 100644 --- a/dbusinterface.cpp +++ b/dbusinterface.cpp @@ -199,6 +199,7 @@ QVariantMap clientToVariantMap(const AbstractClient *c) {QStringLiteral("role"), c->windowRole()}, {QStringLiteral("caption"), c->captionNormal()}, {QStringLiteral("clientMachine"), c->wmClientMachine(true)}, + {QStringLiteral("localhost"), c->isLocalhost()}, {QStringLiteral("type"), c->windowType()}, {QStringLiteral("x"), c->x()}, {QStringLiteral("y"), c->y()}, diff --git a/kcmkwin/kwinrules/CMakeLists.txt b/kcmkwin/kwinrules/CMakeLists.txt index b53f7b5f51..34bc7cbaca 100644 --- a/kcmkwin/kwinrules/CMakeLists.txt +++ b/kcmkwin/kwinrules/CMakeLists.txt @@ -4,7 +4,7 @@ add_definitions(-DKCMRULES) ########### next target ############### include_directories(../../) -set (kwinrules_MOC_HDRS yesnobox.h ../../client_machine.h ../../cursor.h ../../plugins/platforms/x11/standalone/x11cursor.h) +set (kwinrules_MOC_HDRS yesnobox.h ../../cursor.h ../../plugins/platforms/x11/standalone/x11cursor.h) qt5_wrap_cpp(kwinrules_MOC_SRCS ${kwinrules_MOC_HDRS}) set(kwinrules_SRCS ruleswidget.cpp ruleslist.cpp kwinsrc.cpp detectwidget.cpp ${kwinrules_MOC_SRCS}) diff --git a/kcmkwin/kwinrules/kwinsrc.cpp b/kcmkwin/kwinrules/kwinsrc.cpp index 4f48db8240..24a67f85df 100644 --- a/kcmkwin/kwinrules/kwinsrc.cpp +++ b/kcmkwin/kwinrules/kwinsrc.cpp @@ -26,7 +26,6 @@ #include "../../placement.cpp" #include "../../options.cpp" #include "../../utils.cpp" -#include "../../client_machine.cpp" KWin::InputRedirection *KWin::InputRedirection::s_self = nullptr; diff --git a/kcmkwin/kwinrules/main.cpp b/kcmkwin/kwinrules/main.cpp index a13150711f..c001011835 100644 --- a/kcmkwin/kwinrules/main.cpp +++ b/kcmkwin/kwinrules/main.cpp @@ -21,16 +21,19 @@ #include #include #include -#include -#include -#include -#include #include "ruleswidget.h" #include "../../rules.h" -#include "../../client_machine.h" #include +#include +#include +#include +#include +#include + +Q_DECLARE_METATYPE(NET::WindowType) + namespace KWin { @@ -67,29 +70,14 @@ static void saveRules(const QList< Rules* >& rules) } } -static Rules* findRule(const QList< Rules* >& rules, Window wid, bool whole_app) +static Rules* findRule(const QList< Rules* >& rules, const QVariantMap &data, bool whole_app) { - // ClientMachine::resolve calls NETWinInfo::update() which requires properties - // bug #348472 ./. bug #346748 - if (QX11Info::isPlatformX11()) { - qApp->setProperty("x11Connection", QVariant::fromValue(QX11Info::connection())); - qApp->setProperty("x11RootWindow", QVariant::fromValue(QX11Info::appRootWindow())); - } - KWindowInfo info = KWindowInfo(wid, - NET::WMName | NET::WMWindowType, - NET::WM2WindowClass | NET::WM2WindowRole | NET::WM2ClientMachine); - if (!info.valid()) // shouldn't really happen - return nullptr; - ClientMachine clientMachine; - clientMachine.resolve(info.win(), info.groupLeader()); - QByteArray wmclass_class = info.windowClassClass().toLower(); - QByteArray wmclass_name = info.windowClassName().toLower(); - QByteArray role = info.windowRole().toLower(); - NET::WindowType type = info.windowType(NET::NormalMask | NET::DesktopMask | NET::DockMask - | NET::ToolbarMask | NET::MenuMask | NET::DialogMask | NET::OverrideMask | NET::TopMenuMask - | NET::UtilityMask | NET::SplashMask); - QString title = info.name(); - QByteArray machine = clientMachine.hostName(); + QByteArray wmclass_class = data.value("resourceClass").toByteArray().toLower(); + QByteArray wmclass_name = data.value("resourceName").toByteArray().toLower(); + QByteArray role = data.value("role").toByteArray().toLower(); + NET::WindowType type = data.value("type").value(); + QString title = data.value("caption").toString(); + QByteArray machine = data.value("clientMachine").toByteArray(); Rules* best_match = nullptr; int match_quality = 0; for (QList< Rules* >::ConstIterator it = rules.constBegin(); @@ -136,7 +124,7 @@ static Rules* findRule(const QList< Rules* >& rules, Window wid, bool whole_app) if (!rule->matchType(type) || !rule->matchRole(role) || !rule->matchTitle(title) - || !rule->matchClientMachine(machine, clientMachine.isLocal())) + || !rule->matchClientMachine(machine, data.value("localhost").toBool())) continue; if (quality > match_quality) { best_match = rule; @@ -212,16 +200,16 @@ static Rules* findRule(const QList< Rules* >& rules, Window wid, bool whole_app) return ret; } -static int edit(Window wid, bool whole_app) +static void edit(const QVariantMap &data, bool whole_app) { QList< Rules* > rules; loadRules(rules); - Rules* orig_rule = findRule(rules, wid, whole_app); + Rules* orig_rule = findRule(rules, data, whole_app); RulesDialog dlg; if (whole_app) dlg.setWindowTitle(i18nc("Window caption for the application wide rules dialog", "Edit Application-Specific Settings")); // dlg.edit() creates new Rules instance if edited - Rules* edited_rule = dlg.edit(orig_rule, wid, true); + Rules* edited_rule = dlg.edit(orig_rule, data, true); if (edited_rule == nullptr || edited_rule->isEmpty()) { rules.removeAll(orig_rule); delete orig_rule; @@ -240,7 +228,7 @@ static int edit(Window wid, bool whole_app) QDBusMessage message = QDBusMessage::createSignal("/KWin", "org.kde.KWin", "reloadConfig"); QDBusConnection::sessionBus().send(message); - return 0; + qApp->quit(); } } // namespace @@ -248,28 +236,49 @@ static int edit(Window wid, bool whole_app) extern "C" KWIN_EXPORT int kdemain(int argc, char* argv[]) { - qputenv("QT_QPA_PLATFORM", "xcb"); QApplication app(argc, argv); app.setApplicationDisplayName(i18n("KWin")); app.setApplicationName("kwin_rules_dialog"); app.setApplicationVersion("1.0"); bool whole_app = false; - bool id_ok = false; - Window id = None; + QUuid uuid; { QCommandLineParser parser; parser.setApplicationDescription(i18n("KWin helper utility")); - parser.addOption(QCommandLineOption("wid", i18n("WId of the window for special window settings."), "wid")); + parser.addOption(QCommandLineOption("uuid", i18n("KWin id of the window for special window settings."), "uuid")); parser.addOption(QCommandLineOption("whole-app", i18n("Whether the settings should affect all windows of the application."))); parser.process(app); - id = parser.value("wid").toULongLong(&id_ok); + uuid = QUuid::fromString(parser.value("uuid")); whole_app = parser.isSet("whole-app"); } - if (!id_ok || id == None) { + + if (uuid.isNull()) { printf("%s\n", qPrintable(i18n("This helper utility is not supposed to be called directly."))); return 1; } - return KWin::edit(id, whole_app); + QDBusMessage message = QDBusMessage::createMethodCall(QStringLiteral("org.kde.KWin"), + QStringLiteral("/KWin"), + QStringLiteral("org.kde.KWin"), + QStringLiteral("getWindowInfo")); + message.setArguments({uuid.toString()}); + QDBusPendingReply async = QDBusConnection::sessionBus().asyncCall(message); + + QDBusPendingCallWatcher *callWatcher = new QDBusPendingCallWatcher(async, &app); + QObject::connect(callWatcher, &QDBusPendingCallWatcher::finished, &app, + [&whole_app] (QDBusPendingCallWatcher *self) { + QDBusPendingReply reply = *self; + self->deleteLater(); + if (!reply.isValid() || reply.value().isEmpty()) { + qApp->quit(); + return; + } + KWin::edit(reply.value(), whole_app); + } + ); + + + + return app.exec(); } diff --git a/kcmkwin/kwinrules/ruleslist.cpp b/kcmkwin/kwinrules/ruleslist.cpp index 495711339d..ae5fc68a39 100644 --- a/kcmkwin/kwinrules/ruleslist.cpp +++ b/kcmkwin/kwinrules/ruleslist.cpp @@ -82,7 +82,7 @@ void KCMRulesList::activeChanged() void KCMRulesList::newClicked() { RulesDialog dlg(this); - Rules* rule = dlg.edit(nullptr, 0, false); + Rules* rule = dlg.edit(nullptr, {}, false); if (rule == nullptr) return; int pos = rules_listbox->currentRow() + 1; @@ -98,7 +98,7 @@ void KCMRulesList::modifyClicked() if (pos == -1) return; RulesDialog dlg(this); - Rules* rule = dlg.edit(rules[ pos ], 0, false); + Rules* rule = dlg.edit(rules[ pos ], {}, false); if (rule == rules[ pos ]) return; delete rules[ pos ]; diff --git a/kcmkwin/kwinrules/ruleswidget.cpp b/kcmkwin/kwinrules/ruleswidget.cpp index 30374429b4..9fb889374a 100644 --- a/kcmkwin/kwinrules/ruleswidget.cpp +++ b/kcmkwin/kwinrules/ruleswidget.cpp @@ -24,7 +24,6 @@ #include #include #include -#include #include #include @@ -740,45 +739,6 @@ void RulesWidget::detected(bool ok) #define COMBOBOX_PREFILL( var, func, info ) GENERIC_PREFILL( var, func, info, setCurrentIndex ) #define SPINBOX_PREFILL( var, func, info ) GENERIC_PREFILL( var, func, info, setValue ) -void RulesWidget::prefillUnusedValues(const KWindowInfo& info) -{ - LINEEDIT_PREFILL(position, positionToStr, info.frameGeometry().topLeft()); - LINEEDIT_PREFILL(size, sizeToStr, info.frameGeometry().size()); - COMBOBOX_PREFILL(desktop, desktopToCombo, info.desktop()); - // COMBOBOX_PREFILL(activity, activityToCombo, info.activity()); // TODO: ivan - CHECKBOX_PREFILL(maximizehoriz, , info.state() & NET::MaxHoriz); - CHECKBOX_PREFILL(maximizevert, , info.state() & NET::MaxVert); - CHECKBOX_PREFILL(minimize, , info.isMinimized()); - CHECKBOX_PREFILL(shade, , info.state() & NET::Shaded); - CHECKBOX_PREFILL(fullscreen, , info.state() & NET::FullScreen); - //COMBOBOX_PREFILL( placement, placementToCombo ); - CHECKBOX_PREFILL(above, , info.state() & NET::KeepAbove); - CHECKBOX_PREFILL(below, , info.state() & NET::KeepBelow); - // noborder is only internal KWin information, so let's guess - CHECKBOX_PREFILL(noborder, , info.frameGeometry() == info.geometry()); - CHECKBOX_PREFILL(skiptaskbar, , info.state() & NET::SkipTaskbar); - CHECKBOX_PREFILL(skippager, , info.state() & NET::SkipPager); - CHECKBOX_PREFILL(skipswitcher, , info.state() & NET::SkipSwitcher); - //CHECKBOX_PREFILL( acceptfocus, ); - //CHECKBOX_PREFILL( closeable, ); - //CHECKBOX_PREFILL( autogroup, ); - //CHECKBOX_PREFILL( autogroupfg, ); - //LINEEDIT_PREFILL( autogroupid, ); - SPINBOX_PREFILL(opacityactive, , 100 /*get the actual opacity somehow*/); - SPINBOX_PREFILL(opacityinactive, , 100 /*get the actual opacity somehow*/); - //LINEEDIT_PREFILL( shortcut, ); - //COMBOBOX_PREFILL( fsplevel, ); - //COMBOBOX_PREFILL( fpplevel, ); - COMBOBOX_PREFILL(type, typeToCombo, info.windowType(SUPPORTED_MANAGED_WINDOW_TYPES_MASK)); - //CHECKBOX_PREFILL( ignoregeometry, ); - LINEEDIT_PREFILL(minsize, sizeToStr, info.frameGeometry().size()); - LINEEDIT_PREFILL(maxsize, sizeToStr, info.frameGeometry().size()); - //CHECKBOX_PREFILL( strictgeometry, ); - //CHECKBOX_PREFILL( disableglobalshortcuts, ); - //CHECKBOX_PREFILL( blockcompositing, ); - LINEEDIT_PREFILL(desktopfile, , info.desktopFileName()); -} - void RulesWidget::prefillUnusedValues(const QVariantMap& info) { const QSize windowSize{info.value("width").toInt(), info.value("height").toInt()}; @@ -849,11 +809,9 @@ bool RulesWidget::finalCheck() return true; } -void RulesWidget::prepareWindowSpecific(WId window) +void RulesWidget::prepareWindowSpecific(const QVariantMap &info) { - // TODO: adjust for Wayland tabs->setCurrentIndex(1); // geometry tab, skip tab for window identification - KWindowInfo info(window, NET::WMAllProperties, NET::WM2AllProperties); // read everything prefillUnusedValues(info); } @@ -886,12 +844,14 @@ RulesDialog::RulesDialog(QWidget* parent, const char* name) // window is set only for Alt+F3/Window-specific settings, because the dialog // is then related to one specific window -Rules* RulesDialog::edit(Rules* r, WId window, bool show_hints) +Rules* RulesDialog::edit(Rules* r, const QVariantMap& info, bool show_hints) { rules = r; widget->setRules(rules); - if (window != 0) - widget->prepareWindowSpecific(window); + if (!info.isEmpty()) + { + widget->prepareWindowSpecific(info); + } if (show_hints) QTimer::singleShot(0, this, SLOT(displayHints())); exec(); diff --git a/kcmkwin/kwinrules/ruleswidget.h b/kcmkwin/kwinrules/ruleswidget.h index 3f48c643a7..6c1253d4cf 100644 --- a/kcmkwin/kwinrules/ruleswidget.h +++ b/kcmkwin/kwinrules/ruleswidget.h @@ -23,7 +23,6 @@ #include #include -#include #include #include "ui_ruleswidgetbase.h" @@ -50,7 +49,7 @@ public: void setRules(Rules* r); Rules* rules() const; bool finalCheck(); - void prepareWindowSpecific(WId window); + void prepareWindowSpecific(const QVariantMap &info); Q_SIGNALS: void changed(bool state); protected Q_SLOTS: @@ -117,7 +116,6 @@ private: int comboToTiling(int val) const; int inc(int i) const { return i+1; } int dec(int i) const { return i-1; } - void prefillUnusedValues(const KWindowInfo& info); void prefillUnusedValues(const QVariantMap& info); DetectDialog* detect_dlg; bool detect_dlg_ok; @@ -129,7 +127,7 @@ class RulesDialog Q_OBJECT public: explicit RulesDialog(QWidget* parent = nullptr, const char* name = nullptr); - Rules* edit(Rules* r, WId window, bool show_hints); + Rules* edit(Rules* r, const QVariantMap& info, bool show_hints); protected: virtual void accept(); private Q_SLOTS: diff --git a/rules.cpp b/rules.cpp index eab6f93479..af31d1538d 100644 --- a/rules.cpp +++ b/rules.cpp @@ -1042,7 +1042,7 @@ void RuleBook::edit(AbstractClient* c, bool whole_app) { save(); QStringList args; - args << QStringLiteral("--wid") << QString::number(c->window()); + args << QStringLiteral("--uuid") << c->internalId().toString(); if (whole_app) args << QStringLiteral("--whole-app"); QProcess *p = new Process(this); @@ -1050,6 +1050,7 @@ void RuleBook::edit(AbstractClient* c, bool whole_app) p->setProcessEnvironment(kwinApp()->processStartupEnvironment()); const QFileInfo buildDirBinary{QDir{QCoreApplication::applicationDirPath()}, QStringLiteral("kwin_rules_dialog")}; p->setProgram(buildDirBinary.exists() ? buildDirBinary.absoluteFilePath() : QStringLiteral(KWIN_RULES_DIALOG_BIN)); + p->setProcessChannelMode(QProcess::MergedChannels); connect(p, static_cast(&QProcess::finished), p, &QProcess::deleteLater); connect(p, static_cast(&QProcess::error), this, [p] (QProcess::ProcessError e) { diff --git a/shell_client.h b/shell_client.h index 60754ac936..f5557f4e6d 100644 --- a/shell_client.h +++ b/shell_client.h @@ -168,6 +168,11 @@ public: bool isPopupWindow() const override; + bool isLocalhost() const override + { + return true; + } + protected: void addDamage(const QRegion &damage) override; bool belongsToSameApplication(const AbstractClient *other, SameApplicationChecks checks) const override; diff --git a/toplevel.cpp b/toplevel.cpp index 0b83e621f8..9605e6a99b 100644 --- a/toplevel.cpp +++ b/toplevel.cpp @@ -558,5 +558,13 @@ QRect Toplevel::inputGeometry() const return geometry(); } +bool Toplevel::isLocalhost() const +{ + if (!m_clientMachine) { + return true; + } + return m_clientMachine->isLocal(); +} + } // namespace diff --git a/toplevel.h b/toplevel.h index 7a4a2c7c2f..23bf1fda97 100644 --- a/toplevel.h +++ b/toplevel.h @@ -307,6 +307,7 @@ public: QByteArray wmCommand(); QByteArray wmClientMachine(bool use_localhost) const; const ClientMachine *clientMachine() const; + virtual bool isLocalhost() const; Window wmClientLeader() const; virtual pid_t pid() const; static bool resourceMatch(const Toplevel* c1, const Toplevel* c2); diff --git a/useractions.cpp b/useractions.cpp index 8eea2ebfcb..c5d7ec092f 100644 --- a/useractions.cpp +++ b/useractions.cpp @@ -471,14 +471,8 @@ void UserActionsMenu::menuAboutToShow() action->setText(i18n("&Extensions")); } - // disable rules for Wayland windows - dialog is X11 only - if (qobject_cast(m_client.data())) { - m_rulesOperation->setEnabled(false); - m_applicationRulesOperation->setEnabled(false); - } else { - m_rulesOperation->setEnabled(true); - m_applicationRulesOperation->setEnabled(true); - } + m_rulesOperation->setEnabled(true); + m_applicationRulesOperation->setEnabled(true); showHideActivityMenu(); }