Bug 404146 - Tabbox kcm incorrectly falls back to informative on broken theme
Summary: Tabbox kcm incorrectly falls back to informative on broken theme
Status: REPORTED
Alias: None
Product: kwin
Classification: Plasma
Component: tabbox (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-09 19:21 UTC by Chris Holland
Modified: 2022-07-01 16:00 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Holland 2019-02-09 19:21:37 UTC
Issue was raised when discussing the workaround in https://phabricator.kde.org/T10464

The relevant code is at:
* https://github.com/KDE/kwin/blob/cfecb1e0770ca6c8fa879124e11b03081342b9ed/tabbox/tabboxhandler.cpp#L281

1. It first checks if a "plasma/look-and-feel/%1/contents/windowswitcher/WindowSwitcher.qml" file exists and uses that.
2. If not, it looks for a "kwin/tabbox/%1/contents/ui/main.qml"
3. Then it uses the hardcoded "informative" QML file "kwin/tabbox/informative/contents/ui/main.qml" as a fallback (I always wondered why it was using informative when I selected certain "look and feel" skins).

So it seems we need a new function perform the QML loading stuff which returns a `QObject` or `nullptr`. Something like this:


	QObject *TabBoxHandlerPrivate::loadSwitcherItem(QString file)
	{
		if (file.isNull()) {
			m_qmlComponent->loadUrl(QUrl::fromLocalFile(file));
			if (m_qmlComponent->isError()) {
				m_qmlComponent.reset(...) // ???
				return nullptr;
			} else {
				QObject *object = m_qmlComponent->create(m_qmlContext.data());
				return object
			}
		}
	}

	QObject *TabBoxHandlerPrivate::createSwitcherItem(bool desktopMode)
	{
		QString lookAndFeelFilepath = desktopMode ? ... : ...
		QString file = lookAndFeelFilepath;
		QObject *object = loadSwitcherItem(file);
		if (object != nullptr) {
			return object;
		}

		QString tabboxFilepath = ...
		QString file = tabboxFilepath;
		QObject *object = loadSwitcherItem(file);
		if (object != nullptr) {
			return object;
		}

		QString informativeTabboxFilepath = ...
		QString file = informativeTabboxFilepath;
		QObject *object = loadSwitcherItem(file);
		if (object != nullptr) {
			QStringList args;
			args << QStringLiteral("The Window Switcher failed to load, using Informative")
			KProcess::startDetached(QStringLiteral("kdialog"), args);

			return object;
		} else {
			QStringList args;
			args << QStringLiteral("The Window Switcher installation is broken, resources are missing.\nContact your distribution about this")
			KProcess::startDetached(QStringLiteral("kdialog"), args);

			return nullptr;
		}
	}
Comment 1 Martin Flöser 2019-02-09 20:01:06 UTC
This means the kcm is wrong - I reworded the title accordingly. The informative theme is not part of KWin (any more) and not installed by default in most distributions. Using it as fallback is an obvious oversight.
Comment 2 Chris Holland 2019-02-09 20:35:21 UTC
The main reason I made this bug is for when a downloaded QML skin from the KDE Store is loaded but fails to load (eg: was written for KWin 4.x).

However, you do raise a good point, I completely forgot "informative" is part of kdeplasma-addons.

So it seems the hardcoded default needs to be... "org.kde.breath.desktop" so that it loads:
/usr/share/plasma/look-and-feel/org.kde.breath.desktop/contents/windowswitcher/WindowSwitcher.qml

However that still depends on the breeze package. KWin itself does not seem to ship with any "QML" tabbox skins that I can see. So does this mean it needs to somehow fallback to a C++ TabBox theme? Like the "kwin/effects/coverswitch" desktop effect? I'm not quite sure how it loads the C++ effects to be honest...

Looking at the kcm code, it seems that the desktop effect itself has a "TabBox" config property. I take it that it will then take over the "Alt+Tab" and "Alt+Shift+Tab" shortcuts.

* https://github.com/KDE/kwin/blob/cfecb1e0770ca6c8fa879124e11b03081342b9ed/kcmkwin/kwintabbox/main.cpp#L368
* https://github.com/KDE/kwin/blob/master/effects/coverswitch/coverswitch.kcfg#L35
* 

Falling back to a "kwin effect" skin sounds like a fairly complicated fallback, as you'd need to copy the kcm's save() code.
Comment 3 Martin Flöser 2019-02-10 07:43:26 UTC
Well the fallback will have to be removed.