From b15eaf38b6bf8d5af7fdc0caff05679a063819cf Mon Sep 17 00:00:00 2001 From: Eike Hein Date: Tue, 11 Sep 2018 00:45:01 +0900 Subject: [PATCH] Handle clients which change window metadata during early startup 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 sort 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. This 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 should not just 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 to the right one a bit later. This patch takes the following approach: * It adds a new config key to taskmanagerrulesrc for listing ids of matching tasks to completely hide, and of course the code needed to implement this. * It adds LibreOffice' bogus initial metadata to this key, so the tasks is initially hidden. * The sort code skips over hidden window tasks in the sort insert queue instead of moving them. The queue is marked as stale then, and cleared on unrelated windowing system changes. * It resorts when tasks are unhidden (i.e. once the metadata update has occured and the task no longer matches the above config key). The visual result is that the startup notification on the launcher spins a little bit longer than before, even though the window has already appeared (although LO lags in filling in its contents anyway), and then morphs into the window task representation once the client has completed the window metadata change. This happens in such a short order as to be more or less imperceptible. If startup notifications are turned off it's broadly the same, minus the spinning. BUG:396871 Reviewers: davidedmundson, broulik, ngraham Subscribers: plasma-devel Tags: #plasma Differential Revision: https://phabricator.kde.org/D15410 --- libtaskmanager/taskmanagerrulesrc | 1 + libtaskmanager/tasksmodel.cpp | 157 ++++++++++++++++++--------- libtaskmanager/tasktools.cpp | 28 ++++- libtaskmanager/tasktools.h | 1 + libtaskmanager/waylandtasksmodel.cpp | 7 +- libtaskmanager/xwindowtasksmodel.cpp | 9 +- 6 files changed, 144 insertions(+), 59 deletions(-) diff --git a/libtaskmanager/taskmanagerrulesrc b/libtaskmanager/taskmanagerrulesrc index fb27df5ba..ded9eafbe 100644 --- a/libtaskmanager/taskmanagerrulesrc +++ b/libtaskmanager/taskmanagerrulesrc @@ -11,3 +11,4 @@ VirtualBox Machine=virtualbox [Settings] MatchCommandLineFirst=perl TryIgnoreRuntimes=perl +SkipTaskbar=Soffice diff --git a/libtaskmanager/tasksmodel.cpp b/libtaskmanager/tasksmodel.cpp index c881369cb..c6a6b8d29 100644 --- a/libtaskmanager/tasksmodel.cpp +++ b/libtaskmanager/tasksmodel.cpp @@ -73,6 +73,7 @@ public: bool launcherCheckNeeded = false; QList sortedPreFilterRows; QVector sortRowInsertQueue; + bool sortRowInsertQueueStale = false; QHash activityTaskCounts; static ActivityInfo* activityInfo; static int activityInfoUsers; @@ -191,6 +192,18 @@ void TasksModel::Private::initModels() if (roles.contains(AbstractTasksModel::IsActive)) { emit q->activeTaskChanged(); } + + // In manual sort mode, updateManualSortMap() may consult the sortRowInsertQueue + // for new tasks to sort in. Hidden tasks remain in the queue to potentially sort + // them later, when they are are actually revealed to the user. + // This is particularly useful in concert with taskmanagerrulesrc's SkipTaskbar + // key, which is used to hide window tasks which update from bogus to useful + // window metadata early in startup. The role change then coincides with positive + // app identication, which is when updateManualSortMap() becomes able to sort the + // task adjacent to its launcher when required to do so. + if (sortMode == SortManual && roles.contains(AbstractTasksModel::SkipTaskbar)) { + updateManualSortMap(); + } } ); @@ -228,6 +241,11 @@ void TasksModel::Private::initModels() sortedPreFilterRows.append(i); if (!separateLaunchers) { + if (sortRowInsertQueueStale) { + sortRowInsertQueue.clear(); + sortRowInsertQueueStale = false; + } + sortRowInsertQueue.append(sortedPreFilterRows.count() - 1); } } @@ -256,6 +274,11 @@ void TasksModel::Private::initModels() return; } + if (sortRowInsertQueueStale) { + sortRowInsertQueue.clear(); + sortRowInsertQueueStale = false; + } + for (int i = first; i <= last; ++i) { sortedPreFilterRows.removeOne(i); } @@ -539,71 +562,101 @@ void TasksModel::Private::updateManualSortMap() // Otherwise process any entries in the insert queue and move them intelligently // in the sort map. } else { - while (sortRowInsertQueue.count()) { - const int row = sortRowInsertQueue.takeFirst(); - const QModelIndex &idx = concatProxyModel->index(sortedPreFilterRows.at(row), 0); + QMutableVectorIterator i(sortRowInsertQueue); + + while (i.hasNext()) { + i.next(); + + const int row = i.value(); + const QModelIndex &idx = concatProxyModel->index(sortedPreFilterRows.at(row), 0); + + // If a window task is currently hidden, we may want to keep it in the queue + // to sort it in later once it gets revealed. + // This is important in concert with taskmanagerrulesrc's SkipTaskbar key, which + // is used to hide window tasks which update from bogus to useful window metadata + // early in startup. Once the task no longer uses bogus metadata listed in the + // config key, its SkipTaskbar role changes to false, and then is it possible to + // sort the task adjacent to its launcher in the code below. + if (idx.data(AbstractTasksModel::IsWindow).toBool() && idx.data(AbstractTasksModel::SkipTaskbar).toBool()) { + // Since we're going to keep a row in the queue for now, make sure to + // mark the queue as stale so it's cleared on appends or row removals + // when they follow this sorting attempt. This frees us from having to + // update the indices in the queue to keep them valid. + // This means windowing system changes such as the opening or closing + // of a window task which happen during the time period that a window + // task has known bogus metadata, can upset what we're trying to + // achieve with this exception. However, due to the briefness of the + // time period and usage patterns, this is improbable, making this + // likely good enough. If it turns out not to be, this decision may be + // revisited later. + sortRowInsertQueueStale = true; + + break; + } else { + i.remove(); + } - bool moved = false; + bool moved = false; - // Try to move the task up to its right-most app sibling, unless this - // is us sorting in a launcher list for the first time. - if (launchersEverSet && !idx.data(AbstractTasksModel::IsLauncher).toBool()) { - for (int i = (row - 1); i >= 0; --i) { - const QModelIndex &concatProxyIndex = concatProxyModel->index(sortedPreFilterRows.at(i), 0); + // Try to move the task up to its right-most app sibling, unless this + // is us sorting in a launcher list for the first time. + if (launchersEverSet && !idx.data(AbstractTasksModel::IsLauncher).toBool()) { + for (int i = (row - 1); i >= 0; --i) { + const QModelIndex &concatProxyIndex = concatProxyModel->index(sortedPreFilterRows.at(i), 0); - if (appsMatch(concatProxyIndex, idx)) { - // Our sort map contains row indices prior to any filtering, but we don't - // want to sort new tasks in next to siblings we're filtering out higher up - // in the proxy chain, so check in with the filter model. - const QModelIndex &filterProxyIndex = filterProxyModel->mapFromSource(concatProxyIndex); + if (appsMatch(concatProxyIndex, idx)) { + // Our sort map contains row indices prior to any filtering, but we don't + // want to sort new tasks in next to siblings we're filtering out higher up + // in the proxy chain, so check in with the filter model. + const QModelIndex &filterProxyIndex = filterProxyModel->mapFromSource(concatProxyIndex); - if (filterProxyIndex.isValid()) { + if (filterProxyIndex.isValid()) { sortedPreFilterRows.move(row, i + 1); moved = true; - } + } - break; - } - } - } + break; + } + } + } - int insertPos = 0; + int insertPos = 0; - // If unsuccessful or skipped, and the new task is a launcher, put after - // the rightmost launcher or launcher-backed task in the map, or failing - // that at the start of the map. - if (!moved && idx.data(AbstractTasksModel::IsLauncher).toBool()) { - for (int i = 0; i < row; ++i) { - const QModelIndex &concatProxyIndex = concatProxyModel->index(sortedPreFilterRows.at(i), 0); + // If unsuccessful or skipped, and the new task is a launcher, put after + // the rightmost launcher or launcher-backed task in the map, or failing + // that at the start of the map. + if (!moved && idx.data(AbstractTasksModel::IsLauncher).toBool()) { + for (int i = 0; i < row; ++i) { + const QModelIndex &concatProxyIndex = concatProxyModel->index(sortedPreFilterRows.at(i), 0); - if (concatProxyIndex.data(AbstractTasksModel::IsLauncher).toBool() + if (concatProxyIndex.data(AbstractTasksModel::IsLauncher).toBool() || launcherTasksModel->launcherPosition(concatProxyIndex.data(AbstractTasksModel::LauncherUrlWithoutIcon).toUrl()) != -1) { insertPos = i + 1; - } else { + } else { break; - } - } - - sortedPreFilterRows.move(row, insertPos); - moved = true; - } - - // If we sorted in a launcher and it's the first time we're sorting in a - // launcher list, move existing windows to the launcher position now. - if (moved && !launchersEverSet) { - for (int i = (sortedPreFilterRows.count() - 1); i >= 0; --i) { - const QModelIndex &concatProxyIndex = concatProxyModel->index(sortedPreFilterRows.at(i), 0); - - if (!concatProxyIndex.data(AbstractTasksModel::IsLauncher).toBool() - && idx.data(AbstractTasksModel::LauncherUrlWithoutIcon) == concatProxyIndex.data(AbstractTasksModel::LauncherUrlWithoutIcon)) { - sortedPreFilterRows.move(i, insertPos); - - if (insertPos > i) { - --insertPos; - } - } - } - } + } + } + + sortedPreFilterRows.move(row, insertPos); + moved = true; + } + + // If we sorted in a launcher and it's the first time we're sorting in a + // launcher list, move existing windows to the launcher position now. + if (moved && !launchersEverSet) { + for (int i = (sortedPreFilterRows.count() - 1); i >= 0; --i) { + const QModelIndex &concatProxyIndex = concatProxyModel->index(sortedPreFilterRows.at(i), 0); + + if (!concatProxyIndex.data(AbstractTasksModel::IsLauncher).toBool() + && idx.data(AbstractTasksModel::LauncherUrlWithoutIcon) == concatProxyIndex.data(AbstractTasksModel::LauncherUrlWithoutIcon)) { + sortedPreFilterRows.move(i, insertPos); + + if (insertPos > i) { + --insertPos; + } + } + } + } } } } diff --git a/libtaskmanager/tasktools.cpp b/libtaskmanager/tasktools.cpp index 6be094e24..090981eb9 100644 --- a/libtaskmanager/tasktools.cpp +++ b/libtaskmanager/tasktools.cpp @@ -65,6 +65,11 @@ AppData appDataFromUrl(const QUrl &url, const QIcon &fallbackIcon) pixmap.loadFromData(bytes); data.icon.addPixmap(pixmap); } + + if (uQuery.hasQueryItem(QLatin1String("skipTaskbar"))) { + QString skipTaskbar(uQuery.queryItemValue(QLatin1String("skipTaskbar"))); + data.skipTaskbar = (skipTaskbar == QStringLiteral("true")); + } } // applications: URLs are used to refer to applications by their KService::menuId @@ -405,6 +410,23 @@ QUrl windowUrlFromMetadata(const QString &appId, quint32 pid, services = KServiceTypeTrader::self()->query(QStringLiteral("Application"), QStringLiteral("exist Exec and ('%1' =~ Name) and (not exist NoDisplay or not NoDisplay)").arg(appId)); sortServicesByMenuId(services, appId); } + + // Check rules configuration for whether we want to hide this task. + // Some window tasks update from bogus to useful metadata early during startup. + // This config key allows listing the bogus metadata, and the matching window + // tasks are hidden until they perform a metadate update that stops them from + // matching. + QStringList skipTaskbar = set.readEntry("SkipTaskbar", QStringList()); + + if (skipTaskbar.contains(appId)) { + QUrlQuery query(url); + query.addQueryItem(QStringLiteral("skipTaskbar"), QStringLiteral("true")); + url.setQuery(query); + } else if (skipTaskbar.contains(mapped)) { + QUrlQuery query(url); + query.addQueryItem(QStringLiteral("skipTaskbar"), QStringLiteral("true")); + url.setQuery(query); + } } // Ok, absolute *last* chance, try matching via pid (but only if we have not already tried this!) ... @@ -451,7 +473,8 @@ QUrl windowUrlFromMetadata(const QString &appId, quint32 pid, // applications: URLs are used to refer to applications by their KService::menuId // (i.e. .desktop file name) rather than the absolute path to a .desktop file. if (!menuId.isEmpty()) { - return QUrl(QStringLiteral("applications:") + menuId); + url.setUrl(QStringLiteral("applications:") + menuId); + return url; } QString path = services.at(0)->entryPath(); @@ -461,7 +484,10 @@ QUrl windowUrlFromMetadata(const QString &appId, quint32 pid, } if (!path.isEmpty()) { + QString query = url.query(); url = QUrl::fromLocalFile(path); + url.setQuery(query); + return url; } } diff --git a/libtaskmanager/tasktools.h b/libtaskmanager/tasktools.h index b8b8ec79a..45e8d0c3c 100644 --- a/libtaskmanager/tasktools.h +++ b/libtaskmanager/tasktools.h @@ -40,6 +40,7 @@ struct AppData QString genericName; // Generic application name. QIcon icon; QUrl url; + bool skipTaskbar = false; }; enum UrlComparisonMode { diff --git a/libtaskmanager/waylandtasksmodel.cpp b/libtaskmanager/waylandtasksmodel.cpp index 73c3d6e66..ffc236784 100644 --- a/libtaskmanager/waylandtasksmodel.cpp +++ b/libtaskmanager/waylandtasksmodel.cpp @@ -86,7 +86,8 @@ void WaylandTasksModel::Private::init() QVector{Qt::DecorationRole, AbstractTasksModel::AppId, AbstractTasksModel::AppName, AbstractTasksModel::GenericName, AbstractTasksModel::LauncherUrl, - AbstractTasksModel::LauncherUrlWithoutIcon}); + AbstractTasksModel::LauncherUrlWithoutIcon, + AbstractTasksModel::SkipTaskbar}); }; rulesConfig = KSharedConfig::openConfig(QStringLiteral("taskmanagerrulesrc")); @@ -203,7 +204,7 @@ void WaylandTasksModel::Private::addWindow(KWayland::Client::PlasmaWindow *windo // Refresh roles satisfied from the app data cache. this->dataChanged(window, QVector{AppId, AppName, GenericName, - LauncherUrl, LauncherUrlWithoutIcon}); + LauncherUrl, LauncherUrlWithoutIcon, SkipTaskbar}); } ); @@ -405,7 +406,7 @@ QVariant WaylandTasksModel::data(const QModelIndex &index, int role) const } else if (role == IsDemandingAttention) { return window->isDemandingAttention(); } else if (role == SkipTaskbar) { - return window->skipTaskbar(); + return window->skipTaskbar() || d->appData(window).skipTaskbar; } else if (role == SkipPager) { // FIXME Implement. } else if (role == AppPid) { diff --git a/libtaskmanager/xwindowtasksmodel.cpp b/libtaskmanager/xwindowtasksmodel.cpp index f0d6a80f4..853ce8b53 100644 --- a/libtaskmanager/xwindowtasksmodel.cpp +++ b/libtaskmanager/xwindowtasksmodel.cpp @@ -116,7 +116,8 @@ void XWindowTasksModel::Private::init() QVector{Qt::DecorationRole, AbstractTasksModel::AppId, AbstractTasksModel::AppName, AbstractTasksModel::GenericName, AbstractTasksModel::LauncherUrl, - AbstractTasksModel::LauncherUrlWithoutIcon}); + AbstractTasksModel::LauncherUrlWithoutIcon, + AbstractTasksModel::SkipTaskbar}); }; sycocaChangeTimer.setSingleShot(true); @@ -330,7 +331,7 @@ void XWindowTasksModel::Private::windowChanged(WId window, NET::Properties prope || properties2 & (NET::WM2DesktopFileName | NET::WM2WindowClass)) { wipeInfoCache = true; wipeAppDataCache = true; - changedRoles << Qt::DecorationRole << AppId << AppName << GenericName << LauncherUrl << AppPid; + changedRoles << Qt::DecorationRole << AppId << AppName << GenericName << LauncherUrl << AppPid << SkipTaskbar; } if (properties & (NET::WMName | NET::WMVisibleName)) { @@ -645,7 +646,9 @@ QVariant XWindowTasksModel::data(const QModelIndex &index, int role) const const KWindowInfo *info = d->windowInfo(window); // _NET_WM_WINDOW_TYPE_UTILITY type windows should not be on task bars, // but they should be shown on pagers. - return (info->hasState(NET::SkipTaskbar) || info->windowType(NET::UtilityMask) == NET::Utility); + return (info->hasState(NET::SkipTaskbar) + || info->windowType(NET::UtilityMask) == NET::Utility + || d->appData(window).skipTaskbar); } else if (role == SkipPager) { return d->windowInfo(window)->hasState(NET::SkipPager); } else if (role == AppPid) {