KXmlGui: port away from KF5 deprecated API
AbandonedPublic

Authored by dfaure on Oct 26 2019, 2:47 PM.

Details

Summary

KAboutData::setProgramIconName is deprecated and says
"Use QApplication::windowIcon", so I removed the code
that reads from programIconName() as fallback.

Test Plan

Builds

Diff Detail

Repository
R263 KXmlGui
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18196
Build 18214: arc lint + arc unit
dfaure created this revision.Oct 26 2019, 2:47 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptOct 26 2019, 2:47 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
dfaure requested review of this revision.Oct 26 2019, 2:47 PM

For backward-compatibility, we want to support this still, no? See D24965#554354

Yep, actually I found code in kcmutils (KPluginSelector::Private::PluginDelegate::slotAboutClicked) which sets the program icon name from a plugin (so NOT qApp->windowIcon()) on the aboutData passed to this dialog.
So the functionality kind of made sense, but it was removed and deprecated because KAboutData is in kcoreaddons and can't link to QIcon (commit 62aad4fc37fd by @jriddell).
I have to wonder about this logic however, as a container for a string, loaded in the above dialog, it was still useful.

The problem is that KAboutData can be about the program (main about data) or about anything else (part, plugin).
For the program, we don't need to set an icon name in KAboutData.
But for parts/plugins, I can see how this was (somewhat) useful.

The above commit deprecated setProgramIconName because KAboutData::setApplicationData can't make use of it, but the code in kaboutapplicationdialog.cpp can.
How about we undeprecate it?
And possibly rename it setIconName...

Actually I have a WIP undeprecation patch somewhere, which reduced the deprecation to notes with setApplicationData/appliactionData, Guess I should pick this from the attic, clean up and share,

Playing with reviving my programIconName undeprecation patch (by adding a new property iconName w/ setter/getter), and looking through the rest of the API, I though now think that we should keep KAboutData untouched and leave it for metadata about programs, while instead making use of KPluginMetaData instead when it comes to plugins.

As it seems KAboutData is only used with plugins to be able to reuse the KAboutApplicationDialog. So I have sketched a KAboutPluginDialog variant, which takes a KPluginMetaData instead. First tests work fine, should be uploading later tonight. So far tested with KDevelop, custom dialog for plugins actually improves things. KPluginSelector will be able to drop some code, and we can also deprecate KAboutData::fromPluginMetaData, as using KAboutApplicationDialog for a plugin was the only purpose by what lxr tells me (only in Okular & KDevelop).

To leave KAboutData::programIconData as a deprecated property and instead turn to use KPluginMetaData where the iconName property is undisputed in its usefullnes, I have now uploaded 3 patches for view;

  • D25063: Add KAboutPluginDialog, to be used with KPluginMetaData
  • D25059: KPluginSelector: use new KAboutPluginDialog
  • D25063: Deprecate KAboutData::fromPluginMetaData, now there is KAboutPluginDialog

Now, there is still the idea of using KAboutData with plugins in the KAboutData API, by the methods void KAboutData::registerPluginData(const KAboutData &aboutData) & KAboutData *KAboutData::pluginData(const QString &componentName), By what lxr.kde.org reports, the register method is still in use, but nothing seems to actually query the registration data by the other method. So possibly we could discard that pair of methods as well. Would be curious though what the usecase for this once was, it surely had a piurpose.

So, IMHO the current patch as proposed here should not be done, instead we should just silence the warnings via push/pop of compiler settings stash. We still need to support any legacy code out there which still relies on setting the icon via KAboutData::setApplicationData()). Which actually might also get unnoticed with code only tested with Wayland, as QGuiApplication::setWindowIcon is without effect there and the window icon actually fetched via the desktop file metadata entry. And we actually have a problem if no icon is provided by either way... Hm... possibly we need to resurrect KAboutData::programIconName for this wayland-only future, so the program itself has the same set of metadata about itself as the shell has... though this needs QCoreApplication to have a way to support us here, so any metaddata does not get out-of-sync...

dfaure abandoned this revision.Nov 20 2019, 8:36 AM

Excellent, thanks!