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

KDE/Wayland: Backintime has a Wayland icon in the taskbar #1244

Closed
parrenin opened this issue Apr 5, 2022 · 14 comments
Closed

KDE/Wayland: Backintime has a Wayland icon in the taskbar #1244

parrenin opened this issue Apr 5, 2022 · 14 comments
Assignees
Labels
Bug Distro-Specific only for certain distributions, desktop environments or display servers

Comments

@parrenin
Copy link

parrenin commented Apr 5, 2022

When run under KDE/Wayland, Backintime does not have its proper icon in the taskbar.
Instead, it has a Wayland icon.

@aryoda
Copy link
Contributor

aryoda commented Aug 23, 2022

Which version of BiT did you use? The commonly rolled-out 1.2.1 or a newer one (eg. from here at github)?

@parrenin
Copy link
Author

I use 1.3.2 from debian testing.

@aryoda
Copy link
Contributor

aryoda commented Sep 9, 2022

May be related to https://bugs.kde.org/show_bug.cgi?id=447845 and there-in linked further bug reports: App ID does not match desktop file name so the icon is not found on Wayland (but works on X11)...

@emtiu emtiu added the systray label Sep 12, 2022
@emtiu emtiu added Distro-Specific only for certain distributions, desktop environments or display servers Bug and removed systray labels Sep 22, 2022
@parrenin
Copy link
Author

I got a similar problem with Gnumeric, which was fixed:
https://gitlab.gnome.org/GNOME/gnumeric/-/issues/677
So I was wondering if anything can be learned from the Gnumeric's case.
I ran backintime-qt with WAYLAND_DEBUG=1 and I got set_app_id("python3").
So I guess the app_id is wrong, it should be "backintime-qt".
Fixing this is beyond my skills, but if somebody fixes it, I will be happy.

@aryoda
Copy link
Contributor

aryoda commented Oct 17, 2022

We are using Qt5 for the GUI and the challenge will be to find a python API in Qt5 that does always work (not only on Wayland).

I just saw a python example which sets the app id but uses a Wayland window class (not Qt5):
https://github.com/sde1000/python-wayland/blob/b017596de0d550aabfc24a26554a22e43ceb7861/demo.py#L127

Edit: It looks like it could work by setting QGuiApplication::setDesktopFileName(QStringLiteral("libreoffice-startcenter.desktop")); correctly (found in this diff). QGuiApplication is contained in PyQt5.QtGui

BiT does instantiate a QApplication (to use QtWidgets) instead of a QGuiApplication (uses QML) but the setter is also available and could be added to

backintime/qt/qttools.py

Lines 153 to 168 in b4dd2d1

def createQApplication(app_name = 'Back In Time'):
global qapp
try:
return qapp
except NameError:
pass
if Version(QT_VERSION_STR) >= Version('5.6') and \
hasattr(Qt, 'AA_EnableHighDpiScaling'):
QApplication.setAttribute(Qt.AA_EnableHighDpiScaling)
qapp = QApplication(sys.argv)
qapp.setApplicationName(app_name)
if os.geteuid() == 0 and \
qapp.style().objectName().lower() == 'windows' and \
'GTK+' in QStyleFactory.keys():
qapp.setStyle('GTK+')
return qapp

eg.

    qapp = QApplication(sys.argv)
    qapp.setDesktopFileName("backintime-qt") # TODO find the right app id and protect with "try" since the setter exists only since v5.7

Edit 2: "backintime-qt" works for the non-root GUI in Manjaro KDE Plasma with Wayland. As root still the Wayland icon is shown (I'll do some more testing...)

@aryoda
Copy link
Contributor

aryoda commented Oct 17, 2022

BTW: Setting the App ID to identify the related .desktop file is not only important for displaying the correct icon:

https://nicolasfella.de/posts/importance-of-desktop-file-mapping/

@aryoda
Copy link
Contributor

aryoda commented Oct 17, 2022

@parrenin I have just opened the PR #1336 to fix this. It would be great if you could test this fix on your machine.

You can clone and test the fixed version from my clone https://github.com/aryoda/backintime/tree/issue/1244_wrong_icon_on_wayland or the PR itself:

git clone https://github.com/aryoda/backintime.git
git switch issue/122_wrong_icon_on_wayland
cd backintime/common
./configure
make
make test
sudo make install
cd ../qt
./configure
make
sudo make install

I think you have to go through all the configure/make/make test/make install machinery of common and qt so there is some risk doing this on a non-testing machine (up to you).

I have tested this on a Manjaro KDE Plasma VM with Wayland.

@parrenin
Copy link
Author

parrenin commented Oct 18, 2022

Sorry, but I get an error:

# git switch issue/122_wrong_icon_on_wayland
fatal: référence invalide : issue/122_wrong_icon_on_wayland

@parrenin
Copy link
Author

I patched by hand and it works, thanks!

By the way, make install installs directly in /usr/bin/.
I would suggest to install in /usr/local/bin by default, to not override the distro package.

@aryoda
Copy link
Contributor

aryoda commented Oct 18, 2022

I patched by hand and it works, thanks!

Thanks a lot for backtesting!

By the way, make install installs directly in /usr/bin/.
I would suggest to install in /usr/local/bin by default, to not override the distro package.

Thanks for this hint! Yes, for testing purposes it would be great to do a parallel installation and I have used
make install here (which is the official way of installation - package maintainers also use this)
because we currently have no other make target for installing a separate release candidate.

I will discuss this with the team.

@buhtz
Copy link
Member

buhtz commented Oct 18, 2022

By the way, make install installs directly in /usr/bin/.
I would suggest to install in /usr/local/bin by default,

I see it the same. See the SO-answer to compare the different bin folders.

to not override the distro package.

No matter if we install in /usr/bin or /usr/local/bin the question is if we should do this at all. Maybe the make-file should check for the existence of a bit installation and avoid installation?

@aryoda
Copy link
Contributor

aryoda commented Oct 18, 2022

@buhtzz I'd say let's discuss this in our email news group to keep the issue clean and don't mix topics here

@emtiu
Copy link
Member

emtiu commented Oct 21, 2022

Fixed by merging #1336. Thanks, everyone!

@emtiu emtiu closed this as completed Oct 21, 2022
@emtiu
Copy link
Member

emtiu commented Oct 21, 2022

Re-opening until the problem mentioned in #1336 (comment) is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Distro-Specific only for certain distributions, desktop environments or display servers
Projects
None yet
Development

No branches or pull requests

4 participants