Why here?
It should be linked as PRIVATE module in dialogs target.

What is it here for?

Why not?

#include <KIOGui/KIO/FavIconRequestJob>

Is using temporary file as per this example is too much to write? I think this method is unjustified and based on Qt docs probably returns invalid filename.

According to this source,
"Auto-remove is on by default." . Why setting it explicitly then?

Did you know this and this? I think it's better to adhere to guidelines of libraries we are using.

Where is this used?

It's static function so you can save on QByteArray construction by using it as follows

QByteArray::fromPercentEncoding

Would it break if you would capture by reference i.e. [&] ?

Why using constData() here? Isn't QString overloaded on QByteArray returned from toPercentEncoding?

You store here whole QIcon as a QString and that's big (especially in case of Icons::Icon::ViewBank). I'm not against it but haven't you thought about storing path to icon only?

Isn't it better to use static QTimer::singleShot? No data member and no signal connect needed then.

I know it's not related to this patch but maybe it's worth to change it to

private Q_SLOTS:

A leftover, removed.

Also a leftover trying to solve another problem.

Method is unused and a leftover from a previous version.

Yes, but I adhere to the style used throughout the whole KMyMoney code base.

Removed, more leftover.

Right, I have overlooked the static quantifier.

Yes, it crashes.

Fixed the toPercentEncoding conversion into a QString. You're right, no need to use constData().

Yes, but I want to have access to the icon w/o downloading it from the internet. And in fact, it is pretty short, the one downloaded from my bank is much bigger (factor of 10). And the path might disappear without notice as it is just a cache.

No, because you cannot re-trigger it. This is why I have chosen this implementation.

True.

Hmm... I still can see it.

That style is not compatible with any of the mentioned styles. Is there any reason why you stick explicitly with it?

Oh. And I thought you download it from the internet each time. I'm not big fan of storing encoded data in kmy file because of this. I thought your opinion was the same. If not then we should also accept D10614.

Don't you need to call

d->m_iconLoadTimer.start(200);

to re-trigger it? In that case QTimer::singleShot still has advantages.

Besides you're setting

m_iconLoadTimer.setSingleShot(true);

to not re-trigger it automatically.

For sure, there is something that I do not understand.

Now gone for good.

D10614 is a bit different I think. There an already local file is added which does not make sense in my eyes. Here we load the icon from the internet and don't have control over it's presence in the favicon cache. Since I plan to display it everytime someone opens a ledger for the account, the user might not have access to the internet at that time. Or it is slow. I also don't want to check upon opening of the ledger if the internet is present but do that only in case the user selects to open the bank's web-site. In fact, this is performed by the browser.

That last sentence is the key here :-) I might have to write a blog entry.

The only difference I see is that in your case the icon is downloaded automatically instead of pointed manually. Both icon types are meant to be FavIcons. The end effect is equal i.e. obfuscated storage.

I would establish our own cache with icons (FavIcons and icons from D10614) and point all links there but I leave it up to you.

Now I see. Thanks for the explanation.

Why don't you check this URL in KNewBankDlg::slotUrlChanged? That way you could start KNewBankDlg::slotLoadIcon only after url check passes and not every time url changes. Maybe I don't see something in your code but it seems logical to me that way.

Is that needed?

Why did you not use QStandardPaths::CacheLocation? Then no need for #include QDir or for this function at all.

It's a bit unclear what are you searching here for, so maybe a one line comment would be good.

QString::compare is overloaded with QLatin1String so no need for a heavy QStringLiteral.

It's better to write

QString::fromLatin1("%1/%2-%3")

Applies to similar strings as well.

Those new function names are a little bit ambigious with "get" function in backgound.
What would you say for:
loadIconFromCache
storeIconInCache

You sure remember that slotLoadIcon() is only called when that single shot timer elapses after the last change of the URL. So it is not checked on every change.

QDir is still needed for QDir::mkpath() due to this note in Qt docs:

The storage location returned can be a directory that does not exist; i.e., it may need to be created by the system or the user.

Good point. Will change it.