Don't hardcode QT_QPA_PLATFORMTHEME

Forcing QT_QPA_PLATFORMTHEME for all platforms breaks some useful platform integrations that Qt has to offer. For example portals (flatpak integration) and native file picker support. Seeing the GTK file picker open from an Qt applications looks pretty bad especially on KDE. I hope this will be fixed in the Qt 6.2 port but for now I have this patch:

jami-client-qt_PLATFORMTHEME.patch

changed the description

The major problem is that the basic value is pretty bad. Imho a better solution would be to redefine the QT_QPA_PLATFORMTHEME="the platform you want" to override the value

Isn't Qt doing this automatically?

By default the native theme is really ugly (and not set on all platforms), that's why we set it to gtk3

I did this patch: https://review.jami.net/c/jami-client-qt/+/21071

So, if your system set QT_QPA_PLATFORMTHEME, jami-qt will follow. Does flatpak set this variable?

else another solution can add a new env variable to force jami-qt to not override the theme if wanted (like JAMI_NO_OVERRIDE_THEME)

QT_QPA_PLATFORMTHEME isn't set by KDE, Gnome or any Linux distribution but get's set internally depending on the value of XDG_CURRENT_DESKTOP as far is I understand. It would make much more sense to check if XDG_CURRENT_DESKTOP equals KDE or GNOME and only fall back to QT_QPA_PLATFORMTHEME=gtk if none is the case. This patch works on my KDE Desktop: jami-client-qt_platformtheme.patch

I updated https://review.jami.net/c/jami-client-qt/+/21071

(cause the one in mainapplication was not correct and I added a way to also follow QT_QPA_PLATFORMTHEME if set)

added 1 deleted label

mentioned in commit dfae73d0

closed

For some reason it stopped working and I now learned that CString can't be compared to String directly but has to be converted using std::string() first.
Here is the reworked patch:
jami-client-qt_platformtheme.patch
With this change the patch finally works, again.

The patch looks correct, but I think you can just use strcmp to avoid any convertations.

With strcmp the return value has to be checked that's why I settled with std::string().
jami-client-qt_platformtheme.patch

Btw, if you push to review.jami.net this will avoid any author changes :) else I only add in the changelog: https://git.jami.net/savoirfairelinux/jami-client-qt/-/wikis/Changelog

https://review.jami.net/c/jami-client-qt/+/21279

I currently don't have access to like 98% of my login credentials, otherwise I would have done that.

np, it's just for your infos :)

I also prefer to use strncmp as XDG_CURRENT_DESKTOP can be entered by the user.

Also don't forget that getenv() can return null pointer and lead to undefined behavior.

reopened

Will be fixed on next nightly

closed

mentioned in commit 0140f72d

https://github.com/flathub/net.jami.Jami/issues/85#issuecomment-2005165358

added severitylow label