flathub / com.qq.QQ

https://flathub.org/apps/details/com.qq.QQ
16 stars 18 forks source link

Enable Wayland support as an opt-in feature #86

Closed WhiredPlanck closed 6 months ago

WhiredPlanck commented 6 months ago

Ref:

  1. https://docs.flatpak.org/en/latest/manifests.html#finishing
  2. https://docs.flatpak.org/en/latest/electron.html#sandbox-permissions
flathubbot commented 6 months ago

Started test build 117541

flathubbot commented 6 months ago

Build 117541 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/100364/com.qq.QQ.flatpakref
WhiredPlanck commented 6 months ago

Run and it ends up with following error:

[preload] succeeded. /app/extra/QQ/resources/app/major.node
[preload] succeeded. /app/extra/QQ/resources/app/major.node
[2:0429/192801.516650:ERROR:ozone_platform_x11.cc(240)] Missing X server or $DISPLAY
[2:0429/192801.516667:ERROR:env.cc(257)] The platform failed to initialize.  Exiting.
[0429/192801.518993:ERROR:scoped_ptrace_attach.cc(27)] ptrace: Operation not permitted (1)

Need to investigate.

WhiredPlanck commented 6 months ago

So according to https://docs.flatpak.org/en/latest/electron.html#sandbox-permissions:

The recommended option is to leave it to the user. So --socket=x11 should be used in manifest and Wayland can be tested with: flatpak run --socket=wayland org.flathub.electron-sample-app

I should not modify the access to x11 like --socket=fallback-x11.

WhiredPlanck commented 6 months ago

Maybe we can follow in the steps of the Discord flatpak and add an optional Wayland native mode. With the Discord flatpak, it by default only comes with X11 access. But if you grant it Wayland access, it will automatically launch in native Wayland mode.

flathubbot commented 6 months ago

Started test build 117568

flathubbot commented 6 months ago

Build 117568 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/100390/com.qq.QQ.flatpakref
flathubbot commented 6 months ago

Started test build 117579

flathubbot commented 6 months ago

Build 117579 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/100403/com.qq.QQ.flatpakref
WhiredPlanck commented 6 months ago

@chenzhiwei LGTM

image

flathubbot commented 6 months ago

Started test build 117765

flathubbot commented 6 months ago

Build 117765 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/100591/com.qq.QQ.flatpakref
WhiredPlanck commented 6 months ago

@arenekosreal LGTM still.

pugaizai commented 6 months ago

我的建议是在README中再加个风险提示

WhiredPlanck commented 6 months ago

我的建议是在README中再加个风险提示

是 Wayland 使用方面还是法律方面?如果是后者的话我已经加了。

pugaizai commented 6 months ago

我的建议是在README中再加个风险提示

是 Wayland 使用方面还是法律方面?如果是后者的话我已经加了。

当然是前者,比如可能导致的问题之类的

WhiredPlanck commented 6 months ago

当然是前者,比如可能导致的问题之类的

好像也没有什么问题。虽然 Electron 的 Wayland 支持还处于一个比较实验性的状态,不过目前来说配合 flags 开启还是比较稳定的。一定要说的话,就是 QQ 这里鼠标移入应用会变得很小,移出就正常,但似乎只是 QQ 这样子。

arenekosreal commented 6 months ago

It looks fine now but I personally suggest waiting until more experienced one reviewing this PR.

I think you may add a note in README to let KDE user install xdg-desktop-portal-gtk. Installing this package is suggested to fix cursor issue of chromium or electron based applications in flatpak on KDE, or people who using KDE will have to use default adwaita cursor in those applications without modifying their current theme.

WhiredPlanck commented 6 months ago

I think you may add a note in README to let KDE user install xdg-desktop-portal-gtk. Installing this package is suggested to fix cursor issue of chromium or electron based applications in flatpak on KDE, or people who using KDE will have to use default adwaita cursor in those applications without modifying their current theme.

I have xdg-desktop-portal-gtk installed, but the cursor is still so small.

arenekosreal commented 6 months ago

I think you may add a note in README to let KDE user install xdg-desktop-portal-gtk. Installing this package is suggested to fix cursor issue of chromium or electron based applications in flatpak on KDE, or people who using KDE will have to use default adwaita cursor in those applications without modifying their current theme.

I have xdg-desktop-portal-gtk installed, but the cursor is still so small.

xdg-desktop-portal-gtk is just for letting application in flatpak use cursor theme of host

As for the size is too small, I found this and it seems to be the workaround: https://github.com/flatpak/flatpak/issues/709#issuecomment-1416710789 I tested on my device and the cursor size looks right.

WhiredPlanck commented 6 months ago

@arenekosreal Great, I fix it by commit https://github.com/flathub/com.qq.QQ/pull/86/commits/dd6cffe3eb9cf11b5262f869aef2798b259b48c3

flathubbot commented 6 months ago

Started test build 117937

flathubbot commented 6 months ago

Build 117937 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/100764/com.qq.QQ.flatpakref
arenekosreal commented 6 months ago

@arenekosreal Great, I fix it by commit dd6cffe

I think you can add this in launch script instead in manifest, as this is needed just when using wayland.

chenzhiwei commented 6 months ago

@arenekosreal going to merge it as this is a workaround for wayland and will be removed finally.

@WhiredPlanck Thank you for your contribution.

WhiredPlanck commented 6 months ago

@chenzhiwei No problem.

@arenekosreal the launch script is generated by the manifest. I just keep the minimal changes.