[kcm/kwinrules] Fix size properties not being stored
ClosedPublic

Authored by iasensio on Thu, May 14, 10:01 PM.

Details

Summary

Use QSize/QPoint to handle and store coordinate values (size and position)

Previously, the rules model stored the "coordinate" type properties as a
QString with format x, y.

This fails when setting the properties to the config schema, as it requires
a proper QPoint or QSize value, specially the latter which can't be
convert from such a string.

BUG: 421055
FIXED-IN: 5.19.0

Test Plan
  • Add a new rule and set its position and size properties
  • Hitting apply stores the right values in ~\.config\kwinrulesrc
  • Close the kcm and reopen, the values are loaded
  • Property detection still works for size and position

Please note that there is a pre-existing bug of some position/sizes not being
applied to the windows in some cases, when using Apply Initially.
Better try using the Force policy.

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
iasensio created this revision.Thu, May 14, 10:01 PM
Restricted Application added a project: KWin. · View Herald TranscriptThu, May 14, 10:01 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
iasensio requested review of this revision.Thu, May 14, 10:01 PM
zzag accepted this revision.Fri, May 15, 5:48 AM
zzag added subscribers: yurchor, ltoscano, zzag.

@ltoscano @yurchor Would it be okay to push this change to Plasma/5.19?

kcmkwin/kwinrules/package/contents/ui/RulesEditor.qml
224

The context says "coordinates", but I think it needs to say "size (width, height)".

This revision is now accepted and ready to land.Fri, May 15, 5:48 AM
yurchor added inline comments.Fri, May 15, 6:30 AM
kcmkwin/kwinrules/package/contents/ui/RulesEditor.qml
224

No objections.

iasensio added inline comments.Fri, May 15, 1:16 PM
kcmkwin/kwinrules/package/contents/ui/RulesEditor.qml
224

I didn't want to add a string for 5.19, but since you agree I'll change it, thanks!

iasensio updated this revision to Diff 82940.Fri, May 15, 2:18 PM
  • Change string context
iasensio marked an inline comment as done.Fri, May 15, 2:51 PM
iasensio edited the summary of this revision. (Show Details)Fri, May 15, 5:26 PM
ngraham accepted this revision.Fri, May 15, 5:48 PM
This revision was automatically updated to reflect the committed changes.