Create desktop file name based on organization domain unless set explicitely
ClosedPublic

Authored by stikonas on Apr 11 2017, 11:14 PM.

Details

Summary

On Wayland icons are obtained from desktop files that are named reverseOrganizationDomain.componentName.desktop. Unfortunately the current code sets desktop file name to reverseHomepage.comeponentName.desktop so kwin is not able to find an icon.

Test Plan

Many applications still need to correctly set organization domain, but those that do now show proper icon.

Diff Detail

Repository
R467 K3b
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
stikonas created this revision.Apr 11 2017, 11:14 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 11 2017, 11:14 PM
Restricted Application added subscribers: Frameworks, plasma-devel. · View Herald Transcript
kossebau requested changes to this revision.Apr 12 2017, 3:50 PM

Having slept over this one night, I still think that this change should be not the way to go to fix the seen issues.

The actual problem is bad usage in application code, usually due to lack of knowledge what is needed.
The documented API of KAboutData, especially AboutData::setDesktopFileName(), is very explicite what needs to be done and when. This patch would change this behaviour and runs a chance to break things for application code which relies on the documented behaviour.
Changing this behaviour to help application code whose authors never bothered to get things only means screwing over those application developers who did the effort to read up in the API dox and write proper code.

If we were to redesign this API, the behaviour proposed in this patch would be fine IMHO. So IMHO please add a comment to change things for KF6, as that the desktopfilename should be auto-generated not in the constructor and then be fixed, but from the currently set component name and org domain. Unless set explicitely to a value.

What could be done here, is to add some example code to the class API dox, for the section "Detailed Description", which would serve as some template code how to use the existing API for application metadata, together with the needed desktopfile name and/or D-Bus names. And then write a blog post to try to catch more people's awareness of this issue, pointing out also all applications which have been fixed already.

Some additional unit tests for KAboutData around the issue would be good to have as well.

This revision now requires changes to proceed.Apr 12 2017, 3:50 PM
ltoscano edited edge metadata.Apr 12 2017, 3:54 PM

I'm still confused. Documentation or not, bad usage or not, is it correct that desktop file name is constructed from both the organization domain AND the homepage?

mak added a subscriber: mak.Apr 12 2017, 4:04 PM

I'm still confused. Documentation or not, bad usage or not, is it correct that desktop file name is constructed from both the organization domain AND the homepage?

Yes. See https://standards.freedesktop.org/desktop-entry-spec/latest/ar01s02.html

I'm still confused. Documentation or not, bad usage or not, is it correct that desktop file name is constructed from both the organization domain AND the homepage?

In the current KAboutData code, it is constructed from the org domain only. But the org domain itself is default constructed from the homepage url, if one is passed in that one constructor where possible (default to kde.org homepage). So if there is a relationship, it is indirectly.
The other constructor simply defaults to kde.org/org.kde for both.

In D5405#101575, @mak wrote:

I'm still confused. Documentation or not, bad usage or not, is it correct that desktop file name is constructed from both the organization domain AND the homepage?

Yes. See https://standards.freedesktop.org/desktop-entry-spec/latest/ar01s02.html

The specs only mention reverse DNS, it says nothing about homepage. But all projects that I saw use reverse DNS of organization name, e.g. org.kde, never reverse of homepage in desktop files.

In D5405#101575, @mak wrote:

I'm still confused. Documentation or not, bad usage or not, is it correct that desktop file name is constructed from both the organization domain AND the homepage?

Yes. See https://standards.freedesktop.org/desktop-entry-spec/latest/ar01s02.html

No. As Andrius pointed out, the only relevant sentence is

For applications, the part of the name of the desktop file (before the .desktop) should follow the "reverse DNS" convention, e.g. org.example.FooViewer.desktop. The name should be a valid D-Bus well-known bus name. The valid characters (aside '.') are [A-Z][a-z][0-9] along with dash ('-') and underscore ('_'). Components may not begin with a digit.

which talks about the format (reverse DNS *convention*), but it does not say the source. Here we are talking about create this naming in KAboutData. What I would expect is:

  • if the desktop file name is specified, it is used; otherwise
  • if the organization domain is specified, regardless of the value of the homepage, the organization domain is used; otherwise
  • if the homepage is specified, the homepage is used; otherwise
  • uhm, not sure here. Just nothing, and the world will explode as expected, or a default value (if we enforced it so far) .

organization domain only should be enough. If not specified, sure, the homepage can be used

This comment was removed by stikonas.

I'm still confused. Documentation or not, bad usage or not, is it correct that desktop file name is constructed from both the organization domain AND the homepage?

In the current KAboutData code, it is constructed from the org domain only. But the org domain itself is default constructed from the homepage url, if one is passed in that one constructor where possible (default to kde.org homepage). So if there is a relationship, it is indirectly.
The other constructor simply defaults to kde.org/org.kde for both.

But the point is not whether the organization domain is constructed from the homepage by default. The point is that, if both are explicitly set, the desktop files come from both, if I'm not misreading the code.

One thing for sure: the code should be moved back to the constructor, and not dynamically at each call of desktopFileName(), or it would be a behavioral change. (Or at least it should be probably cached if we get an agreement in breaking this).

And regarding this:

Having slept over this one night, I still think that this change should be not the way to go to fix the seen issues.

The actual problem is bad usage in application code, usually due to lack of knowledge what is needed.
The documented API of KAboutData, especially AboutData::setDesktopFileName(), is very explicite what needs to be done and when. This patch would change this behaviour and runs a chance to break things for application code which relies on the documented behaviour.
Changing this behaviour to help application code whose authors never bothered to get things only means screwing over those application developers who did the effort to read up in the API dox and write proper code.

I don't see why removing the homepage from the calculation of the default value of the desktop file when the org domain is specified breaks the current behavior.
It does not change the fact that program writers should set the domain, but if you set it, it should be the relevant source (see my points above).
Sane default; yes, you can call setDesktopFileName explicitly, but is it really needed?

The desktop file name should follow the way how the dbus name is created. If the applications are broken, then they are broken. Given that I mentioned this several times at KDE conferences, blogged about it, sent mails to KDE devel lists I assume the application maintainers don't care whether their applications work on Wayland. Which is totally fine. Then let them stay broken.

The desktop file name should follow the way how the dbus name is created. If the applications are broken, then they are broken. Given that I mentioned this several times at KDE conferences, blogged about it, sent mails to KDE devel lists I assume the application maintainers don't care whether their applications work on Wayland. Which is totally fine. Then let them stay broken.

When you talk about "desktop file name", do you mean the method desktopFileName() in KAboutData, or the real file name of the desktop file?

Also: I did not maintain an application back then and I missed those notifications. Could you please link at least one reference to the explanation (or which Akademy)? I will get the others from that.

Are you saying the d-bus service name should be equal to the .desktop filename? I don't see where this requirement comes from.

The desktop file name should follow the way how the dbus name is created. If the applications are broken, then they are broken. Given that I mentioned this several times at KDE conferences, blogged about it, sent mails to KDE devel lists I assume the application maintainers don't care whether their applications work on Wayland. Which is totally fine. Then let them stay broken.

When you talk about "desktop file name", do you mean the method desktopFileName() in KAboutData, or the real file name of the desktop file?

Here I meant the method in Kaboutdata.

Also: I did not maintain an application back then and I missed those notifications. Could you please link at least one reference to the explanation (or which Akademy)? I will get the others from that.

https://blog.martin-graesslin.com/blog/2015/07/porting-qt-applications-to-wayland/

The desktop file name should follow the way how the dbus name is created. If the applications are broken, then they are broken. Given that I mentioned this several times at KDE conferences, blogged about it, sent mails to KDE devel lists I assume the application maintainers don't care whether their applications work on Wayland. Which is totally fine. Then let them stay broken.

When you talk about "desktop file name", do you mean the method desktopFileName() in KAboutData, or the real file name of the desktop file?

Here I meant the method in Kaboutdata.

Also: I did not maintain an application back then and I missed those notifications. Could you please link at least one reference to the explanation (or which Akademy)? I will get the others from that.

https://blog.martin-graesslin.com/blog/2015/07/porting-qt-applications-to-wayland/

Thanks. I guess it's the section "Setting window icon", but:

  • there are no references to D-Bus, which you referred to in the previous comment (not also in the rest of the page in this context);
  • the "domain name" link refers to organizationDomain. The problem here is that the icon was not visible in applications were organizationDomain was properly set, because the homepage was used when setting the desktop file name. So setDesktopFileName must be used explicitly, and maybe it was not available when you wrote the blog post, could you please add a note there to reference that method?

The desktop file name should follow the way how the dbus name is created. If the applications are broken, then they are broken. Given that I mentioned this several times at KDE conferences, blogged about it, sent mails to KDE devel lists I assume the application maintainers don't care whether their applications work on Wayland. Which is totally fine. Then let them stay broken.

When you talk about "desktop file name", do you mean the method desktopFileName() in KAboutData, or the real file name of the desktop file?

Here I meant the method in Kaboutdata.

Also: I did not maintain an application back then and I missed those notifications. Could you please link at least one reference to the explanation (or which Akademy)? I will get the others from that.

https://blog.martin-graesslin.com/blog/2015/07/porting-qt-applications-to-wayland/

Thanks. I guess it's the section "Setting window icon", but:

  • there are no references to D-Bus, which you referred to in the previous comment (not also in the rest of the page in this context);

Yes. I didn't want to imply that it must be the same as dbus. I think our code should apply the same logic to generate dbus name and desktop file name.

  • the "domain name" link refers to organizationDomain. The problem here is that the icon was not visible in applications were organizationDomain was properly set, because the homepage was used when setting the desktop file name.

It might be that this used to work back then or that I was not aware of the problem.

So setDesktopFileName must be used explicitly, and maybe it was not available when you wrote the blog post, could you please add a note there to reference that method?

Yes I added setDesktopFileName after that blog post. I normally do not modify old blog posts as they should not be our documentation.

stikonas abandoned this revision.Apr 12 2017, 7:57 PM

Ok, so let us not change kcoreaddons now although it's still something that could have better default behaviour in KF6.

Ok, so let us not change kcoreaddons now although it's still something that could have better default behaviour in KF6.

My comment was not meant as a stop-this-effort comment. If we can improve here to do more sane things I'm all for it.

! In D5405#101683, @graesslin wrote:
My comment was not meant as a stop-this-effort comment. If we can improve here to do more sane things I'm all for it.

No, it's not your comment. We were discussing this on #kde-devel and it looks like any improvements would require changes to the documented behaviour of KAboutData. And it could be that some apps are relying on it (although I think it's unlikely), so we didn't reach the consensus on what can be done in KF5, although everybody agreed things could be made saner for KF6...

In any quite a few apps in 17.04 or 17.08 and extragear are now fixed and I also asked some 3rd party Qt apps to fix things, i.e. qbittorrent, qtox already merged fixes in git. I think people are just not aware enough about the various issues (its not unwillingness to fix them), even those mentioned in your blog... Actually even plasmashell hasn't fixed it's icon in wayland....

This revision was automatically updated to reflect the committed changes.