onyxbits / remotekeyboard

Android input method
http://www.onyxbits.de/remotekeyboard
Apache License 2.0
110 stars 25 forks source link

Line breaks get messed up after pasting multiple lines onto telnet client #5

Open denilsonsa opened 10 years ago

denilsonsa commented 10 years ago

How to reproduce:

  1. Start Remote Keyboard, focus any multi-line text field (such as List My Apps template editor, or Gmail compose window).
  2. Connect to Remote Keyboard using Linux telnet client. Should probably be reproducible if connecting from Android (using ConnectBot as a telnet client) or connecting from Mac or Windows.
  3. Select some multi-line text on your client.
  4. Paste the multi-line text onto the telnet client. (e.g. by middle-clicking)

What happens: At the Android side, all lines are concatenated together. Followed by all newlines at the end.

So, when pasting:

foo
bar
dummy
text

It gets written in the Android device as:

foobardummytext
<blank>
<blank>
<blank>

As a (boring and time-consuming) workaround, I can paste each line separately, manually. The issue only happens when pasting multiple lines at once.

onyxbits commented 10 years ago

Whoops. Thats an old bug. I wonder why that popped up again. I'll have to look into that later.

Am Dienstag, den 01.10.2013, 08:23 -0700 schrieb Denilson Figueiredo de Sá:

How to reproduce:

 1. Start Remote Keyboard, focus any multi-line text field (such
    as List My Apps template editor, or Gmail compose window).
 2. Connect to Remote Keyboard using Linux telnet client. Should
    probably be reproducible if connecting from Android (using
    ConnectBot as a telnet client) or connecting from Mac or
    Windows.
 3. Select some multi-line text on your client.
 4. Paste the multi-line text onto the telnet client. (e.g. by
    middle-clicking)

What happens: At the Android side, all lines are concatenated together. Followed by all newlines at the end.

So, when pasting:

foo bar dummy text

It gets written in the Android device as:

foobardummytext

As a (boring and time-consuming) workaround, I can paste each line separately, manually. The issue only happens when pasting multiple lines at once. — Reply to this email directly or view it on GitHub.
cbhushan commented 10 years ago

I have also encountered this bug/issue and reproduce it in current version.

PaulSD commented 9 years ago

For reference, it looks like the old fix for this bug was in 362635c355ee1651da7c3f943bed300cfcd945e7

The issue is that sendKeyEvent() is internally asynchronous (see https://code.google.com/p/android/issues/detail?id=6101 and https://android.googlesource.com/platform/frameworks/base/+/3fadee479107f0494e1e190aba2a1eea12cb0a75/core/java/android/view/inputmethod/BaseInputConnection.java#514 and https://android.googlesource.com/platform/frameworks/base/+/b3ba1d4e717d6292df4273108f0da61dac4f7b10/core/java/android/view/ViewRootImpl.java#5962), so while the fix in 362635c3 ensures that the calls to commitText() and sendKeyEvent() are made in order, it does not (and cannot) ensure that the text in the view is updated in order by those calls.

Based on https://android.googlesource.com/platform/packages/inputmethods/LatinIME/+/ecea8551c39a497e036be5c010d7ddb6b51a36bc/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java#1926 it looks like the proper way to handle this is to use commitText() itself to send newline characters. I've submitted a pull request that makes this change: https://github.com/onyxbits/remotekeyboard/pull/12

onyxbits commented 9 years ago

Sorry, but that's the wrong way to do it. Mapping LF to commitText("\n") is (probably) the right thing to do for TextAreas, but not for TextFields. It will also cause trouble in other situations, where you actually want to "press enter".

PaulSD commented 9 years ago

Sorry, the links I had in my comment were bad... I have corrected them.

If you take a look at InputLogic.java, the official android LatinIME code calls commitText() and not sendKeyEvent() if the receiving app targets JB or later, regardless of the field type, so as best as I can tell, this is the right thing to do in most cases. It was probably a mistake for me to assume it was OK to drop the backward-compatibility call to sendKeyEvent for <JB apps, so I can add that to my patch... There is also a TODO comment in LatinIME that says it should call sendKeyEvent() if the editor has TYPE_NULL ... I can implement that as well.

Do you think that making those changes might fix TextFields and other cases where you might want to just "press enter"?

onyxbits commented 9 years ago

I currently have my mind on other projects, but as far as I recall, I had the idea of calling commitText() myself, but then had to roll back. The problem simply is that commitText() is part of the (horribly ill-designed) IME interface, but there are situations in which you actually need to send a "physical" keypress event (mostly to commit textfields). It would be ok to use commitText() if you are sure that you are connected to a TextArea (and use sendKey() if not), but AFAIR there is no way of checking.

PaulSD commented 9 years ago

Ok, thanks. I will hunt around and see if I can find a way to check what kind of view you are connected to...