Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A small fix to .desktop file that improves Gnome dock integration on #12424

Merged
merged 1 commit into from Dec 14, 2023

Conversation

uncleeugene
Copy link
Contributor

linux.
The issue was that without that line gnome dock couldn't pick right icon for Mixxx window and it wasn't possible to pin Mixxx to the dock. Both fixed.

linux.
The issue was that gnome dock couldn't pick right icon for the app and
it wasn't possible to pin Mixxx to the dock. Both fixed.
@ronso0
Copy link
Member

ronso0 commented Dec 11, 2023

There's little info on thi attribute StartupWMClass https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html

@uncleeugene Which distro + version do you use?

@ronso0
Copy link
Member

ronso0 commented Dec 11, 2023

FWIW on Ubuntu 20.04 (with xfwm4) only a few .desktop files have this set (50 of ~300), though all apps show the app icon in the taskbar and app switcher.
Save to merge I'd say 🤷

@uklotzde
Copy link
Contributor

FWIW on Ubuntu 20.04 (with xfwm4) only a few .desktop files have this set (50 of ~300), though all apps show the app icon in the taskbar and app switcher. Save to merge I'd say 🤷

Please extract these unrelated changes into a separate PR.

@ronso0
Copy link
Member

ronso0 commented Dec 11, 2023

FWIW on Ubuntu 20.04 (with xfwm4) only a few .desktop files have this set (50 of ~300), though all apps show the app icon in the taskbar and app switcher. Save to merge I'd say 🤷

Please extract these unrelated changes into a separate PR.

?
This PR is one line.

@uncleeugene
Copy link
Contributor Author

@uncleeugene Which distro + version do you use?
I run manjaro gnome/wayland. Mixxx version is 2.5 but it was the same with 2.4. I switched to 2.5 because i suffered window rendering issues in 2.4 and i haven't manage to resolve it.

@uncleeugene
Copy link
Contributor Author

FWIW on Ubuntu 20.04 (with xfwm4) only a few .desktop files have this set (50 of ~300), though all apps show the app icon in the taskbar and app switcher. Save to merge I'd say 🤷

Yes, here on Manjaro most of .desktop files live just well without that line. yet still it helped. The issue was that upon starting Mixxx gnome drew some default gear-like icon in dock, and there was just one option in context menu - "show window". Quick google shown that it is kinda like gnome couldn't deduce which window it is and this line will help it.

@daschuer
Copy link
Member

Thank you for the fix. Even though this is only one line we like to have your formal permission to distribute you changes with Mixxx.

So please sign: https://docs.google.com/a/mixxx.org/spreadsheet/viewform?formkey=dEpYN2NkVEFnWWQzbkFfM0ZYYUZ5X2c6MQ
and comment here when done.

Thank you.

@uklotzde
Copy link
Contributor

FWIW on Ubuntu 20.04 (with xfwm4) only a few .desktop files have this set (50 of ~300), though all apps show the app icon in the taskbar and app switcher. Save to merge I'd say 🤷

Please extract these unrelated changes into a separate PR.

? This PR is one line.

Doesn't matter. The modification is completely unrelated to a controller mapping PR where you would never expect such a change.

@ronso0
Copy link
Member

ronso0 commented Dec 11, 2023

@uklotzde You're confusing this with #12422
This PR is solely about the desktop launcher.

@uncleeugene
Copy link
Contributor Author

FWIW on Ubuntu 20.04 (with xfwm4) only a few .desktop files have this set (50 of ~300), though all apps show the app icon in the taskbar and app switcher. Save to merge I'd say 🤷

Please extract these unrelated changes into a separate PR.

? This PR is one line.

Doesn't matter. The modification is completely unrelated to a controller mapping PR where you would never expect such a change.

I made this commit to mapping PR by mistake but then i reverted it. This PR is just for this feature, as far as i can say.

@uncleeugene
Copy link
Contributor Author

and comment here when done.

Thank you.

Done!

@uklotzde
Copy link
Contributor

@uklotzde You're confusing this with #12422 This PR is solely about the desktop launcher.

🙈 😅

@uklotzde
Copy link
Contributor

That's what I see in Looking Glass on Wayland when starting Mixxx from the command line or with the current desktop launcher:

image

The icon is shown both in the dock and in the task switcher.

@ronso0
Copy link
Member

ronso0 commented Dec 11, 2023

I switched to 2.5 because i suffered window rendering issues in 2.4 and i haven't manage to resolve it.

@uncleeugene Please file a bug report, maybe we can work it out.

@@ -13,4 +13,5 @@ Terminal=false
Icon=mixxx
Type=Application
StartupNotify=true
StartupWMClass=org.mixxx.mixxx
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why org.mixxx.mixxx instead of org.mixxx.Mixxx like the file name?

Did you check the actual WM Class of the window using xprop or Looking Glass? My system always shows Mixxx which differs from the proposed entry here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, didn't notice the case. Yet it doesn't work with capital letter. Just rechecked.
Yes, i've checked, just commented on that.

@uklotzde
Copy link
Contributor

On X.org:

$ xprop WM_CLASS
WM_CLASS(STRING) = "mixxx", "Mixxx"

I don't see any justification for the newly proposed string.

@uncleeugene
Copy link
Contributor Author

uncleeugene commented Dec 11, 2023

Tried to remove the line. Left the pinned icon in dock. Without the line Mixxx starts just next to the pinned icon.

Снимок экрана от 2023-12-11 22-42-13

Looking glass shows "wmclass: org.mixxx.mixxx" and "untracked".

When i put the line back everything starts working as it's supposed to do, and Looking Glass shows this:

Снимок экрана от 2023-12-11 22-36-24

I will file a bug report, but maybe this information will help resolve the issue.

@uklotzde
Copy link
Contributor

I recommend to first collect wmclass/WM_CLASS reports from other Linux platforms for making an informed decision.

@uklotzde
Copy link
Contributor

Both Qt5 and Qt6 builds show the same WM Class values on Fedora. This couldn't be the issue.

Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting with an issue to collect the available information and to analyze the actual cause would probably be more appropriate.

Trial and error based on the observation from a single installation is dangerous.

@uklotzde
Copy link
Contributor

Btw, your screenshot does not show the original Mixxx icon.

@Holzhaus
Copy link
Member

@uncleeugene did you build Mixxx yourself or are you using distro packages/third party builds? Maybe they patched something which results in the different wmclass string?

@uncleeugene
Copy link
Contributor Author

Btw, your screenshot does not show the original Mixxx icon.

Yes, it is gnome theming thing.

@uncleeugene
Copy link
Contributor Author

@uncleeugene did you build Mixxx yourself or are you using distro packages/third party builds? Maybe they patched something which results in the different wmclass string?

I built it myself. I don't remember whether i pulled it from github or through AUR though. I will check this evening.

@daschuer
Copy link
Member

Ok, thank you for investigation. Lets merge it ans see if it is an improvement.

@daschuer daschuer merged commit 673d891 into mixxxdj:2.4 Dec 14, 2023
12 checks passed
@daschuer daschuer added this to the 2.4.0 milestone Dec 14, 2023
@ronso0
Copy link
Member

ronso0 commented Dec 14, 2023

I think we should indeed have waited for @uncleeugene & others to provide more info.

FWIW, Mixxx 2.4-beta from the ppa on Ubuntu 20.04.5

$  xprop WM_CLASS
WM_CLASS(STRING) = "mixxx", "Mixxx"

In the app launcher the icon is not what I expected
image
It is correct in the app switcher and taskbar
image

@uklotzde
Copy link
Contributor

@ronso0 Thanks for your investigations.

The way how valuable feedback and valid concerns are ignored is confusing 👀

@ronso0
Copy link
Member

ronso0 commented Dec 14, 2023

In the app launcher the icon is not what I expected
image

this is also used as icon for the QML windows, so I assumed it's some default icon used for the 'Music' apps category in Ubuntu Studio.

Turns out this is the icon from the Papyrus icon theme (which I don't use) and obviously my current icon theme doens't provide a mixxx icon, so this fallback is used.

With Breeze theme by KDE I get this X)
image

This is with xfce 4.14 btw

I'm don't know if it's intended but I'd prefer if Mixxx would override those 'custom' icons.

edit I'll fetch from the ppa later on to check whether this PR makes a difference.

@uklotzde
Copy link
Contributor

uklotzde commented Dec 20, 2023

Mixxx built with Qt6 running natively on Wayland without the -platform xcb argument behaves differently:

Screenshot from 2023-12-21 00-13-36
Screenshot from 2023-12-21 00-14-32

The WM Class is indeed org.mixxx.mixxx in lowercase.

@uklotzde
Copy link
Contributor

@uncleeugene Which distro + version do you use?
I run manjaro gnome/wayland. Mixxx version is 2.5 but it was the same with 2.4. I switched to 2.5 because i suffered window rendering issues in 2.4 and i haven't manage to resolve it.

Qt5 on Wayland is not working properly. At least not in Mixxx. This is a known issue that could only be resolved by building Mixxx for Qt6 instead.

Whoever packages a Qt5 build of Mixxx in a way that doesn't enable the XWayland fallback at runtime is doing it wrong.

@Swiftb0y
Copy link
Member

I'm don't know if it's intended but I'd prefer if Mixxx would override those 'custom' icons.

I don't think distro maintainers would like that. If they think that some icon or rather icon theme is better than our icon, we should let them customize. We don't need to do marketing by forcing our icon everywhere. Anyways, not important.

@ronso0
Copy link
Member

ronso0 commented Dec 21, 2023

True, probably they woldn't like.
Until now it was my impression theme designers only 'flavour' icons to match a certain mood / color scheme. But, having discovered the icons I posted above, it becomes obvious they defeat the purpose / meaning of an distinct app icon: recognisability. Switch the icon theme and it looks entirely different. wtf?

I know that the official Mixxx is not optimal, but IMO it leaves enough room to fit it into themes.
image
One way to fix this is probably to contribute adapted icons to theme providers.

@Swiftb0y
Copy link
Member

Swiftb0y commented Dec 21, 2023

recognisability. Switch the icon theme and it looks entirely different. wtf?

Yup, that's a problem, but on the side of the theme designers. If they create a theme that is totally distinct from other themes, its their the fault of their bad design work... I think users are smart enough not to blame us when they install a 3rd party theme that makes their applications unrecognizable.

@uklotzde
Copy link
Contributor

@Swiftb0y Any ideas how to fix the Wayland/XWayland inconsistencies? I can confirm the issues after I was switched back to the Wayland backend for my Qt6 build.

@ronso0
Copy link
Member

ronso0 commented Dec 21, 2023

I'm not afraid of Mixxx being blamed. It would just be nice to have a somewhat consistent icon across themes.
And a certain inconsistency is inevitable because the window icon is fixed 🤷‍♂️

@Swiftb0y
Copy link
Member

Any ideas how to fix the Wayland/XWayland inconsistencies?

No idea... I'd blame it on Gnome. I don't find a missing app icon to be severe enough to care about...

@ronso0 ronso0 mentioned this pull request Feb 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants