p-e-w / shin

A shell in every text input on your system
GNU General Public License v3.0
278 stars 12 forks source link

Shin doesn't exit when losing focus while the cursor remains within the text input #5

Closed p-e-w closed 2 years ago

p-e-w commented 2 years ago
  1. Open a text editor
  2. Activate Shin
  3. Type a command but don't press Enter
  4. Click with the mouse somewhere else in the editor to move the cursor
  5. The pre-edit text disappears and it seems as if Shin has exited
  6. Press Enter
  7. The command is executed and its output inserted at the new cursor position
Zachu commented 2 years ago

I'm going to add to this thread because it might be related. At least this seems to be a focus issue :) Probably it has nothing to do with shin at all. In that case I'm just wishing you would have some knowledge to bump me into right direction :)

For me shin doesn't work in many applications. When I press my global shortcut for shin everything works fine in Logseq, Teams, and Rambox apps which all are IIRC Electron apps (if that has anything to do with that). But when I do press the same shortcut in Firefox, Thunderbird, or Sublime Text I get nothing. Looking at processes on my computer I see /usr/share/ibus-shin/shin flash there shortly and then quit.

What makes me think it's not issue in shin though is that if I do something like sleep 5; ibus engine shin in the console and change the focus to the application I want to use, it works totally correctly. So perhaps it's my GNOME version doing some quick focus-stealing on global hotkeys or something.

I also tried to disable the "exit on FocusOut" functionality with this patch and rebuilt shin, and it also actually helps. After this patch I could use shin pretty much everywhere. So something happens from the point of view of shin to make it lose focus shortly when I press my Gnome global key, but only inside some apps.

diff --git a/main.go b/main.go
index d6e11ee..703d5cc 100644
--- a/main.go
+++ b/main.go
@@ -337,12 +337,14 @@ func (e *engine) ProcessKeyEvent(keyval uint32, keycode uint32, state uint32) (b
        return true, nil
 }

+/*
 func (e *engine) FocusOut() *dbus.Error {
        e.clearText()
        e.exit()

        return nil
 }
+*/

 func main() {
        log.SetPrefix("[Shin] ")

TL;DR

Case1

  1. Activate shin through global gnome keyboard shortcuts when focus is on Firefox (for example in this textarea I'm currently typing)
  2. Shin process flashes quickly and exits
  3. Shin doesn't work

Case2

  1. Activate shin through global gnome keyboard shortcuts when focus is on Teams
  2. Shin process waits there nicely and everything works

Case3

  1. Activate shin from cli with a timer and change focus to Firefox
  2. Shin process waits there nicely and everything works
p-e-w commented 2 years ago

@Zachu I cannot reproduce the problem you describe, but I have fixed the original issue and have also added more detailed logging, so if you try again with the latest master you may get more insight into what's going on (run ibus-daemon --replace --verbose in a terminal to see the log output).

BTW, I very much appreciate your highly detailed reports!

Zachu commented 2 years ago

Thanks. Having a dev/sysadmin/nerd background helps in this, and I'm thinking that giving out as much details as I can would help me get my issues resolved better :) But hey I really really appreciate your work on this tool too! Wouldn't be spending time on the issues if I wouldn't. It's hugely awesome tool. Thanks for this!

I just also noticed on the other thread how I can get logs out of this =) I was just doing that!

Here's what happens when I try to start Shin on this exact textarea on Firefox:

[Shin] 2022/10/28 08:42:36 Starting
[Shin] 2022/10/28 08:42:36 Started
[Shin] 2022/10/28 08:42:36 Creating engine 1
[Shin] 2022/10/28 08:42:36 FocusIn()
[Shin] 2022/10/28 08:42:36 Enable()
[Shin] 2022/10/28 08:42:36 SetCapabilities(cap = 41)
[Shin] 2022/10/28 08:42:36 FocusOut()
[Shin] 2022/10/28 08:42:36 updateText(text = '', cursorPos = 0)
[Shin] 2022/10/28 08:42:36 Exiting
[Shin] 2022/10/28 08:42:36 FocusIn()
[Shin] 2022/10/28 08:42:36 SetCursorLocation(x = 2345, y = 2037, w = 0, h = 16)

So indeed something seems to make it quickly FocusOut and FocusIn back immediately.

This on the other hand is Logseq where things do work correctly:

[Shin] 2022/10/28 08:45:36 Starting
[Shin] 2022/10/28 08:45:36 Started
[Shin] 2022/10/28 08:45:36 Creating engine 1
[Shin] 2022/10/28 08:45:36 FocusIn()
[Shin] 2022/10/28 08:45:36 Enable()
[Shin] 2022/10/28 08:45:36 SetCursorLocation(x = 4171, y = 603, w = 0, h = 20)
[Shin] 2022/10/28 08:45:36 SetCapabilities(cap = 41)
# Here I started typing "echo foo"
[Shin] 2022/10/28 08:45:39 ProcessKeyEvent(keyval = 101, keycode = 18, state = 16)
[Shin] 2022/10/28 08:45:39 updateText(text = 'e', cursorPos = 1)
[Shin] 2022/10/28 08:45:39 ProcessKeyEvent(keyval = 99, keycode = 46, state = 16)
[Shin] 2022/10/28 08:45:39 updateText(text = 'ec', cursorPos = 2)
[Shin] 2022/10/28 08:45:39 SetCursorLocation(x = 4179, y = 602, w = 0, h = 20)
[Shin] 2022/10/28 08:45:40 SetCursorLocation(x = 4187, y = 602, w = 0, h = 20)
[Shin] 2022/10/28 08:45:40 ProcessKeyEvent(keyval = 104, keycode = 35, state = 16)
[Shin] 2022/10/28 08:45:40 updateText(text = 'ech', cursorPos = 3)
[Shin] 2022/10/28 08:45:40 ProcessKeyEvent(keyval = 101, keycode = 18, state = 1073741840)
[Shin] 2022/10/28 08:45:40 SetCursorLocation(x = 4196, y = 602, w = 0, h = 20)
[Shin] 2022/10/28 08:45:40 ProcessKeyEvent(keyval = 111, keycode = 24, state = 16)
[Shin] 2022/10/28 08:45:40 updateText(text = 'echo', cursorPos = 4)
[Shin] 2022/10/28 08:45:40 ProcessKeyEvent(keyval = 99, keycode = 46, state = 1073741840)
[Shin] 2022/10/28 08:45:40 ProcessKeyEvent(keyval = 104, keycode = 35, state = 1073741840)
[Shin] 2022/10/28 08:45:40 SetCursorLocation(x = 4206, y = 602, w = 0, h = 20)
[Shin] 2022/10/28 08:45:40 ProcessKeyEvent(keyval = 111, keycode = 24, state = 1073741840)
[Shin] 2022/10/28 08:45:40 ProcessKeyEvent(keyval = 32, keycode = 57, state = 16)
[Shin] 2022/10/28 08:45:40 updateText(text = 'echo ', cursorPos = 5)
[Shin] 2022/10/28 08:45:40 ProcessKeyEvent(keyval = 32, keycode = 57, state = 1073741840)
[Shin] 2022/10/28 08:45:40 SetCursorLocation(x = 4210, y = 602, w = 0, h = 20)
[Shin] 2022/10/28 08:45:40 ProcessKeyEvent(keyval = 102, keycode = 33, state = 16)
[Shin] 2022/10/28 08:45:40 updateText(text = 'echo f', cursorPos = 6)
[Shin] 2022/10/28 08:45:40 SetCursorLocation(x = 4215, y = 602, w = 0, h = 20)
[Shin] 2022/10/28 08:45:40 ProcessKeyEvent(keyval = 111, keycode = 24, state = 16)
[Shin] 2022/10/28 08:45:40 updateText(text = 'echo fo', cursorPos = 7)
[Shin] 2022/10/28 08:45:40 SetCursorLocation(x = 4224, y = 602, w = 0, h = 20)
[Shin] 2022/10/28 08:45:40 ProcessKeyEvent(keyval = 111, keycode = 24, state = 1073741840)
[Shin] 2022/10/28 08:45:40 ProcessKeyEvent(keyval = 102, keycode = 33, state = 1073741840)
[Shin] 2022/10/28 08:45:40 ProcessKeyEvent(keyval = 111, keycode = 24, state = 16)
[Shin] 2022/10/28 08:45:40 updateText(text = 'echo foo', cursorPos = 8)
[Shin] 2022/10/28 08:45:41 ProcessKeyEvent(keyval = 111, keycode = 24, state = 1073741840)
[Shin] 2022/10/28 08:45:41 SetCursorLocation(x = 4234, y = 602, w = 0, h = 20)
[Shin] 2022/10/28 08:45:42 ProcessKeyEvent(keyval = 65293, keycode = 28, state = 16)
[Shin] 2022/10/28 08:45:42 updateText(text = '', cursorPos = 0)
[Shin] 2022/10/28 08:45:42 Exiting
[Shin] 2022/10/28 08:45:42 SetCursorLocation(x = 4194, y = 602, w = 0, h = 20)
[Shin] 2022/10/28 08:45:42 ProcessKeyEvent(keyval = 65293, keycode = 28, state = 1073741840)

I guess since it is a focus issue outside of Shin there's not much other than dirty workarounds that could be done with it. Like setting some timeout where if "Exiting" is still happening and focus comes back in the exit would be canceled, or something of that sorts.

p-e-w commented 2 years ago

Like setting some timeout where if "Exiting" is still happening and focus comes back in the exit would be canceled, or something of that sorts.

That sounds good in theory, but AFAIK, IBus engines cannot tell which application/window the focus is in. So there's no discernible difference between the focus going out and back in, and the focus shifting to another window.

This always struck me as a weird design choice. I would have expected IBus to create one engine instance per input widget so that editing contexts are kept separate, but most of the time, it creates only one instance globally, which is why "reset on focus out" is what all IBus engines seem to do.

One very interesting thing from your logs is that you are receiving SetCursorLocation events. I have never seen one of those myself.

Zachu commented 2 years ago

Oh so that's the reason I get multiple FocusOut's and FocusIn's here and there. Then my quick and dirty hack-around is actually probably working pretty much only accidentally :)

Here's what I quickly came up with ```diff diff --git a/main.go b/main.go index 7186563..e6d81af 100644 --- a/main.go +++ b/main.go @@ -54,17 +54,25 @@ type engine struct { history *history historyPrefix string historyIndex uint32 + + focus bool + exitReason string } func (e *engine) exit() { log.Printf("Exiting") - go func() { // Give the calling function time to return, // so that pending IBus operations can finish. time.Sleep(100 * time.Millisecond) - os.Exit(0) + if e.exitReason == "Focus" && e.focus == true { + log.Printf("Exit reason was %v but focus was quickly given back. Skipping exit.", e.exitReason) + e.exitReason = "" + } else { + e.clearText() + os.Exit(0) + } }() } @@ -364,14 +372,16 @@ func (e *engine) SetCapabilities(cap uint32) *dbus.Error { func (e *engine) FocusIn() *dbus.Error { log.Printf("FocusIn()") + e.focus = true return nil } func (e *engine) FocusOut() *dbus.Error { log.Printf("FocusOut()") + e.focus = false + e.exitReason = "Focus" - e.clearText() e.exit() return nil ```

But perhaps then one choice would have a grace period on the start of Shin where FocusOut wouldn't exit yet. But yeah I don't expect these hacks to be cooked into Shin unless other people are coming up with same problems as well :)

Zachu commented 2 years ago

Actually something like this would be probably anyways even more production worthy

diff --git a/main.go b/main.go
index 7186563..d81126a 100644
--- a/main.go
+++ b/main.go
@@ -54,6 +54,8 @@ type engine struct {
        history       *history
        historyPrefix string
        historyIndex  uint32
+
+       startTime int64
 }

 func (e *engine) exit() {
@@ -371,8 +373,12 @@ func (e *engine) FocusIn() *dbus.Error {
 func (e *engine) FocusOut() *dbus.Error {
        log.Printf("FocusOut()")

-       e.clearText()
-       e.exit()
+       if time.Now().UnixMilli() < e.startTime + 250 {
+               log.Printf("FocusOut was too quick after starting. Skipping exit")
+       } else {
+               e.clearText()
+               e.exit()
+       }

        return nil
 }
@@ -418,6 +424,7 @@ func (e *engine) CandidateClicked(index uint32, button uint32, state uint32) *db

 func (e *engine) Enable() *dbus.Error {
        log.Printf("Enable()")
+       e.startTime = time.Now().UnixMilli()

        return nil
 }

Would you want this sort of thing to upstream?

p-e-w commented 2 years ago

Checking how much time has passed since the engine was activated is actually a really good idea. And yes, I would accept a PR implementing that. Obviously, it would be better if such hacks weren't necessary, but the Linux desktop is complex and if we expect Shin to work in a variety of situations such compromises are unavoidable. Every widely used piece of software is filled with this type of hack, because reality trumps concept every time.

I would want the time threshold to be as tight as possible, though. A quarter second seems quite high, unless that's actually how long it takes typically. I just pushed a commit that adds microsecond timestamps to the log. You can use that to determine what a reasonable delay looks like (something like 3-5 times the maximum delay you observe should be fine I think).

Incidentally, are you using X11 or Wayland? And if Wayland, are those particular apps running natively or on XWayland? I vaguely remember that global shortcuts on X11 are implemented using a dummy window, so perhaps that window is stealing the focus.

Also, have you tried changing the shortcut to another key combination? Perhaps the particular shortcut you use is also bound to something else, and that other action is the problem.

Zachu commented 2 years ago

Time time between Enable() and the FocusOut() has been between ~50ms to a bit less than 200ms in around my tests. I think the maximum I got was 186ms or something like that. I haven't figured out anything that reliably affects the time though. I have tried different keycombos (set in the Gnome settings Keyboard -> Keyboard shortcuts) but it doesn't affect it at all. I'm using X11, not Wayland.

Example ~59ms

[Shin] 12:49:39.746039 Starting
[Shin] 12:49:39.747120 Started
[Shin] 12:49:39.747363 Creating engine 1
[Shin] 12:49:39.748443 FocusIn()
[Shin] 12:49:39.748455 Enable()
[Shin] 12:49:39.748444 SetCapabilities(cap = 41)
[Shin] 12:49:39.807672 FocusOut()

Example ~114ms

[Shin] 12:52:44.014450 Starting
[Shin] 12:52:44.015658 Started
[Shin] 12:52:44.015860 Creating engine 1
[Shin] 12:52:44.017132 FocusIn()
[Shin] 12:52:44.017169 Enable()
[Shin] 12:52:44.017280 SetCapabilities(cap = 41)
[Shin] 12:52:44.131198 FocusOut()
p-e-w commented 2 years ago

Does switching to a Wayland session and running the affected applications under Wayland (rather than XWayland) fix the problem?

Zachu commented 1 year ago

Does switching to a Wayland session and running the affected applications under Wayland (rather than XWayland) fix the problem?

Oh sorry, almot forgot to answer this. I don't have a wayland setup anywhere to easily test this :/