rkkr / simple-keyboard

Apache License 2.0
1.13k stars 224 forks source link

Issue with Delete-Swiping and Spacebar-Swiping #183

Open Glitchy-Tozier opened 4 years ago

Glitchy-Tozier commented 4 years ago

The App OpenBoard has the same issue, so let me just post the link to their identical Page: https://github.com/dslul/openboard/issues/65

rkkr commented 4 years ago

Checked https://www.collinsdictionary.com/ with chrome and firefox focus and the search field swipe works just fine.

Glitchy-Tozier commented 4 years ago

Interesting! Unfortunately it's not really consistent and only happens once every while (except for the https://www.collinsdictionary.com/ Firefox-Web-App which seems to constantly make the keyboard freak out)

So it's either that or an Samsung-specific issue I guess...

Btw, the search-field on the https://www.collinsdictionary.com/ - website (On Firefox Focus) also creates the issue, seemingly less consistently than when trying it on the Web-App though. I suggest trying it there or staying with the website and

  1. Swiping a little faster and paying attention to whether the cursor jumps around a little or goes back 1-2 positions after reaching the end of the text
  2. Swiping somewhat fast (swiping as if you want to get to the end of the phrase)
  3. Trying delete-swiping... sometimes it's easier to see than when using the cursor
  4. Reloading the page if everything works perfectly fine.

I have to admit that it seems less buggy than OpenBoard in that regard so it might not be that big of an issue.

Glitchy-Tozier commented 4 years ago

If you want I can try to upload a screen-recording of the bug... it's kind of an annoying procedure so I'd appreciate it if you found it yourself.

rkkr commented 4 years ago

https://play.google.com/store/apps/details?id=com.mobisystems.msdict.embedded.wireless.collins.englishfreemium this does work buggy. I'm 99% sure that this bug is in the dictionary app. Looks like either an overflow (cause it gets into a bad state after a fast swipe) or a race condition in curor move. You should file a bug for the dictionary.

Glitchy-Tozier commented 4 years ago

Okay, i guess it's possible that some apps just handle things in a different manner compared to others. You're right, it's entirely possible that it's actually the app.

Just wanted to mention: It seems to work in a couple of different apps. For example, it just happened 3 times in a row in Firefox Focus. (on the very first, completely purple screen in the URL-bar. All i did was write nonsense like "ueoiprwo f" or " euoamd dn" or "ksjdjkfjkl" and after a bit of quick swiping the cursor started getting stuck or not reaching the end of the word. (and obviously I closed the app after successful attempts to reset it)

I feel like this bug starts out unnoticeable and then gets worse and worse the longer you swipe around in one textfield.

JonnyTech commented 4 years ago

Similar issue here, Simple Keyboard with Fennec F-Droid does not swipe to end to text, stops after penultimate character. Used to work a few versions ago. To test, click on fennec address bar and swipe spacebar back and forth, although can be replicated in most text boxes.

rkkr commented 4 years ago

Few versions ago used a totally different pointer move functionality. Previously it imitated left/right keyboard clicks. This caused issues where these keys have special meaning. Now we set the cursor index position and the app itself is responsible to handle it (this usually works unless they hacked something up).

There is a fallback to key clicks if index position is unavailable, but if app says it has the pointer index, we move it. This will probably be a no-fix as GBoard works the same way.

JonnyTech commented 4 years ago

Ah, so in app/src/main/java/rkr/simplekeyboard/inputmethod/latin/LatinIME.java there is public boolean hasCursorPosition():

return mExpectedSelStart != INVALID_CURSOR_POSITION && mExpectedSelEnd != INVALID_CURSOR_POSITION;

or as I understand it:

return ((mExpectedSelStart != INVALID_CURSOR_POSITION) && (mExpectedSelEnd != INVALID_CURSOR_POSITION));

which then is used in public void onMovePointer(int steps):

final int end = mInputLogic.mConnection.getExpectedSelectionEnd() + steps;

But is steps zero-based, which should make it:

final int end = mInputLogic.mConnection.getExpectedSelectionEnd() + (steps +1);

(forgive me if I am wrong - I am not a programmer)

rkkr commented 4 years ago

Nah, everything is fine with index calculation. The issue is that mExpectedSelStart and mExpectedSelEnd is correctly reported by application, but mInputLogic.mConnection.setSelection(start, end) is not handled properly by the application cause they hacked something up.

Glitchy-Tozier commented 4 years ago

There is a fallback to key clicks if index position is unavailable, but if app says it has the pointer index, we move it. This will probably be a no-fix as GBoard works the same way.

Btw, I couldn't reproduce the swiping issue with GBoard. (I tested both in F-Droid)

Shadowspear123 commented 4 years ago

So it's not just me! I've had this issue since a couple updates ago. A couple updates back, both swiping gestures used to work flawlessly and reliably. Now it randomly jumps around. Sometimes the spacebar swiping doesn't work at all or spazzes back and is not accurate. It definitely happens more in some apps than others, but it's not limited to just a single app for me.

Wilker-uwu commented 4 years ago

i can also relate. but for me the cursor gets stuck seemingly at random, and only starts moving again when i reopen the keyboard

JonnyTech commented 4 years ago

Indeed, everything worked perfectly several versions ago. We need to pinpoint which update broke functionality.

rkkr commented 4 years ago

Space swipe was changed in 5c1e49598965ccaccaeb1ecd9b4a4ee6222d5eff which fixes #147 .

Delete swipe (that selects text) is not changed for a long time since https://github.com/rkkr/simple-keyboard/pull/99 .

Glitchy-Tozier commented 4 years ago

To me, the interesting thing is that OpenBoard, another Open Source keyboard has the same bug. (See link in the first comment)

RickyM7 commented 4 years ago

Same bug on v3.23 Phone: Redmi Note 4 (MIUI 11 - Android 7.0)

thiswillbeyourgithub commented 3 years ago

Still happenning in 4.8

Though I noticed that if I swipe right and it gets stuck, continuing to swipe right several times will not change the position of the cursor but now : if I want to go left I will have to do it as much left as I had been going right.

So it behaves like if, when the cursor is stuck, motion is still somewhat registered. And changing direction has to move all this virtual travel back.

This is happenning in maybe 3 message out of 4 that I type in Signal in french. It's really annoying :/

rkkr commented 3 years ago

Nothing has changed in swipe code so don't expect for it to magically start working differently.

The "swipe right stuck and then need to swipe left as much" is easy to explain. If app says there is still text to the right of the cursor, we say to move the cursor to higher index. Now after much swiping reported index position is at over 9000 (in reality at 42), but as there is still text to the right of current index being reported, we continue swiping right. Now it's obvious that you'll need to swipe all the way back to the real index position for cursor to get unstuck.

I'm not eager to fix bugs like these. Adding workarounds to mask bugs in other applications usually end up in more bugs. Unless you're able to create some AI that understands which workaround to apply (which google has resources to create).

Glitchy-Tozier commented 3 years ago

The weird thing is, even some open-source keyboards somehow manage to prevent this bug.

I don't remember how well ASK handles this issue, but FlorisBoard works flawlessly.

rkkr commented 3 years ago
            SwipeAction.MOVE_CURSOR_LEFT -> KeyData.ARROW_LEFT
            SwipeAction.MOVE_CURSOR_RIGHT -> KeyData.ARROW_RIGHT

            inputEventDispatcher.send(InputKeyEvent.downUp(keyData))

FlorisBoard clicks left/right virtual keys on space swipe. SimpleKeyboard worked this way before 5c1e49598965ccaccaeb1ecd9b4a4ee6222d5eff. Left/right keys break apps that have special binding for these keys (or don't handle them properly) #147.

Glitchy-Tozier commented 3 years ago

So it's either break apps one way or break apps the other way?

I haven't encountered any issues with florisboards implementation so far. On the other hand, i encounter bugs with Simpe Keyboard's and OpenBoard's implementation on ever, fifth app i use.

rkkr commented 3 years ago

Yeah, pretty much. App can also not support indexes at all, then SimpleKeyboard falls back to clicking left/right keys (apps like ConnectBot). But if app tells it supports indexes, they're used, even if they don't work properly.

What is really strange is how easily index position can get out of sync. This is the flow under the hood:

  1. When keyboard is opened onStartInput is called. This contains the initial cursor position. a. If no cursor position is reported, we assume that indexes are not supported and we fall back to left/right keys.
  2. This position is stored in mExpectedSelStart and mExpectedSelEnd variables.
  3. mExpectedSelStart and mExpectedSelEnd are adjusted before each action like move cursor and enter/delete character. a. This is a workaround for cases when target app doesn't report cursor changes so we would have our own. b. Also the application may not be able to report updates before the next swipe step is made. c. This "local index calculation" in setSelection is from in LatinIME, so majority of open source keyboards will have the same bug.
  4. onUpdateSelection is called (or not) by the application to report the real cursor position and mExpectedSelStart and mExpectedSelEnd are updated.

"Collins Dictionary" is the case where onUpdateSelection randomly stops being called. So our calculated cursor position gets out of sync real fast. And when it gets out of real text range, "Collins Dictionary" just stops responding to cursor move requests.

I have tried to comment out that "local index calculation" and pointer no longer gets stuck after fast swipes. But this makes swipe action bit jittery due to callback to onUpdateSelection delays. Swipe right is blocked until onUpdateSelection is called by the application. Jitteriness level differs between applications.

So it's again, swapping one bug for another. I've attached an APK with local index calculation removed. Try it out if you want to.

app-release.zip

Glitchy-Tozier commented 3 years ago

Thank you for your detailed response!

I personally have switched to Florisboard some time ago, but as a way to help users of Simple Keyboard navigate this problem:

What do you think about adding an "Always use virtual keys for cursor movement"-switch in advanced options?

thiswillbeyourgithub commented 3 years ago

Thanks a lot @rkkr

I installedl the version you just provided and it fixed completely my issue with Signal. I have yet to find any drawbacks. Thanks!

ghost commented 2 years ago

Are there any updates on this issue?