Re-evaluate the window rules when the window class of a Client changes
ClosedPublic

Authored by graesslin on Nov 4 2018, 7:32 PM.

Details

Summary

So far KWin did not re-evaluate the window rules when the Client's
window class changes. Window class is the main (static) feature the rule
selection is based on. For dynamic changing mapping features like caption
KWin does re-evaluate the rules.

The reason for KWin to not evaluate when the class changes is that KWin
expects the class not to change. From ICCCM section 4.1.2.5:

> This property must be present when the window leaves the Withdrawn
> state and may be changed only while the window is in the Withdrawn
> state. Window managers may examine the property only when they start
> up and when the window leaves the Withdrawn state, but there should be
> no need for a client to change its state dynamically.

Unfortunately there are prominent applications such as Spotify which
violate this rule and do change the window class dynamically. While this
is a clear ICCCM violation there is nothing which really forbids it (may
not != must not) and nothing which forbids KWin to react on changes.

As also libtaskmanager started to react on it, it makes sense to also
hook up the required bits for window rules. After all KWin detects
changes to the window class for some time already and has the
functionality to evaluate the rules. So all there is, is one connect
which improves the situation for our users, while at the same time it
should be rather risk free. If a setup window rule breaks after this
change it's due to the client not being ICCCM compliant.

Test Plan

I don't use any of the affected applications, so it's only
tested with the new added unit test.

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.
graesslin created this revision.Nov 4 2018, 7:32 PM
Restricted Application added a project: KWin. · View Herald TranscriptNov 4 2018, 7:32 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
graesslin requested review of this revision.Nov 4 2018, 7:32 PM
zzag added a subscriber: zzag.Nov 4 2018, 9:48 PM
zzag added inline comments.
autotests/integration/window_rules_test.cpp
192–193

Do we need these events?

239

again

Is it a typo?

graesslin planned changes to this revision.Nov 5 2018, 8:13 AM

The connect should be done in Client::manage to prevent that initially setting the class triggers it.

autotests/integration/window_rules_test.cpp
239

No, copy and paste from other test. And probably a Germanism. It's how you would write it in German and just translated.

graesslin added inline comments.Nov 5 2018, 8:15 AM
autotests/integration/window_rules_test.cpp
192–193

Not sure. Copied from other effect which also seems to not need them. So I assumed we need them.

graesslin updated this revision to Diff 44886.Nov 5 2018, 9:00 AM

connect in manage after init of window class

zzag accepted this revision.Nov 5 2018, 9:13 AM
This revision is now accepted and ready to land.Nov 5 2018, 9:13 AM
This revision was automatically updated to reflect the committed changes.