Remove Client fullscreen hack
Needs ReviewPublic

Authored by romangg on Thu, Jan 10, 3:18 PM.

Details

Reviewers
None
Group Reviewers
KWin
Summary

This hack seems to be deactivated by default nowadays and I did not find an
option in the KCMs to activate it. Git blame shows some last commits from
around 2012 to the functions, but they only deal with code style.

Also even if it would be enabled, in light of Wayland we don't want some
roque XWayland client go bonkers via this hack.

This patch removes the code path in order to decrease complexity and ease
future changes to the fullscreen handling overall.

Test Plan

Manually and autotests pass.

Diff Detail

Repository
R108 KWin
Branch
fullscreenHackRemoval
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6947
Build 6965: arc lint + arc unit
romangg created this revision.Thu, Jan 10, 3:18 PM
Restricted Application added a project: KWin. · View Herald TranscriptThu, Jan 10, 3:18 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
romangg requested review of this revision.Thu, Jan 10, 3:18 PM
romangg planned changes to this revision.Thu, Jan 10, 4:24 PM

I think I can remove the stuff in Options as well without worries when I remove the corresponding kcfg key.

For safety I suggest to push after 5.15 has been branched off. That gives us more time to test that nobody still has that key around. Though it's unlikely.

For safety I suggest to push after 5.15 has been branched off. That gives us more time to test that nobody still has that key around. Though it's unlikely.

Thanks for the feedback. If you also think this hack can be removed, then I'm not so worried about removing used functionality.

Just a question: what would be the outcome of somebody still having the key around (in the KWin config file)? I assumed it would just mean that the key would be ignored when reading out the file, but nothing more.

romangg updated this revision to Diff 49222.Fri, Jan 11, 10:13 AM
  • Remove option entry completely
zzag added a subscriber: zzag.Fri, Jan 11, 10:27 AM

Please keep in mind that Options is exposed to scripting, though I doubt any script checks or sets legacyFullscreenSupport.

client.h
522

Unrelated change.

For safety I suggest to push after 5.15 has been branched off. That gives us more time to test that nobody still has that key around. Though it's unlikely.

Thanks for the feedback. If you also think this hack can be removed, then I'm not so worried about removing used functionality.

I was worried when I saw the change, to be honest. So I did a check that it's not set anywhere. Given that this option is old and all configuration broke with the switch to Qt 5 this is probably safe to remove. I doubt that anybody relies on it as it's impossible that configuration options did the migration from KDE 3 to Plasma 5.

Just a question: what would be the outcome of somebody still having the key around (in the KWin config file)? I assumed it would just mean that the key would be ignored when reading out the file, but nothing more.

Yes.