Bug 337938 - kaboutdata can't set icon
Summary: kaboutdata can't set icon
Status: RESOLVED FIXED
Alias: None
Product: frameworks-kcoreaddons
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Michael Pyne
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-31 09:41 UTC by Harald Sitter
Modified: 2014-09-07 09:21 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Harald Sitter 2014-07-31 09:41:30 UTC
void KAboutData::setApplicationData(const KAboutData &aboutData)
does the following:
>   QCoreApplication *app = QCoreApplication::instance()
> ...
>         app->setProperty("applicationIconName", s_registry->m_appData->programIconName());

this property does not exist, a QApplication variant would have a property windowIcon that however wants a QIcon instance and not a string.

maybe I am missing something but it appears to me that icon setting via KAboutData currenty is simply non-functional.
Comment 1 Jonathan Riddell 2014-07-31 09:44:46 UTC
Using the kapptemplate Frameworks example I wrote I have no icon. I was pointed out that I missed a call to KAboutData::setApplicationData() but even adding this I get no icon in the window.
Comment 2 Michael Pyne 2014-08-05 01:37:00 UTC
I didn't do the KAboutData porting, but I suspect the reason that properties are used for the icon instead of setting it directly is because kcoreaddons cannot depend on QtWidgets or QtGui (and thereby, QApplication or QGuiApplication).

Perhaps setting the property worked in an earlier Qt 5? It would almost certainly have been undocumented behavior though. I wonder what the best way to handle this is. We have a KGuiAddons but I don't see any hooks in there for KAboutData.
Comment 3 Alexander Volkov 2014-08-26 09:08:55 UTC
See commits 4827ace365ac36aada717d83914c25375c854f15 and f1e9207d0ab8c4039d70f4bc5b81ec0078fc91d2 in monolithic kdelibs repository.
The property "applicationIconName" was introduced to break dependency KUiServerJobTracker -> KAboutData rather than to set an icon for application.

The issue is that KAboutData::programIconName() was used by KApplication. Now this class is gone and "programIconName" is almost useless. I suppose that it would be better to remove this method from KAboutData and set icons by QGuiApplication::setWindowIcon().
Comment 4 Harald Sitter 2014-08-26 09:54:25 UTC
Well.
I do think that not having the icon set via aboutdata would be the most reasonable thing as it wouldn't really do anything useful anyway other than QIcon::fromTheme. 

Short of adding an applicationIconName property to qguiapp (which still wouldn't do much and might not be liked by upstream) the only way to resolve this is to have some sort of plugin-like nonesense which seems faaaaaaaaaar out of scope given that the icon feature is solving a non-problem at this point.
Comment 5 Jonathan Riddell 2014-08-28 15:35:28 UTC
As discussed on IRC KAboutData can't call setWindowIcon( QIcon::fromTheme() ) as kdecore only depends on qtcore and qicon as well as qapplication are in qtgui/widgets.

so it should be documented that it's a broken feature of the API
Comment 6 Jonathan Riddell 2014-08-28 16:39:52 UTC
https://git.reviewboard.kde.org/r/119977/
Comment 7 Jonathan Riddell 2014-09-07 09:21:21 UTC
Git commit 62aad4fc37fdc7794e5d059a5897ab1c7eabecdf by Jonathan Riddell.
Committed on 28/08/2014 at 16:28.
Pushed by jriddell into branch 'master'.

Mark setProgramIconName() as deprecated, it did not do anything

Mark setProgramIconName() as deprecated, it did not do anything since
kcoreaddons no longer uses QtGui so it can not set the icon.
REVIEW: 119977

M  +0    -1    src/lib/kaboutdata.cpp
M  +8    -4    src/lib/kaboutdata.h

http://commits.kde.org/kcoreaddons/62aad4fc37fdc7794e5d059a5897ab1c7eabecdf