[KDE plasma] Right-click on systray icon does nothing

PVince81 commented 7 years ago

Steps

Install owncloud-client RC1
Run it
Right click on system tray icon
Left click

Actual result

Right-click doesn't work but left click does display the popup.

Expected result

Right-click must bring up the menu.

Env

openSUSE Leap 42.2 with all updates (zypper patch)
Repo from: http://download.opensuse.org/repositories/isv:/ownCloud:/community:/testing/openSUSE_Leap_42.2/
owncloud-client-2.3.3\~rc1-8192.3.x86_64 (upgraded from owncloud-client2.3.3\~beta1-8073.1 which worked fine)
distro Qt 5.6.1-3.3.1
plasma 5.8.6

PVince81 commented 7 years ago

@guruz FYI

Going to try compiling from source to see if the problem is with the bundled libs.

PVince81 commented 7 years ago

Checked out v2.3.3-rc1 from source:

[x] Leap package with bundled Qt 5.6.2: right-click doesn't work. Systray icon is gray cloud. 🚫
[x] Leap local build with local Qt 5.6.1: right-click works, systray icon is cloud border only (looks nicer with Plasma 5) ✅
[x] Leap local build with Qt 5.6.2 bundled by package: right-click does not work, see https://github.com/owncloud/client/issues/5960#issuecomment-322235963

I also have a Tumbleweed box but there is currently no package so I can't test.

[x] Tumbleweed package with bundled Qt 5.6.2: right-click works ✅
[x] Tumbleweed local build with local Qt 5.9.1: right-click works ✅
[ ] Tumbleweed local build with bundled Qt: TODO

jnweiger commented 7 years ago

thanks for testing.

the cmake call for using the bundled Qt is something like this

cmake .. \
  -DWITH_DOC=TRUE \
  -DCMAKE_FIND_ROOT_PATH="/opt/ownCloud/qt-5.6.2" \
  -DMIRALL_VERSION_SUFFIX="rc1" \
  -DMIRALL_VERSION_BUILD="8192" \
  -DKDE_INSTALL_USE_QT_SYS_PATHS=1 \
  -DCMAKE_C_FLAGS:STRING="%{optflags}" \
  -DCMAKE_CXX_FLAGS:STRING="%{optflags}" \
  -DCMAKE_SKIP_RPATH=OFF \
  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
  -DCMAKE_INSTALL_PREFIX=/opt/ownCloud/ownCloud \
  -DCMAKE_INSTALL_DATAROOTDIR:PATH=%{_datadir} \
  -DDATA_INSTALL_DIR:PATH=%{_datadir} \
  -DCMAKE_INSTALL_DOCDIR:PATH=%{_docdir} \
  -DSYSCONF_INSTALL_DIR=%{_sysconfdir} \
%if %{_lib} == lib64
  -DLIB_SUFFIX=64 \
%endif
  -DQTKEYCHAIN_INCLUDE_DIR=/opt/ownCloud/qt-5.6.2/include/qt5keychain \
  -DQTKEYCHAIN_LIBRARY=/opt/ownCloud/qt-5.6.2/%{_lib}/libqt5keychain.so \
  -DGIT_SHA1="HACK FIXME" \
  -DBUILD_SHELL_INTEGRATION=OFF \
  -DPACKAGE="%{name}" \
  -DCMAKE_SKIP_BUILD_RPATH=OFF -DCMAKE_BUILD_WITH_INSTALL_RPATH=ON -DCMAKE_INSTALL_RPATH="/opt/ownCloud/ownCloud/%{_lib};/opt/ownCloud/qt-5.6.2/%{_lib}" -DCMAKE_INSTALL_RPATH_USE_LINK_PATH=ON

PVince81 commented 7 years ago

from the build log:

cmake .. -DWITH_DOC=TRUE -DCMAKE_FIND_ROOT_PATH=/opt/ownCloud/qt-5.6.2 -DMIRALL_VERSION_SUFFIX=rc1 -DMIRALL_VERSION_BUILD=8192 -DKDE_INSTALL_USE_QT_SYS_PATHS=1 '-DCMAKE_C_FLAGS:STRING=-fmessage-length=0 -grecord-gcc-switches -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables -fasynchronous-unwind-tables' '-DCMAKE_CXX_FLAGS:STRING=-fmessage-length=0 -grecord-gcc-switches -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables -fasynchronous-unwind-tables' -DCMAKE_SKIP_RPATH=OFF -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_INSTALL_PREFIX=/opt/ownCloud/ownCloud -DCMAKE_INSTALL_DATAROOTDIR:PATH=/usr/share -DDATA_INSTALL_DIR:PATH=/usr/share -DCMAKE_INSTALL_DOCDIR:PATH=/usr/share/doc/packages -DSYSCONF_INSTALL_DIR=/etc -DLIB_SUFFIX=64 -DQTKEYCHAIN_INCLUDE_DIR=/opt/ownCloud/qt-5.6.2/include/qt5keychain -DQTKEYCHAIN_LIBRARY=/opt/ownCloud/qt-5.6.2/lib64/libqt5keychain.so '-DGIT_SHA1=HACK FIXME' -DBUILD_SHELL_INTEGRATION=OFF -DPACKAGE=owncloud-client -DCMAKE_SKIP_BUILD_RPATH=OFF -DCMAKE_BUILD_WITH_INSTALL_RPATH=ON '-DCMAKE_INSTALL_RPATH=/opt/ownCloud/ownCloud/lib64;/opt/ownCloud/qt-5.6.2/lib64' -DCMAKE_INSTALL_RPATH_USE_LINK_PATH=ON

PVince81 commented 7 years ago

after some struggles I got it running:

export LD_LIBRARY_PATH=/opt/ownCloud/qt-5.6.2/lib64:/opt/ownCloud/ownCloud/lib64/
bin/owncloud

right-click doesn't work here.

PVince81 commented 7 years ago

The packaged owncloud-client works if I start it with:

OWNCLOUD_FORCE_QDBUS_TRAY_WORKAROUND=1 owncloud

right click works then

PVince81 commented 7 years ago

We tried other things like XDG_CURRENT_DESKTOP=gnome and XDG_CURRENT_DESKTOP=xfce but it doesn't work.

We checked LD_DEBUG and lsof to check if other Qt libs were loaded, but none outside of "/opt/ownCloud" were loaded.

Also apparently no KDE theme plugin.

PVince81 commented 7 years ago

I've setup a Leap 42.3 VM (my local system is 42.2).

Inside that VM, the owncloud client works fine and the right click works. It looks like Leap 42.3 already provides Qt 5.6.2 so it might explain why it works there.

Would need to test again with a 42.2 VM with older Qt to confirm if this whole issue is about some incompatibilities.

PVince81 commented 7 years ago

this also means that bundling 5.6.2 for Leap 42.3 is a bit superfluous...

jnweiger commented 7 years ago

Tumbleweed packages with bundled Qt are there:

PVince81 commented 7 years ago

Just tested Tumbleweed package owncloud-client-2.3.3~rc1-8194.1.x86_64. Desktop client works, right-click works. This is probably because Tumbleweed has Qt 5.9.1 which might work better when sharing systray with 5.6.2 than 5.6.1<->5.6.2

PVince81 commented 7 years ago

I've updated the test results post: https://github.com/owncloud/client/issues/5960#issuecomment-322227235

jnweiger commented 7 years ago

Apply this for leap 42.2 ?

%if 0%{?is_opensuse} && 0%{?suse_version} == 1315 && 0%{?sle_version} == 120200 OWNCLOUD_FORCE_QDBUS_TRAY_WORKAROUND.patch.txt %endif

PVince81 commented 7 years ago

I'm fine with this if @guruz is as well.

Let's hope it's the only affected version. There is a slight risk that other distros with older Qts might be affected as well, or at least those which are on 5.6.1.

jnweiger commented 7 years ago

I'd attribute the risks more to distro specific patches and corner case config. E.g. opensuse Leap is the only distro that needs a patch, so that our icons show up properly.

jturcotte commented 7 years ago

opensuse Leap is the only distro that needs a patch, so that our icons show up properly.

@jnweiger Which patch is this?

guruz commented 7 years ago

FYI @ckamm @ogoffart @dschmidt

jnweiger commented 7 years ago

@jturcotte

%if 0%{?is_opensuse} && 0%{?suse_version} == 1315 && 0%{?sle_version} > 120100
# openSUSE_Leap_42.2
Patch2:         no_icon_substitution.diff
%endif


-- src/libsync/theme.cpp    2017-04-24 13:42:05.000000000 +0200
+++ src/libsync/theme.cpp   2017-04-27 16:58:07.671945560 +0200
@@ -130,7 +130,8 @@
     if (cached.isNull()) {
         if(QIcon::hasThemeIcon(name)) {
             // use from theme
-            return cached = QIcon::fromTheme(name);
+       // or don't: https://github.com/owncloud/ownbrander/issues/715
+            // return cached = QIcon::fromTheme(name);
         }

         QList<int> sizes;

PVince81 commented 7 years ago

@jnweiger tested Leap 42.2 with the patched package, right-click works for me.

owncloud-client-2.3.3~rc1-8196.1.x86_64

jturcotte commented 7 years ago

From my understanding this patch works around a KDE feature to provide themed icons, and this isn't related to the tray icon context menu.

I looked at the libqt5-qtbase patches and I can't find much related to the tray icon, so it's quite possible that this also affect other distros. I just hope that this would be specific to Qt 5.6.1 and not earlier versions as well.

SamuAlfageme commented 7 years ago

Hmm... while testing other plasma-related issues with 2.3.3RC2 I also saw this one on Debian 9 w/ plasma 5.8 and Qt 5.7.1:

However I could not reproduce it on distributions with higher versions of KDE plasma. 😕

guruz commented 7 years ago

What's the conclusion on this now?

Does this need more package patches/workarounds @SamuAlfageme ? https://github.com/owncloud/client/issues/5960#issuecomment-324305705 FYI @jnweiger

If not please close this issue so we can proceed with a 2.3.3 final :-)

PVince81 commented 7 years ago

Nothing more from me, feel free to close.

dragotin commented 7 years ago

Hm, what is the conclusion now? Remember that there still is also downstream...

Do I need to include the patch in the 'official' openSUSE Tumbleweed packages? Obviously not, as Tumbleweed is on Qt version 5.9.1currently. Is it needed for the released Leap versions? And for which?

I am talking about this patch:

--- owncloudclient-2.3.3-rc1/src/gui/owncloudgui.cpp.orig   2017-08-15 09:42:26.555714353 +0000
+++ owncloudclient-2.3.3-rc1/src/gui/owncloudgui.cpp    2017-08-15 09:59:04.137384119 +0000
@@ -59,7 +59,7 @@
     _logBrowser(0),
     _contextMenuVisibleOsx(false),
     _recentActionsMenu(0),
-    _qdbusmenuWorkaround(false),
+    _qdbusmenuWorkaround(true),
     _folderOpenActionMapper(new QSignalMapper(this)),
     _recentItemsMapper(new QSignalMapper(this)),
     _app(parent)

Can anybody outline what this patch actually does? Thanks. A bit more communication on these kind of things would be great... I guess the packaging-mailinglist was killed?

ogoffart commented 7 years ago

I think there is a bug in one version of plasma that needs the work around. With newer distribution, this work around should not be required. The best is to test with plasma. If cou can right click to get the menu.

guruz commented 7 years ago

I'm closing this for now. If anyone has issues please create a new bug mentioning which package you use, which distro etc :)

owncloud / client