D15410: Handle clients which change window metadata during early startup

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:b15eaf38b6bf: Handle clients which change window metadata 
during early startup (authored by hein).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15410?vs=41356&id=41487

REVISION DETAIL
  https://phabricator.kde.org/D15410

AFFECTED FILES
  libtaskmanager/taskmanagerrulesrc
  libtaskmanager/tasksmodel.cpp
  libtaskmanager/tasktools.cpp
  libtaskmanager/tasktools.h
  libtaskmanager/waylandtasksmodel.cpp
  libtaskmanager/xwindowtasksmodel.cpp

To: hein, davidedmundson, broulik, ngraham
Cc: graesslin, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15410: Handle clients which change window metadata during early startup

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Seems to work with LO 5.1.6 for me!

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D15410

To: hein, davidedmundson, broulik, ngraham
Cc: graesslin, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15410: Handle clients which change window metadata during early startup

[Eike Hein]

hein added a comment.


  In D15410#323613 , @graesslin 
wrote:
  
  > I just want to point out that such applications are not ICCCM compliant: 
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.
  >
  > KWin doesn't support updates to wm_class and e.g. window rules are broken.
  
  
  Preaching to the choir I'm afraid :(. I consider this bad application 
behavior and called it a concession to badly written apps on Bugzilla.
  
  Unfortunately the upstream bug report has been ignored however, and there's a 
large installed base as this is long-standing LibreOffice behavior. I am 
eventually considering a backport to the LTS branch of this.
  
  Also, on Wayland the appid can change at runtime in theory (and people hooked 
this up in Mutter and Weston, so I guess it's been encountered), and the 
affected code is mostly generic over the two windowing systems.
  
  Ah well. At least this takes the shape of a generic "allow hiding tasks 
matching some id" feature. It's in practice only needed to handle this app 
misbehavior, but at least it's not brittle weird code to catch ckass changes 
explicitly.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15410

To: hein, davidedmundson, broulik, ngraham
Cc: graesslin, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15410: Handle clients which change window metadata during early startup

[Martin Flöser]

graesslin added a comment.


  I just want to point out that such applications are not ICCCM compliant: 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.
  
  KWin doesn't support updates to wm_class and e.g. window rules are broken.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15410

To: hein, davidedmundson, broulik, ngraham
Cc: graesslin, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15410: Handle clients which change window metadata during early startup

[Eike Hein]

hein updated this revision to Diff 41356.
hein edited the summary of this revision.
hein added a comment.


  Add conclusion to description. Done now, promise.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15410?vs=41355&id=41356

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D15410

AFFECTED FILES
  libtaskmanager/taskmanagerrulesrc
  libtaskmanager/tasksmodel.cpp
  libtaskmanager/tasktools.cpp
  libtaskmanager/tasktools.h
  libtaskmanager/waylandtasksmodel.cpp
  libtaskmanager/xwindowtasksmodel.cpp

To: hein, davidedmundson, broulik, ngraham
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15410: Handle clients which change window metadata during early startup

[Eike Hein]

hein updated this revision to Diff 41355.
hein added a reviewer: ngraham.
hein added a comment.


  Add back Nate. Now I'm committed to using `arc diff --verbatim`, sorry. :P

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15410?vs=41354&id=41355

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D15410

AFFECTED FILES
  libtaskmanager/taskmanagerrulesrc
  libtaskmanager/tasksmodel.cpp
  libtaskmanager/tasktools.cpp
  libtaskmanager/tasktools.h
  libtaskmanager/waylandtasksmodel.cpp
  libtaskmanager/xwindowtasksmodel.cpp

To: hein, davidedmundson, broulik, ngraham
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15410: Handle clients which change window metadata during early startup

[Eike Hein]

hein updated this revision to Diff 41354.
hein edited the summary of this revision.
hein added a comment.


  Improve description further.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15410?vs=41353&id=41354

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D15410

AFFECTED FILES
  libtaskmanager/taskmanagerrulesrc
  libtaskmanager/tasksmodel.cpp
  libtaskmanager/tasktools.cpp
  libtaskmanager/tasktools.h
  libtaskmanager/waylandtasksmodel.cpp
  libtaskmanager/xwindowtasksmodel.cpp

To: hein, davidedmundson, broulik
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15410: Handle clients which change window metadata during early startup

[Eike Hein]

hein updated this revision to Diff 41353.
hein edited the summary of this revision.
hein removed a reviewer: ngraham.
hein added a comment.


  Fix typo in description.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15410?vs=41352&id=41353

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D15410

AFFECTED FILES
  libtaskmanager/taskmanagerrulesrc
  libtaskmanager/tasksmodel.cpp
  libtaskmanager/tasktools.cpp
  libtaskmanager/tasktools.h
  libtaskmanager/waylandtasksmodel.cpp
  libtaskmanager/xwindowtasksmodel.cpp

To: hein, davidedmundson, broulik, ngraham
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15410: Handle clients which change window metadata during early startup

[Eike Hein]

hein created this revision.
hein added reviewers: davidedmundson, broulik.
Herald added a project: Plasma.
hein requested review of this revision.

REVISION SUMMARY
  Some apps initially show their window with bogus/useless window
  metadata and then update to useful metadata during early startup.
  For example, LibreOffice sets WM_CLASS to soffice/Soffice and
  then updates to libreoffice-writer/libreoffice. This leads to
  a poor user experience on particular the Icons-only Task Manager,
  but also the regular Task Manager depending on settings.
  
  Depending on its configuration (and Icons-only Task Manager is
  a particular set of configuration options, as far as the model
  is concerned), TasksModel will try to launch a new window task
  adjacent to its launcher task. The appearance of a new window
  task also causes matching (in terms of identification) launcher
  or startup tasks to be filtered out. To the user, this forms a
  lifecycle of the launcher being replaced by the window in-place
  (and a startup state inbetween, optionally but by default).
  
  Prior to this patch, this sorting decision was only done once,
  when a new window enters the source model stack. That meant the
  LibreOffice window would initially be sorted into the "wrong"
  spot (the bogus metadata doesn't allow us to relate it to its
  launcher) and then, following the metadata change, stick to the
  wrong position.
  
  Simply changing the code to sort things again on any metadata
  change would not have been good enough: Metadata changes can
  occur at any time, and things shouldn't move around on the user
  
  - this sort mode is called "Manual" for a reason. Also, the
  
  visual result would still be poor: The window would initially
  appear at the wrong position, then move into the right one a
  short moment later.
  
  This patch takes the following approach:
  
  - It adds a new config key to taskmanagerrulesrc that allows listing ids used 
to completely hide tasks if they match, and of course the code needed to 
implement this.
  - It adds LibreOffice' bogus initial metadata to this key, so the tasks is 
initially hidden.
  - It skips over hidden tasks in the sort insert queue instead of moving them.
  - It resorts when tasks are unhidden (i.e. once the metadata update has 
occured and the task no longer matches the above config key).
  
  BUG:396871

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D15410

AFFECTED FILES
  libtaskmanager/taskmanagerrulesrc
  libtaskmanager/tasksmodel.cpp
  libtaskmanager/tasktools.cpp
  libtaskmanager/tasktools.h
  libtaskmanager/waylandtasksmodel.cpp
  libtaskmanager/xwindowtasksmodel.cpp

To: hein, davidedmundson, broulik
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart