flathub / io.typora.Typora

https://flathub.org/apps/details/io.typora.Typora
3 stars 5 forks source link

It not the time to enable Wayland support #10

Closed plumlis closed 2 years ago

plumlis commented 2 years ago

Typora still have some issues with wayland, No fcitx input method supported. No title bar.

https://github.com/typora/typora-issues/issues/4000 https://github.com/fcitx/fcitx5/issues/381

I think its not the time to enable wayland support.

Or how can I disable wayland support now?

syco commented 2 years ago

Hi, I can't test this as I don't use Wayland, maybe @Ozymandias42 knows how to fix it. The only solution I can give is to push wayland to flathub beta and restore x11 only in the default repo.

Ozymandias42 commented 2 years ago

@syco thanks for mentioning me.

@plumlis I had tested it on Plasmashell . I did have the title bar and no problems writing files. I have no idea what this fcitx is but I have seen it used on the element (matrix protocol) chat app, which also uses electron.

There it was enabled as dbus session-bus. org.freedesktop.portal.Fcitx according to flatseal. You might want to try to simply add it via flatseal and test if it works.

If it does this would need to be added to the flatpak flags in the .json file. --talk-name=org.freedesktop.portal.Fcitx

The title bar issue is strange what DE/WM are you using? From what I heard of other flatpaks some wayland compositors don't properly set things like GDK_BACKEND=wayland and QT_QPA_PLATFORM=wayland so they are available to flatpaks and they might behave strangely in wayland or even fail to start in wayland mode if they rely on these variables.

Some applications set them in the flatpak itself either via static variable setting (can be tested in flatseal too) or dynamically via script similar to what I've done with the -ozone-platform=wayland thing.

IMO this should not be needed as setting these variables is the job of your host system.

EDIT: The environment variables referred to here might have sth to do with fctix not working for you: https://www.fosskers.ca/en/blog/wayland in /etc/environment

  GTK_IM_MODULE=fcitx
  QT_IM_MODULE=fcitx
  XMODIFIERS=@im=fcitx

Assuming the xmodifers thing is implicit fcitx works on x11 because Xorg handles it. In wayland it would need to go through the windows toolkit used, hence the reliance on these variables.

EDIT2: completely forgot your question about how to disable wayland support. Simply have the XDG_SESSION_TYPE variable be something other than wayland for the Typora flatpak. You can do this either via the override subcommand of flatpak itself or graphically via Flatseal

plumlis commented 2 years ago

@syco thanks for mentioning me.

@plumlis I had tested it on Plasmashell . I did have the title bar and no problems writing files. I have no idea what this fcitx is but I have seen it used on the element (matrix protocol) chat app, which also uses electron.

There it was enabled as dbus session-bus. org.freedesktop.portal.Fcitx according to flatseal. You might want to try to simply add it via flatseal and test if it works.

If it does this would need to be added to the flatpak flags in the .json file. --talk-name=org.freedesktop.portal.Fcitx

The title bar issue is strange what DE/WM are you using? From what I heard of other flatpaks some wayland compositors don't properly set things like GDK_BACKEND=wayland and QT_QPA_PLATFORM=wayland so they are available to flatpaks and they might behave strangely in wayland or even fail to start in wayland mode if they rely on these variables.

Some applications set them in the flatpak itself either via static variable setting (can be tested in flatseal too) or dynamically via script similar to what I've done with the -ozone-platform=wayland thing.

IMO this should not be needed as setting these variables is the job of your host system.

EDIT: The environment variables referred to here might have sth to do with fctix not working for you: https://www.fosskers.ca/en/blog/wayland in /etc/environment

  GTK_IM_MODULE=fcitx
  QT_IM_MODULE=fcitx
  XMODIFIERS=@im=fcitx

Assuming the xmodifers thing is implicit fcitx works on x11 because Xorg handles it. In wayland it would need to go through the windows toolkit used, hence the reliance on these variables.

EDIT2: completely forgot your question about how to disable wayland support. Simply have the XDG_SESSION_TYPE variable be something other than wayland for the Typora flatpak. You can do this either via the override subcommand of flatpak itself or graphically via Flatseal

Hi, Thanks for your reply.

About fcitx, It seems this issue will be fixed in Chromium 98, maybe we should wait for typora update its Electron lib files.

About wayland title bar:

I'm using Gnome 41, Here is the typora issue https://github.com/typora/typora-issues/issues/4592

And the fix is on the way. https://github.com/electron/electron/pull/29618

Overall I think it's not the good timing to enable wayland enable by default. On the other side typora have its own option to enable wayland support. Someone can enable this easily if they need it. https://github.com/typora/typora-issues/issues/4000#issuecomment-885499508

I added XDG_SESSION_TYPE=x11 with Flatseal. This works to me.

catsout commented 2 years ago

@syco Better to check wayland with [ -e $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY ] && echo ok || echo no-wayland
In this way, if someone dose not want wayland, can disable wayland in flatseal with switch

Edit: need to check that WAYLAND_DISPLAY is not empty.

syco commented 2 years ago

Please see https://github.com/flathub/io.typora.Typora/pull/9#pullrequestreview-835798162, this would always be true and break execution on x11.

catsout commented 2 years ago

I think it's not always true as long as WAYLAND_DISPLAY is not empty. image

syco commented 2 years ago

I'm on a standard Debian 11 with x11, this is what I see outside the flatpak and inside the flatpak:

$ lsb_release -a
No LSB modules are available.
Distributor ID: Debian
Description:    Debian GNU/Linux 11 (bullseye)
Release:    11
Codename:   bullseye

$ echo $XDG_SESSION_TYPE
x11

$ [ -e $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY ] && echo ok || echo no-wayland
ok

$ echo "$XDG_RUNTIME_DIR/$WAYLAND_DISPLAY"
/run/user/1000/

$ flatpak run --command=bash io.typora.Typora

$ echo $XDG_SESSION_TYPE
x11

$ [ -e $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY ] && echo ok || echo no-wayland
ok

$ echo "$XDG_RUNTIME_DIR/$WAYLAND_DISPLAY"
/run/user/1000/
catsout commented 2 years ago

As I say: as long as WAYLAND_DISPLA is not empty.
So you need:

wayland_socket=${WAYLAND_DISPLAY:=wayland-0}
[ -e $XDG_RUNTIME_DIR/$wayland_socket ] && echo ok || echo no-wayland

https://tldp.org/LDP/abs/html/parameter-substitution.html#PARAMSUBREF

syco commented 2 years ago

This is clearly a non standard solution, what about all the users on x11 that didn't see this thread and don't know they have to set the WAYLAND_DISPLAY variable to an invalid value? I'm open to solutions that work without hacky changes.

catsout commented 2 years ago

No, they don't need to know any thing
When nosocket=wayland(or flatseal) is specified, there is no any wayland socket in $XDG_RUNTIME_DIR.
When the user don't use wayland, there is also no wayland socket.

If a user using wayland, but wants to run typora at x11(xwayland), the just needs to disable wayland socket in flatseal.

catsout commented 2 years ago

wayland_socket=${WAYLAND_DISPLAY:=wayland-0}
This means let wayland_socket = $WAYLAND_DISPLAY == empty ? wayland-0 : $WAYLAND_DISPLAY
You can using [ -e $XDG_RUNTIME_DIR/${WAYLAND_DISPLAY:=wayland-0} ] && echo ok || echo no-wayland or check empty directly
[ -e $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY ] && [ ! -z "$WAYLAND_DISPLAY" ] && echo ok || echo no-wayland

syco commented 2 years ago

Can you submit a pull request? I'll test it on my system, if it works I'll merge it. Thanks