Bug 367554 - Special Window Settings are written with lowercased WM_WINDOW_ROLE
Summary: Special Window Settings are written with lowercased WM_WINDOW_ROLE
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: rules (other bugs)
Version First Reported In: 5.5.5
Platform: openSUSE Linux
: NOR normal
Target Milestone: ---
Assignee: KWin default assignee
URL: https://phabricator.kde.org/D2574
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-19 10:02 UTC by Tristan Miller
Modified: 2016-09-13 06:37 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:
mgraesslin: ReviewRequest+


Attachments
Output of xprop on the KPatience window (36.59 KB, text/plain)
2016-08-20 21:14 UTC, Tristan Miller
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tristan Miller 2016-08-19 10:02:44 UTC
Changes made in the "Special Window Settings" dialog never take effect.  (For settings that are supposed to be persistent, an entry gets created in the Window Rules section of the Window Manager Settings, and the correct settings are listed there, but they never actually get applied.)  On the other hand, "Special Application Settings" seems to work as expected.

Reproducible: Always

Steps to Reproduce:
1. Launch some program.
2. Right-click on the window title bar and select More Actions->Special Window Settings
3. In the "Size & Position Tab", check "Maximized vertically"/"Apply Now"/"Yes" and hit OK.
4. Repeat Step 2.
5. Then in the "Size & Position Tab", check "Maximized vertically"/"Apply Initially"/"Yes" and hit OK.
6. Close the program, and then re-launch it.

Actual Results:  
3a. Nothing happens.
6a. The program launches but the window is not maximized vertically.

Expected Results:  
3a. The window should immediately maximize vertically.
6a. When the program launches, its window should be maximized vertically.

I have tried this with several different programs (KPatience, Kopete, etc.) and always get the same result.
Comment 1 Martin Flöser 2016-08-19 12:18:21 UTC
can you please upload a rule which does not work?
Comment 2 Tristan Miller 2016-08-19 13:24:32 UTC
Sure, here's the "Apply Initially" variant of the rule mentioned in my original report:

[Window settings for kpat]
Description=Window settings for kpat
clientmachine=localhost
clientmachinematch=0
maximizevert=true
maximizevertrule=3
title=KPatience
titlematch=0
types=1
windowrole=mainwindow
windowrolematch=1
wmclass=kpat
wmclasscomplete=false
wmclassmatch=1
Comment 3 Thomas Lübking 2016-08-20 13:37:34 UTC
Smells related to bug #343709 - "mainwindow" is probably really "MainWindow" or similar.
Please attach the ouput of "xprop" on a window you want to control this way.
Comment 4 Tristan Miller 2016-08-20 21:14:00 UTC
Created attachment 100696 [details]
Output of xprop on the KPatience window

As requested, attached is the output of xprop on the KPatience window used in my previous comments.
Comment 5 Thomas Lübking 2016-08-21 07:59:07 UTC
> WM_WINDOW_ROLE(STRING) = "MainWindow"
Bingo. The rule needs to be written case sesitively.
Comment 6 Tristan Miller 2016-08-21 09:47:55 UTC
Wasn't this supposed to be fixed with Bug 343709?
Comment 7 Thomas Lübking 2016-08-21 15:16:49 UTC
No. The role was originally handled lowercased in kwin. This was implicitly changed by no more keeping the value internally but relying on the kwindowsystem value. As a result, the rule now matches case sensitively. All fine, but the rule is still *written* lowercased (at least in this context)
Comment 8 Martin Flöser 2016-08-24 07:34:33 UTC
@Tristan: can you try to change the windowrole to MainWindow in the rule config?
Comment 9 Tristan Miller 2016-08-24 12:34:24 UTC
Yes, the settings are applied correctly if I change the capitalization to MainWindow.
Comment 10 Martin Flöser 2016-08-24 14:21:25 UTC
awesome, so that seems to work then.
Comment 11 Tristan Miller 2016-08-24 15:37:45 UTC
Why WORKSFORME?  Surely manually changing every rule isn't an acceptable workaround.  Or are you saying that the fix to Bug 343709 fixes this issue after all?
Comment 12 Martin Flöser 2016-08-25 08:56:26 UTC
sorry, you are right. Existing rules should also be supported.
Comment 13 Martin Flöser 2016-08-25 11:25:23 UTC
Git commit 6e8a8913d1b4d6a95a2c2b74612d57ba1cce37c7 by Martin Gräßlin.
Committed on 25/08/2016 at 11:23.
Pushed by graesslin into branch 'master'.

[autotests/integration] Add test case for rule matching on window role

New test infrastructure which supports testing window rules at runtime.
Test exposes problem of window rules not able to match window roles in
a case insensitive manner.

M  +1    -0    autotests/integration/CMakeLists.txt
A  +13   -0    autotests/integration/data/rules/maximize-vert-apply-initial
A  +172  -0    autotests/integration/window_rules_test.cpp     [License: GPL (v2)]
M  +1    -1    rules.h

http://commits.kde.org/kwin/6e8a8913d1b4d6a95a2c2b74612d57ba1cce37c7
Comment 14 Martin Flöser 2016-08-25 11:34:09 UTC
Possible fix at https://phabricator.kde.org/D2574
Comment 15 Martin Flöser 2016-09-13 06:37:46 UTC
Git commit 97b594501a439f4e5ee5f4237f253fb84087c423 by Martin Gräßlin.
Committed on 13/09/2016 at 06:37.
Pushed by graesslin into branch 'master'.

Match window role in Rules in a case insensitive manner

Summary:
We used to have a toLower when reading the rule. This was removed with
4f7edb8 which turned it into a case sensitive matching to fix a
regression.

But this created another regression: existing rules written lower case
are no longer matched.

This change makes the role matching case insensitive again.

Reviewers: #kwin

Subscribers: kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D2574

M  +0    -1    autotests/integration/window_rules_test.cpp
M  +2    -2    rules.cpp

http://commits.kde.org/kwin/97b594501a439f4e5ee5f4237f253fb84087c423