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

OpenShot icon is wrong under KDE Plasma Wayland session #1112

Closed
glaubersm opened this issue Dec 11, 2017 · 10 comments · Fixed by #3354
Closed

OpenShot icon is wrong under KDE Plasma Wayland session #1112

glaubersm opened this issue Dec 11, 2017 · 10 comments · Fixed by #3354
Labels
🐞 bug A bug, error, or breakage of any kind

Comments

@glaubersm
Copy link

System Details:

  • Operating System / Distro: Arch Linux
  • OpenShot Version: 2.4.1

Issue Description and steps to reproduce:
openshot shows generic Wayland icon instead its own icon when running under KDE Plasma Wayland session.

start KDE Plasma Wayland session
open openshot
screenshot_20171211_122843

@DylanC DylanC added the 🐞 bug A bug, error, or breakage of any kind label Feb 5, 2018
@DylanC
Copy link
Collaborator

DylanC commented Feb 5, 2018

@glaubersm - Confirmed. Thanks.

@schauveau
Copy link

I think that this could be related to a similar issue I have when using the Sway compositor.
In Wayland, each application (or window?) has an attribute app_id that can used by the compositor to customize its appearance. In Sway, I see that all Openshot windows have their app_id set to "python3" which makes sense since the default behavior is use the client executable filename.
I found a similar bug report for LibreOffice https://bugs.documentfoundation.org/show_bug.cgi?id=125934 and, according to them, the solution for Qt5 application is to declare the name of the desktop file using QGuiApplication::setDesktopFileName().

I tried on the following PyQt program and the app_id value in Sway is effectively set to "foobar":

import sys
from PyQt5 import QtCore, QtWidgets

if __name__ == "__main__":
    app = QtWidgets.QApplication(sys.argv)
    app.setDesktopFileName("foobar")    #
    win = QtWidgets.QMainWindow()
    dialog = QtWidgets.QDialog(win)
    dialog.setModal(True)
    dialog.exec_()

So for Openshot, the desktopFilename property should be set to "org.openshot.OpenShot" (or "openshot-qt" in older versions).

@schauveau
Copy link

I patched my local version of openshot 2.4.3 (Debian) by inserting a call to self.setDesktopFileName("openshot-qt") in OpenShot.__init__() and the app_id was changed as expected. I could not check the effect on the icons since Sway is not using any.

@ferdnyc
Copy link
Contributor

ferdnyc commented Apr 6, 2020

@schauveau Hmm, thanks for the info.

Only downside to that solution is, setDesktopFilename() wasn't added to Qt until release 5.7. I guess we'll have to wrap some version checks around it, since the AppImage builds are still using 5.2.

@ferdnyc
Copy link
Contributor

ferdnyc commented Apr 6, 2020

I'm wondering if we can maybe just get away with calling QGuiApplication.setWindowIcon(), since we never do. And that's supported since at least Qt 5.0. ...Eh, I suppose both can't hurt, even.

PR #3354 adds in both a setWindowIcon() call and a setDesktopFile() call with the current ID, hopefully that'll cover all the bases.

@schauveau
Copy link

The Wayland app_id property is documented here https://cgit.freedesktop.org/wayland/wayland-protocols/tree/stable/xdg-shell/xdg-shell.xml#n631

What I can also tell you is that setWindowIcon() has no effect on the app_id and so will not affect compositors such as Sway that rely on the app_id.

Also, be aware that in Wayland, the window decorations can be drawn either by the compositor or by the application (client side decoration). So it seems to me that setWindowIcon() and setDesktopFile() are needed to address those 2 cases.

@ferdnyc
Copy link
Contributor

ferdnyc commented Apr 6, 2020

@schauveau *nod* That's the essence of my #3354 patch. Except, of course, setDesktopFile will only be used under Qt 5.7+, as it doesn't exist before then.

So, all in all it sounds like we should be covered.

@glaubersm
Copy link
Author

This bug persists with daily build installed on neon unstable via PPA repo.
Could anyone reopen this issue please?

@ferdnyc
Copy link
Contributor

ferdnyc commented Oct 27, 2020

@glaubersm The setWindowIcon and setDesktopFile ended up being removed from the code, they were causing other problems.

(Well, they were suspected of doing so, anyway. Under Windows, strangely enough. Didn't sound right to me, but I wasn't invested enough to argue.)

Afraid this is a #wontfix at this point. It's just... not that big a deal, really.

@glaubersm
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug A bug, error, or breakage of any kind
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants