Mange / rofi-emoji

Emoji selector plugin for Rofi
MIT License
568 stars 26 forks source link

Rofi not giving back focus before clipboard adapter script runs, making inserts fail, on Wayland / River #53

Open inga-lovinde opened 2 years ago

inga-lovinde commented 2 years ago

After I installed rofi-emoji on my Alpine with wayland and river, it worked fine for some time, and then it stopped working at some point.

I've checked that clipboard-adapter works:

localhost:~$ riverctl spawn 'echo "😀" | /usr/share/rofi-emoji/clipboard-adapter.sh insert'
localhost:~$ 😀

I've also added debug statements to /usr/share/rofi-emoji/clipboard-adapter.sh:

     echo $0 $@ >>/tmp/rofi-emoji.log
     env >>/tmp/rofi-emoji.log
     echo "input: '$input'" >>/tmp/rofi-emoji.log

before the call to perform_copy, and logs are identical both for riverctl spawn 'echo "😀" | /usr/share/rofi-emoji/clipboard-adapter.sh insert' (which works), and for rofi -modi emoji -show emoji invoked with river hotkey. Similarly, logs are identical for echo "😀" | /usr/share/rofi-emoji/clipboard-adapter.sh insert and rofi -modi emoji -show emoji invoked from shell.

However, no emojis are actually inserted with rofi-emoji (they're copied to clipboard though).

My only guess now is that maybe the focus is somewhere in the wrong place for rofi-emoji. I've also tried adding sleep 5 in clipboard-adapter.sh right before the call to perform_insert, and now the emoji selector stays on the screen for these five seconds, implying that maybe the focus is on the emoji selector, and it tries to insert the emoji into itself? But maybe my guess is wrong.

Mange commented 2 years ago

Interesting. This could be a Wayland-specific issue, which means I would require more help in trying to fix it.

The plugin will tell Rofi to hide the main window before executing the script, so the focus should be restored. The fact that the window still shows up for you might mean that Rofi doesn't actually hide the window on Wayland when this function is called.

In theory it could also be an issue with River itself. I wonder if another Wayland implementation suffers from the same issue… :thinking:

That's all I can contribute on this by myself, I'm afraid. I'll tag this as a Help Needed issue to see if there might be someone else on Wayland that has a clue on what to look at.

inga-lovinde commented 2 years ago

I have just checked the source code for https://github.com/Mange/rofi-emoji/blob/master/src/actions.c , and it also seems that the only reason for text_adapter_action to wait for clipboard-adapter to return (instead of just creating a separate process with clipboard-adapter and exiting... sorry, I'm not proficient in Linux, so maybe I'm using some words incorrectly) is so that we can reload dialog in case of error?

But in insert_emoji, return value from text_adapter_action is ignored anyway, and it also has this comment: "View is hidden and we cannot get it back again. We must exit at this point."

So maybe it can be fixed in rofi-emoji in an universal way, without requiring any wayland knowledge, by running clipboard-adapter and returning MODE_EXIT immediately, without waiting for clipboard-adapter to complete?

I guess that would involve creating another method in https://github.com/Mange/rofi-emoji/blob/master/src/utils.c next to run_clipboard_adapter, similar but much simpler since it won't have to wait for anything or handle clipboard-adapter exit code. But then again, unfortunately, I don't know anything about Linux or Linux programming.

inga-lovinde commented 2 years ago

I've also checked rofi_view_hide source code at https://github.com/davatorium/rofi/blob/next/source/view.c , and it seems that rofi heavily depends on X (something I was not aware of), and rofi_view_hide hides X window, and will not work on Wayland natively.

So my guess is that, when rofi-emoji worked on my system, it was because it was running under xwayland. I don't know what changed, but now it runs natively (or at least xwininfo does not report anything when I click on it), so rofi_view_hide does not work, naturally.

Mange commented 2 years ago

[…] it also seems that the only reason for text_adapter_action to wait for clipboard-adapter to return (instead of just creating a separate process with clipboard-adapter and exiting... sorry, I'm not proficient in Linux, so maybe I'm using some words incorrectly) is so that we can reload dialog in case of error?

Good guess, but not quite. :smile:

It's also due to the asynchronous nature of the process communication. We have to:

  1. Make sure the child process is running.
  2. Write the text data to the process' stdin.
  3. Wait to make sure the process has successfully read all the data from stdin.
  4. Make sure the process exits to clean it up / reap the process.

We could work around this by spawning the process, and detach from it, but it might lead to other problems. We cannot just quit before the process has started working since we're writing the data to it. In theory we could make the script read all the data into a variable, then fork a child that sleeps for a bit, detach it, and then exit the parent script. Rofi would detect the script has exited, close the pipes, reap the process and exit, then the forked child might pick up and execute afterwards.

I think the complexity might not be worth it, sadly.

But in insert_emoji, return value from text_adapter_action is ignored anyway, and it also has this comment: "View is hidden and we cannot get it back again. We must exit at this point."

Yeah, which is a shame. It makes it harder to debug any script errors, but Rofi has cleaned up a lot at this point and cannot be restored.

inga-lovinde commented 2 years ago

But also it's just a single emoji. Makes me wonder what was the reason for reading input from stdin instead from an argument.

I see that you did it before in https://github.com/Mange/rofi-emoji/pull/13 (and according to the description, it simplified things a lot), but returned to stdin in the "total feature rewrite" in https://github.com/Mange/rofi-emoji/pull/44 . Just curious why :) because PR description does not explain this.

Mange commented 2 years ago

It just makes it simpler to compose it in other scripts, and for example when I wanted to be able to copy other things like the codepoint. We could also add support for accumulating multiple emojis and copy them all at once.

Some of the issues would not go away even if we went with arguments. We still need to wait for the process to finish in order to reap it.

Maybe trying a Wayland-compatible fork of Rofi could yield some good results…?

spikespaz commented 2 years ago

Using rofi-wayland on NixOS yields the same results. I figured this was the issue, but it is good to have confirmation.

niksingh710 commented 2 years ago

The same happens on Hyprland tried rojimoji and it worked perfectly fine. ig it checks the last focused window instead of the current one as that is rofi.

straycosmicllama commented 1 year ago

I don't think this is a good solution but, I run pkill rofi before running the wtype command in my rofi clipboard script to remove the "rofi overlay". This is my cliphist clipboard script

#!/bin/sh

case "$ROFI_RETV" in
    "10") echo "$1" | cliphist delete;;
    "11") echo "$1" | cliphist decode | wl-copy;;
    "1") pkill rofi; wtype "$(echo "$1" | cliphist decode)"; exit;;
esac

printf "\x00use-hot-keys\x1ftrue\n\x00no-custom\x1ftrue\n"
cliphist list
Mange commented 1 year ago

I'm now on Wayland myself (Hyprland to be specific), and for me it actually works in some programs without modification, and not at all in some other programs.

Program Vanilla With pkill Async process
Wezterm :heavy_check_mark: :heavy_check_mark: :heavy_check_mark:
Brave :x: :x: :x:
Discord :x: :x: :x:
Slack :x: :x: :x:
Telegram :heavy_check_mark: :heavy_check_mark: :heavy_check_mark:
Obsidian :boom:¹ :boom:¹ :boom:¹
Firefox :heavy_check_mark: :heavy_check_mark: :heavy_check_mark:

¹ = It types something, but it types the wrong thing. Most emojis end up as a broken character, but sometimes I get East Asian characters typed instead.

Even when running wtype manually on these problematic programs gives no working output with emojis, but they do type out hello when asked to (sleep 2; wtype hello in a terminal, and switch to the other program).

It seems to me that things based on Chromium/Electron won't work, but "native" apps using GTK, QT, etc. works as expected.

Feel free to test this yourself. Here's the different variants I did:

Patches **Vanilla** (I just used the latest release) ```sh case "$tool" in # … wtype) wtype - ;; # … ``` **pkill** ```sh case "$tool" in # … wtype) to_type="$(cat -)" pkill rofi wtype "$to_type" ;; # … ``` **Async** ```sh case "$tool" in # … wtype) to_type="$(cat -)" ( sleep 1 wtype "$to_type" ) & exit 0 ;; # … ```

I anyone else getting different results? Can you send emojis to these programs using wtype?

Mange commented 1 year ago

I've found some issues upstream to read more at:

  1. wtype having issues with Chromium: https://github.com/atx/wtype/issues/31
  2. Another program investigating why Chromium doesn't work: https://gitlab.gnome.org/World/Phosh/squeekboard/-/issues/244#note_1237206
  3. Wayland protocol issue, on why browsers appear to be doing the wrong thing: https://gitlab.freedesktop.org/wayland/wayland/-/issues/390

This all leads me to believe that the issue is not in this repo, but in some programs and how they handle keyboard input from Wayland.