From 5e30fc609e7692129f444e4455d5b9300bfc23ff Mon Sep 17 00:00:00 2001
From: Eike Hein <hein@kde.org>
Date: Sat, 17 Sep 2016 22:53:34 +0900
Subject: [PATCH] Add an ultimate fallback to WM_CLASS Class for
 AbstractTasksModel::AppId in XWindowsTasksModel.

Summary:
This makes grouping work for apps we can't find on the system and
can't produce a launcher URL for, such as wine clients. Any better
app will use our superior codepath, the wonky ones like wine then
get left to making sure their WM_CLASS Class makes sense as per the
ICCCM spec.

Of course this doesn't work for XWayland clients on Wayland though.

BUG:368078

Also nixes an obsolete FIXME and adds a small check to avoid
producing launcher URLs that are only the window icon pixmap
query element.

Reviewers: #plasma, davidedmundson, broulik

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D2804
---
 libtaskmanager/taskgroupingproxymodel.cpp | 11 ++++++++++-
 libtaskmanager/xwindowtasksmodel.cpp      | 20 +++++++++++++++++++-
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/libtaskmanager/taskgroupingproxymodel.cpp b/libtaskmanager/taskgroupingproxymodel.cpp
index 26870a0b8..85e94e4a2 100644
--- a/libtaskmanager/taskgroupingproxymodel.cpp
+++ b/libtaskmanager/taskgroupingproxymodel.cpp
@@ -689,7 +689,16 @@ QVariant TaskGroupingProxyModel::data(const QModelIndex &proxyIndex, int role) c
     if (isGroup) {
         // For group parent items, DisplayRole is mapped to AppName of the first child.
         if (role == Qt::DisplayRole) {
-            return sourceIndex.data(AbstractTasksModel::AppName);
+            const QString &appName = sourceIndex.data(AbstractTasksModel::AppName).toString();
+
+            // Groups are formed by app id or launcher URL; neither requires
+            // AppName to be available. If it's not, fall back to the app id
+            /// rather than an empty string.
+            if (appName.isEmpty()) {
+                return sourceIndex.data(AbstractTasksModel::AppId);
+            }
+
+            return appName;
         } else if (role == AbstractTasksModel::LegacyWinIdList) {
             QVariantList winIds;
 
diff --git a/libtaskmanager/xwindowtasksmodel.cpp b/libtaskmanager/xwindowtasksmodel.cpp
index 437652411..682717b27 100644
--- a/libtaskmanager/xwindowtasksmodel.cpp
+++ b/libtaskmanager/xwindowtasksmodel.cpp
@@ -423,6 +423,21 @@ AppData XWindowTasksModel::Private::appData(WId window)
 {
     if (!appDataCache.contains(window)) {
         const AppData &data = appDataFromUrl(windowUrl(window));
+
+        // If we weren't able to derive a launcher URL from the window meta data,
+        // fall back to WM_CLASS Class string as app id. This helps with apps we
+        // can't map to an URL due to existing outside the regular system
+        // environment, e.g. wine clients.
+        if (data.id.isEmpty() && data.url.isEmpty()) {
+            AppData dataCopy = data;
+
+            dataCopy.id = windowInfo(window)->windowClassClass();
+
+            appDataCache.insert(window, dataCopy);
+
+            return dataCopy;
+        }
+
         appDataCache.insert(window, data);
 
         return data;
@@ -661,7 +676,10 @@ QUrl XWindowTasksModel::Private::launcherUrl(WId window, bool encodeFallbackIcon
 
     QUrl url = data.url;
 
-    // FIXME Hard-coding 64x64 or SizeLarge is not scaling-aware.
+    // Forego adding the window icon pixmap if the URL is otherwise empty.
+    if (!url.isValid()) {
+        return QUrl();
+    }
 
     const QPixmap pixmap = KWindowSystem::icon(window, KIconLoader::SizeLarge, KIconLoader::SizeLarge, false);
     if (pixmap.isNull()) {