[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    D5818: Lift app identification heuristic out of XWindowTasksModel and share it with WaylandTasksMode
From:       Eike Hein <noreply () phabricator ! kde ! org>
Date:       2017-05-11 21:33:16
Message-ID: differential-rev-PHID-DREV-r36cw5j5osddu4om73go-req () phabricator ! kde ! org
[Download RAW message or body]

hein created this revision.
Restricted Application added a project: Plasma.

REVISION SUMMARY
  This factors the app identification heuristic out of XWindowTasksModel
  and turns it into generic code in TaskTools, producing a URL from a set
  of window metadata bits. The key metadata is the 'appId', which is the
  classClass part of WM_CLASS on X11 and PlasmaWindow::appId on Wayland -
  which KWin sets to the former for XWayland clients. The result is much
  improved support for XWayland clients in the Wayland session, with most
  X clients now identified correctly.
  
  As a side effect, the Wayland model gains access to the X model's much
  superior code for grabbing a suitable icon, with PlasmaWindow::icon now
  serving only as a fallback, similar to KWindowSystem::icon in the X
  model.
  
  Moving the code to TaskTools also means it now sports nice API docs.
  
  The heuristic has seen some work as well, namely adding two passes that
  try to parse the appId as a path to a desktop file, which we've never
  seen on X11 but is common on Wayland - this heuristic was previously in
  appDataFromAppId, which has been removed here since the shared heuristic
  now satisfies this case.
  
  Further, an old codepath handling kcmshell in a special way has been
  removed. This is no longer necessary as we have better ways to tell
  libtaskmanager about the KCM KService now, such as the .desktop file
  window hint on X11 and a reliable appId on Wayland.
  
  This patch also fixes some bugs around app data cache eviction and
  telling model clients about data changes when cache eviction happens.
  The X model didn't use to evict the cache and refresh when the
  taskmanagerrulesrc file was changed at runtime, and the refresh for
  sycoca changes didn't refresh the LauncherUrlWithoutIcon role. It does
  now, and the Wayland model - which has gained taskmanagerrulesrc support
  by way of the shared heuristic - now behaves in the same way.
  
  The combined changes achieve near behavior parity between the X
  and Wayland models when it comes to identifying apps by window meta
  data, with the only exception being XWayland clients that need to be
  identified by the (incorrectly used by the client developer) instance
  name part of the WM_CLASS window property, which we can't access (this
  case is so rare it's not worth handling at this time).
  
  Depends on https://phabricator.kde.org/D5747, https://phabricator.kde.org/D5755.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  pid (branched from master)

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

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

To: hein, #plasma, davidedmundson
Cc: plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, \
jensreuterberg, abetts, sebas, apol, lukas


[Attachment #3 (text/html)]

<table><tr><td style="">hein created this revision.<br />Restricted Application added \
a project: Plasma. </td><a style="text-decoration: none; padding: 4px 8px; margin: 0 \
8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; \
background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); \
display: inline-block; border: 1px solid rgba(71,87,120,.2);" \
href="https://phabricator.kde.org/D5818" rel="noreferrer">View \
Revision</a></tr></table><br /><div><strong>REVISION SUMMARY</strong><div><p>This \
factors the app identification heuristic out of XWindowTasksModel<br /> and turns it \
into generic code in TaskTools, producing a URL from a set<br /> of window metadata \
bits. The key metadata is the &#039;appId&#039;, which is the<br /> classClass part \
of WM_CLASS on X11 and PlasmaWindow::appId on Wayland -<br /> which KWin sets to the \
former for XWayland clients. The result is much<br /> improved support for XWayland \
clients in the Wayland session, with most<br /> X clients now identified \
correctly.</p>

<p>As a side effect, the Wayland model gains access to the X model&#039;s much<br />
superior code for grabbing a suitable icon, with PlasmaWindow::icon now<br />
serving only as a fallback, similar to KWindowSystem::icon in the X<br />
model.</p>

<p>Moving the code to TaskTools also means it now sports nice API docs.</p>

<p>The heuristic has seen some work as well, namely adding two passes that<br />
try to parse the appId as a path to a desktop file, which we&#039;ve never<br />
seen on X11 but is common on Wayland - this heuristic was previously in<br />
appDataFromAppId, which has been removed here since the shared heuristic<br />
now satisfies this case.</p>

<p>Further, an old codepath handling kcmshell in a special way has been<br />
removed. This is no longer necessary as we have better ways to tell<br />
libtaskmanager about the KCM KService now, such as the .desktop file<br />
window hint on X11 and a reliable appId on Wayland.</p>

<p>This patch also fixes some bugs around app data cache eviction and<br />
telling model clients about data changes when cache eviction happens.<br />
The X model didn&#039;t use to evict the cache and refresh when the<br />
taskmanagerrulesrc file was changed at runtime, and the refresh for<br />
sycoca changes didn&#039;t refresh the LauncherUrlWithoutIcon role. It does<br />
now, and the Wayland model - which has gained taskmanagerrulesrc support<br />
by way of the shared heuristic - now behaves in the same way.</p>

<p>The combined changes achieve near behavior parity between the X<br />
and Wayland models when it comes to identifying apps by window meta<br />
data, with the only exception being XWayland clients that need to be<br />
identified by the (incorrectly used by the client developer) instance<br />
name part of the WM_CLASS window property, which we can&#039;t access (this<br />
case is so rare it&#039;s not worth handling at this time).</p>

<p>Depends on <a href="https://phabricator.kde.org/D5747" style="background-color: \
#e7e7e7;  border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;" rel="noreferrer">D5747</a>, <a \
href="https://phabricator.kde.org/D5755" style="background-color: #e7e7e7;  \
border-color: #e7e7e7;  border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;" \
rel="noreferrer">D5755</a>.</p></div></div><br \
/><div><strong>REPOSITORY</strong><div><div>R120 Plasma \
Workspace</div></div></div><br /><div><strong>BRANCH</strong><div><div>pid (branched \
from master)</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a \
href="https://phabricator.kde.org/D5818" \
rel="noreferrer">https://phabricator.kde.org/D5818</a></div></div><br \
/><div><strong>AFFECTED FILES</strong><div><div>libtaskmanager/tasktools.cpp<br /> \
libtaskmanager/tasktools.h<br /> libtaskmanager/waylandtasksmodel.cpp<br />
libtaskmanager/xwindowtasksmodel.cpp</div></div></div><br /><div><strong>To: \
</strong>hein, Plasma, davidedmundson<br /><strong>Cc: </strong>plasma-devel, \
ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, \
apol, lukas<br /></div>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic