From 8b9472e0bfcff8cb7467ee3055282a133a808349 Mon Sep 17 00:00:00 2001 From: Ismael Asensio Date: Fri, 19 Jun 2020 20:56:12 +0200 Subject: [PATCH 1/2] [kcm/kwinrules] Fix types property for NET::AllTypesMask When the user selects all of the types or none of them, the "types" property must be set to a special value (`NET::AllTypesMask = -1`), different than the sum of all the flags together. This re-implements this behaviour as the old KCM, fixing some heuristics that prevented finding the rule corresponding to the current window. The enum name that handles this property has been changed to `NetTypes` to make it more explicit. BUG: 423214 FIXED-IN: 5.19.3 TEST PLAN: 1. Open the `Application specific settings` on a window via menu 2. Select every "Window Type" (or none of them) 3. Give the rule a different name than the default and save 4. Open it again and check that the same rule is found --- kcmkwin/kwinrules/package/contents/ui/RulesEditor.qml | 2 +- kcmkwin/kwinrules/package/contents/ui/ValueEditor.qml | 7 ++++--- kcmkwin/kwinrules/ruleitem.cpp | 11 ++++++----- kcmkwin/kwinrules/ruleitem.h | 2 +- kcmkwin/kwinrules/rulesmodel.cpp | 4 ++-- 5 files changed, 14 insertions(+), 12 deletions(-) diff --git a/kcmkwin/kwinrules/package/contents/ui/RulesEditor.qml b/kcmkwin/kwinrules/package/contents/ui/RulesEditor.qml index 5c842f05ab..b118024229 100644 --- a/kcmkwin/kwinrules/package/contents/ui/RulesEditor.qml +++ b/kcmkwin/kwinrules/package/contents/ui/RulesEditor.qml @@ -225,7 +225,7 @@ ScrollViewKCM { return i18nc("Size (width, height)", "(%1, %2)", value.width, value.height); case RuleItem.Option: return options.textOfValue(value); - case RuleItem.FlagsOption: + case RuleItem.NetTypes: var selectedValue = value.toString(2).length - 1; return options.textOfValue(selectedValue); } diff --git a/kcmkwin/kwinrules/package/contents/ui/ValueEditor.qml b/kcmkwin/kwinrules/package/contents/ui/ValueEditor.qml index b0deab6928..9c858cc564 100644 --- a/kcmkwin/kwinrules/package/contents/ui/ValueEditor.qml +++ b/kcmkwin/kwinrules/package/contents/ui/ValueEditor.qml @@ -43,7 +43,7 @@ Loader { case RuleItem.String: return stringEditor case RuleItem.Integer: return integerEditor case RuleItem.Option: return optionEditor - case RuleItem.FlagsOption: return flagsEditor + case RuleItem.NetTypes: return netTypesEditor case RuleItem.Percentage: return percentageEditor case RuleItem.Point: return coordinateEditor case RuleItem.Size: return coordinateEditor @@ -113,12 +113,13 @@ Loader { } Component { - id: flagsEditor + id: netTypesEditor OptionsComboBox { flat: true model: ruleOptions multipleChoice: true - selectionMask: ruleValue + // If NET::AllTypesMask (-1), select everything + selectionMask: (ruleValue == -1) ? 0x3BF : ruleValue onActivated: { valueEditor.valueEdited(selectionMask); } diff --git a/kcmkwin/kwinrules/ruleitem.cpp b/kcmkwin/kwinrules/ruleitem.cpp index ff8047e71d..1c7f8a4eab 100644 --- a/kcmkwin/kwinrules/ruleitem.cpp +++ b/kcmkwin/kwinrules/ruleitem.cpp @@ -157,7 +157,7 @@ QVariant RuleItem::options() const void RuleItem::setOptionsData(const QList &data) { if (!m_options) { - if (m_type != Option && m_type != FlagsOption) { + if (m_type != Option && m_type != NetTypes) { return; } m_options = new OptionsModel(); @@ -202,12 +202,13 @@ QVariant RuleItem::typedValue(const QVariant &value, const RuleItem::Type type) case Integer: case Percentage: return value.toInt(); - case FlagsOption: - // HACK: Currently, the only user of this is "types" property - if (value.toInt() == -1) { //NET:AllTypesMask - return 0x3FF - 0x040; //All possible flags minus NET::Override (deprecated) + case NetTypes: { + const int usefulTypes = value.toInt() & 0x3BF; // remove NET::Override=0x040 (deprecated) + if (usefulTypes == 0 || usefulTypes == 0x3BF) { // if no flags or all of them are selected + return -1; // return NET:AllTypesMask } return value.toInt(); + } case Point: return value.toPoint(); case Size: diff --git a/kcmkwin/kwinrules/ruleitem.h b/kcmkwin/kwinrules/ruleitem.h index a8fe25ec23..c1c4399f7b 100644 --- a/kcmkwin/kwinrules/ruleitem.h +++ b/kcmkwin/kwinrules/ruleitem.h @@ -41,7 +41,7 @@ public: String, Integer, Option, - FlagsOption, + NetTypes, Percentage, Point, Size, diff --git a/kcmkwin/kwinrules/rulesmodel.cpp b/kcmkwin/kwinrules/rulesmodel.cpp index 0044837f1a..12dd4c775d 100644 --- a/kcmkwin/kwinrules/rulesmodel.cpp +++ b/kcmkwin/kwinrules/rulesmodel.cpp @@ -373,7 +373,7 @@ void RulesModel::populateRuleList() wmclasscomplete->setFlag(RuleItem::AlwaysEnabled); auto types = addRule(new RuleItem(QLatin1String("types"), - RulePolicy::NoPolicy, RuleItem::FlagsOption, + RulePolicy::NoPolicy, RuleItem::NetTypes, i18n("Window types"), i18n("Window matching"), QIcon::fromTheme("window-duplicate"))); types->setOptionsData(windowTypesModelData()); @@ -660,7 +660,7 @@ void RulesModel::setWindowProperties(const QVariantMap &info, bool forceValue) if (window_type == NET::Unknown) { window_type = NET::Normal; } - m_rules["types"]->setSuggestedValue(1 << window_type, forceValue); + m_rules["types"]->setSuggestedValue(1 << window_type); const QString wmsimpleclass = info.value("resourceClass").toString(); const QString wmcompleteclass = QStringLiteral("%1 %2").arg(info.value("resourceName").toString(), From ad3d2f5acf1e66276a5b7946f70cbbe96100d141 Mon Sep 17 00:00:00 2001 From: Ismael Asensio Date: Wed, 24 Jun 2020 19:38:30 +0200 Subject: [PATCH 2/2] Provide a mask for flag-type properties (window types) --- .../package/contents/ui/ValueEditor.qml | 4 +-- kcmkwin/kwinrules/ruleitem.cpp | 31 +++++++++++++------ kcmkwin/kwinrules/ruleitem.h | 6 ++-- kcmkwin/kwinrules/rulesmodel.cpp | 3 ++ kcmkwin/kwinrules/rulesmodel.h | 1 + 5 files changed, 31 insertions(+), 14 deletions(-) diff --git a/kcmkwin/kwinrules/package/contents/ui/ValueEditor.qml b/kcmkwin/kwinrules/package/contents/ui/ValueEditor.qml index 9c858cc564..7038f9bd28 100644 --- a/kcmkwin/kwinrules/package/contents/ui/ValueEditor.qml +++ b/kcmkwin/kwinrules/package/contents/ui/ValueEditor.qml @@ -118,8 +118,8 @@ Loader { flat: true model: ruleOptions multipleChoice: true - // If NET::AllTypesMask (-1), select everything - selectionMask: (ruleValue == -1) ? 0x3BF : ruleValue + // Filter the provided value with the options mask + selectionMask: ruleValue & optionsMask onActivated: { valueEditor.valueEdited(selectionMask); } diff --git a/kcmkwin/kwinrules/ruleitem.cpp b/kcmkwin/kwinrules/ruleitem.cpp index 1c7f8a4eab..03d92b27a8 100644 --- a/kcmkwin/kwinrules/ruleitem.cpp +++ b/kcmkwin/kwinrules/ruleitem.cpp @@ -41,6 +41,7 @@ RuleItem::RuleItem(const QString &key, , m_enabled(false) , m_policy(new RulePolicy(policyType)) , m_options(nullptr) + , m_optionsMask(0U - 1) { reset(); } @@ -54,7 +55,7 @@ RuleItem::~RuleItem() void RuleItem::reset() { m_enabled = hasFlag(AlwaysEnabled) | hasFlag(StartEnabled); - m_value = typedValue(QVariant(), m_type); + m_value = typedValue(QVariant()); m_suggestedValue = QVariant(); m_policy->resetValue(); if (m_options) { @@ -130,7 +131,7 @@ void RuleItem::setValue(QVariant value) if (m_options && m_type == Option) { m_options->setValue(value); } - m_value = typedValue(value, m_type); + m_value = typedValue(value); } QVariant RuleItem::suggestedValue() const @@ -143,7 +144,7 @@ void RuleItem::setSuggestedValue(QVariant value, bool forceValue) if (forceValue) { setValue(value); } - m_suggestedValue = value.isNull() ? QVariant() : typedValue(value, m_type); + m_suggestedValue = value.isNull() ? QVariant() : typedValue(value); } QVariant RuleItem::options() const @@ -164,6 +165,18 @@ void RuleItem::setOptionsData(const QList &data) } m_options->updateModelData(data); m_options->setValue(m_value); + + if (m_type == NetTypes) { + m_optionsMask = 0; + for (const OptionsModel::Data &dataItem : data) { + m_optionsMask += 1 << dataItem.value.toUInt(); + } + } +} + +uint RuleItem::optionsMask() const +{ + return m_optionsMask; } int RuleItem::policy() const @@ -191,9 +204,9 @@ QString RuleItem::policyKey() const return m_policy->policyKey(m_key); } -QVariant RuleItem::typedValue(const QVariant &value, const RuleItem::Type type) +QVariant RuleItem::typedValue(const QVariant &value) const { - switch (type) { + switch (type()) { case Undefined: case Option: return value; @@ -203,11 +216,11 @@ QVariant RuleItem::typedValue(const QVariant &value, const RuleItem::Type type) case Percentage: return value.toInt(); case NetTypes: { - const int usefulTypes = value.toInt() & 0x3BF; // remove NET::Override=0x040 (deprecated) - if (usefulTypes == 0 || usefulTypes == 0x3BF) { // if no flags or all of them are selected - return -1; // return NET:AllTypesMask + const uint typesMask = value.toUInt() & optionsMask(); // filter by the allowed mask in the model + if (typesMask == 0 || typesMask == optionsMask()) { // if no types or all of them are selected + return 0U - 1; // return an all active mask (NET:AllTypesMask) } - return value.toInt(); + return typesMask; } case Point: return value.toPoint(); diff --git a/kcmkwin/kwinrules/ruleitem.h b/kcmkwin/kwinrules/ruleitem.h index c1c4399f7b..cf12c899d7 100644 --- a/kcmkwin/kwinrules/ruleitem.h +++ b/kcmkwin/kwinrules/ruleitem.h @@ -91,6 +91,7 @@ public: QVariant options() const; void setOptionsData(const QList &data); + uint optionsMask() const; RulePolicy::Type policyType() const; int policy() const; // int belongs to anonymous enum in Rules:: @@ -98,12 +99,10 @@ public: QVariant policyModel() const; QString policyKey() const; - - void reset(); private: - static QVariant typedValue(const QVariant &value, const Type type); + QVariant typedValue(const QVariant &value) const; private: QString m_key; @@ -121,6 +120,7 @@ private: RulePolicy *m_policy; OptionsModel *m_options; + uint m_optionsMask; }; } //namespace diff --git a/kcmkwin/kwinrules/rulesmodel.cpp b/kcmkwin/kwinrules/rulesmodel.cpp index 12dd4c775d..a51a44a42c 100644 --- a/kcmkwin/kwinrules/rulesmodel.cpp +++ b/kcmkwin/kwinrules/rulesmodel.cpp @@ -71,6 +71,7 @@ QHash< int, QByteArray > RulesModel::roleNames() const {PolicyRole, QByteArrayLiteral("policy")}, {PolicyModelRole, QByteArrayLiteral("policyModel")}, {OptionsModelRole, QByteArrayLiteral("options")}, + {OptionsMaskRole, QByteArrayLiteral("optionsMask")}, {SuggestedValueRole, QByteArrayLiteral("suggested")}, }; } @@ -118,6 +119,8 @@ QVariant RulesModel::data(const QModelIndex &index, int role) const return rule->policyModel(); case OptionsModelRole: return rule->options(); + case OptionsMaskRole: + return rule->optionsMask(); case SuggestedValueRole: return rule->suggestedValue(); } diff --git a/kcmkwin/kwinrules/rulesmodel.h b/kcmkwin/kwinrules/rulesmodel.h index a62cff5525..ab9b27c848 100644 --- a/kcmkwin/kwinrules/rulesmodel.h +++ b/kcmkwin/kwinrules/rulesmodel.h @@ -60,6 +60,7 @@ public: PolicyRole, PolicyModelRole, OptionsModelRole, + OptionsMaskRole, SuggestedValueRole }; Q_ENUM(RulesRole)