sspanak / tt9

A T9 keyboard for Android devices with a hardware keypad.
Apache License 2.0
248 stars 43 forks source link

[123] mode does not produce input on any input fields on KYF33 #241

Closed mcfrei closed 8 months ago

mcfrei commented 1 year ago

Thank you so much for a wonderful project! Could you help me diagnose this issue better?

Short description: entering digits in phone number fields only does not work, digits do not appear.

Longer description: On fields for phone numbers, this keyboard does nothing (does not input a number) when pressing number keys, if field is open. I can enter exactly one digit by moving from field with cursor keys, returning to it and pressing the digit while keyboard is not open.

Device: Kyocera Torque X01 KYF33 Android version: 5.1.1 No touchscreen.

Comments: Some of the fields are [123] only on this device, for example phone numbers (it means that "change Input Mode" key does not do anything in them) (probably InputType.TYPE_CLASS_PHONE or something like this).

Existing IME input does work on this phone, unfortunately does not support any languages except Japanese and English. I can take a video of trying to input digits, or extract the PhoneBook.apk, if that helps.

sspanak commented 1 year ago

This may be the same as #225. Please, check out the discussion there.

Thank you so much for a wonderful project! Could you help me diagnose this issue better?

I am glad you joined into the effort of making it even better!

mcfrei commented 1 year ago

I plan to set up Android Studio on weekend to try and help fixing it. Thank you for a quick answer and a latest beta version! However, the problem of not entering the numbers in the restricted InputType fields is still present and is still the same in the latest beta version 20.1 (a0b9e09)

sspanak commented 1 year ago

I plan to set up Android Studio on weekend to try and help fixing it.

That would be great. From my experience, Kyocera phones are particularly problematic, because of their customized Android version. It's hard to test and debug when I don't have one.

Also, thank you for the detailed description of the problem, but I have no idea what could be wrong, as I can't reproduce it on my phones.

Some of the fields are [123] only on this device, for example phone numbers (it means that "change Input Mode" key does not do anything in them) (probably InputType.TYPE_CLASS_PHONE or something like this).

This is correct, you shouldn't be able to type letters in phone fields. The type of the field is detected in ime/helpers/InputType.java and the restriction is applied in io/github/sspanak/tt9/ime/TraditionalT9.java, in determineAllowedInputModes(). It's fyi, if you are going to dig into the code.

However, it doesn't sound to me the input field is detected in a wrong way, but it is something else.

The text modes (ABC and Predictive) do generate text and put it manually in the input fields, but 123 mode just sends KeyEvents to the connected application, through the InputConnection. Now, I am not sure what exactly Android does internally with that and how it delivers it to your phonebook app, but along the way, the phonebook may be expecting something to generate text (the digits), but instead it may only be receiving events, which it doesn't know what to do with.

Or, it may be trying to handle the KeyEvents on its own, so in this type of field, TT9 should be completely disabled.

Or... it may be something else... :slightly_smiling_face:. Anyway, I hope all this would help you debug the problem.

sspanak commented 1 year ago

@mcfrei, I did some more improvements to the Numeric mode. Try out the new v21.0, it may resolve your problem.

mcfrei commented 1 year ago

Gladly. However, I added quite a lot of names to my personal dictionary, and I am using the debug build I'm building myself, so I installed a version I built from commit b3fde40aa. I have verified that install was successful by checking the info in the About section of Settings.

Unfortunately, I am still only able to input one digit at a time, by selecting and deselecting the input field.

I am also still receiving a lot of following errors in logcat:

SPAN_EXCLUSIVE_EXCLUSIVE spans cannot have a zero length

I now have more experience using this Kyocera phone and what I have noticed is that when I use the preinstalled keyboard to enter numbers in the text fields, another text field is opened over the app window, that takes whole screen. And tt9 does not do that. I wonder is that a standard behavior?

What kind of debug info could be helpful here? Logcat? video of the work, maybe? I would be really grateful for a hint.

mcfrei commented 1 year ago

What I have found is that every time I press or release a number button in the calling interface getInstance is called and a new ModeDialer is created, and when the number key is doubled in the calling interface, there are two ModeDialers created at the exact same time.

I have found it by adding debug log in getInstance and various other functions, I can push them if you are interested.

Regarding initTyping in TraditionalT9 - there is an init of mInputMode in determineAllowedInputModes and reinit of it later when you do the getInstance, is that correct? I assume that in Kyocera calling interface input is reinit with every key press and release.

mcfrei commented 1 year ago

Here are logcat logs from the first button press, when the digit [4] IS inserted in the "phone number field":

2023-05-20 20:43:28.662 11367-11367 tt9/onStartInputView    io.github.sspanak.tt9                D  inputType: 3 fieldId: 19 fieldName: null packageName: com.android.contacts restarting: false
2023-05-20 20:43:28.702 11367-11367 onKeyDown               io.github.sspanak.tt9                D  Key: KeyEvent { action=ACTION_DOWN, keyCode=KEYCODE_4, scanCode=5, metaState=0, flags=0x8, repeatCount=0, eventTime=7012440, downTime=7012440, deviceId=23, source=0x301 } repeat?: 0 long-time: false
2023-05-20 20:43:28.952 11367-11367 onKeyUp                 io.github.sspanak.tt9                D  Key: 11 repeat?: 0

And here are logcat logs from the second button press, when the digit [5] is NOT inserted in the "phone number field":

2023-05-20 20:46:28.142 11367-11367 onKeyDown               io.github.sspanak.tt9                D  Key: KeyEvent { action=ACTION_DOWN, keyCode=KEYCODE_5, scanCode=6, metaState=0, flags=0x8, repeatCount=0, eventTime=7191920, downTime=7191920, deviceId=23, source=0x301 } repeat?: 0 long-time: false
2023-05-20 20:46:28.262 11367-11367 onKeyUp                 io.github.sspanak.tt9                D  Key: 12 repeat?: 0

They seem the same so I will continue debugging.

mcfrei commented 1 year ago

Regarding k9t9 - I have installed the latest release from github: https://github.com/danielshorten/k9t9/releases/tag/0.12.0 and I can confirm that it does allow input correctly in "phone number fields" in com.android.contacts, not ignoring key presses when field is "in focus".

Thanks for pointing it out, I will try to read and understand the difference in processing key events between tt9 and k9t9 when I have time, and update the issue or submit a pull request if I get to fix it.

mcfrei commented 1 year ago

I've narrowed it down to sendDownUpKeyEvents(mInputMode.getKeyCode()); not producing a number in "phone number fields" in TraditionalT9.handleSuggestions()

mcfrei commented 1 year ago

I also verified that sendDownUpKeyEvents(mInputMode.getKeyCode()); is the exact line that produces first number when field was not focused and keyboard was not open, by commenting that line and checking that I no longer can enter one digit by leaving field and returning to it and by replacing mInputMode.getKeyCode() in it with KeyEvent.KEYCODE_8 and verifying that the digit is always 8 even if I press other key. I am still puzzled why sendDownUpKeyEvents stops working after the keyboard is open in [123] mode though. The search continues.

sspanak commented 1 year ago

I am still puzzled why sendDownUpKeyEvents stops working after the keyboard is open in [123] mode though. The search continues.

It may have to do with the restarts you are experiencing. currentInputConnection probably becomes invalid, so no key events are making through. This is just a theory, though.

mcfrei commented 1 year ago

I have noticed that [123] mode actually does not work in any input fields, not only in numeric, both in in b3fde40 and in 4405c0e. Other modes work fine in them, though.

But there is one curious exception: long-pressing the 0 button does produce the + symbol, and I can change modes by pressing the # key, and access settings by long-pressing the * key.

That is so strange, considering the line

keyCode = (number == 0 && hold) ? KeyEvent.KEYCODE_PLUS : Key.numberToCode(number);

in Mode123.

Lastly, I have not been experiencing any restarts since I have fixed the error in the pull request you gracefully and swifty approved. thanks for that again!

mcfrei commented 1 year ago

I managed to fix the issue finally by using suggestions instead of keyCode in Mode123/ModeDialer, I'll get a pull request ready for you to check out and test on your device asap.

mcfrei commented 1 year ago

Something is wrong with long press, i need more time. :( Specifically, with my fixes, after entering dialer interface with long pressing the * button to get + character, no keyboard input passes through, thus invalidating fix for #261

mcfrei commented 1 year ago

Issue is still present in 09e5e1b (v 22.0). I think that the cause for it is that sendDownUpKeyEvents(mInputMode.getKeyCode()); in handleSuggestions does not seem to work at all on KYF33, so I was thinking that I could make a Mode123Alt class that uses suggestions instead of the keyEvent but I am not sure how to set it up so that it only is being used on KYF33, probably like that in InputMode.getInstance ?

case MODE_123:
   if(settings.is123AltMode())
      return new Mode123();
   else
     return new Mode123Alt();
mcfrei commented 1 year ago

See also my take on solving this idea in https://github.com/sspanak/tt9/compare/master...mcfrei:tt9:master (except that I replaced Mode123 with Mode123Alt in InputMode.getInstance for my build and that fixes this issue for me)

Is there anything I should add or fix in that diff to make it into a pull request?

sspanak commented 1 year ago

There may be a simpler way of doing what you are trying to do.

It seems that Mode123 is working fine on your phone, if it produces text instead of key codes. But this is weird, because other modes also produce key codes sometimes. So I need to know the following:

  1. Make sure that "#" and "*" are not hotkeys, so you can use them for typing. Do they work in ABC or Predictive modes? If you go back to 123 from another mode, do they still work? Are "Backspace" and "OK" working. They also send actions using sendDownUpKeyEvents(), so if there is something wrong with that function, they must be affected too.
  2. If the number keys are not working, but "Backspace" and "OK" are fine, maybe the key codes are wrong. In TraditionalT9.handleSuggestions(), what does mInputMode.getKeyCode() return? Are the key codes equal to KeyEvent.KEYCODE_0 through KeyEvent.KEYCODE_9, KeyEvent.KEYCODE_POUND and whatnot?
  3. If the key codes are fine, you could change the way they are sent to the system:
    • In Mode123.onOtherKey(), remove the conditions, just do:
      public boolean onOtherKey(int key) {
      reset();
      keyCode = key;
      return true;
      }
    • Find TraditionalT9.handleSuggestions()
    • Change sendDownUpKeyEvents(mInputMode.getKeyCode()) to this:
      currentInputConnection.sendKeyEvent(new KeyEvent(KeyEvent.ACTION_DOWN, mInputMode.getKeyCode()));
      currentInputConnection.sendKeyEvent(new KeyEvent(KeyEvent.ACTION_UP, mInputMode.getKeyCode()));
  4. If nothing else works, you could make Mode123 return strings:

    • Change onNumber() and reset() like this:
      
      @Override
      public boolean onNumber(int number, boolean hold, int repeat) {
      reset();
      suggestions.add((number == 0 && hold) ? "+" : String.valueOf(number));
      return true;
      }

    @Override public void reset() { super.reset(); autoAcceptTimeout = 0; }

    
    In that case `onOtherKey()` will be a bit more complex, but we'll get to it if needed.

Please, try all the scenarios above and let me know of the results. It is important not to do big changes to the Numeric mode, not to break it on other phones.

mcfrei commented 1 year ago

Thank you so much for the review and the ideas on how to do my humble change better! Here are the results of the tests.

All tests were conducted on last master cf76633 (verified by checking the About info in settings, very handy!).

  1. I removed # and * from hotkeys, and they can't type the corresponding symbol in any of the modes, namely [ abc / en ], [ english ], or [ 123 ]. Backspace deletes the last symbol both when it's bind on Back key, and if it's bind on F3 key. Ok also works both by exiting the "choose word" mode and after that by switching to the next input field.

  2. I have used the Logger.i to output the code that is being returned. The key codes are indeed equal to their constants in KeyEvent class, with 0 equal to 7, * to 17 and # to 18.

  3. I have tested this approach (replacing sendUpDownKeyEvents with two sendKeyEvent) earlier and it did not work, but I did test it again exactly as you suggested and they definitely do not produce the corresponding symbol in the text field. I did not find any other ways to send key event, so I tried using suggestions.

  4. Exactly that was what I made originally, and after the code became too complicated, I separated the strings functionality to Mode123, aiming to choose between new Mode123 and new Mode123Alt in InputMode.getInstance either by determining device model somehow, or by changing the preferred mode in settings. The full functionality also required adding boolean hold to onOtherKey to support entering alternative * and # characters on hold, correspondingly + and ;.

sspanak commented 1 year ago

So, generating numeric strings is the way to go. I reviewed k9t9 and it looks like it is doing the same.

Exactly that was what I made originally, and after the code became too complicated, I separated the strings functionality...

As I said, there is a simpler way of doing what you want to do. Let's try this solution:

package io.github.sspanak.tt9.ime.modes;

import android.view.KeyEvent;

import androidx.annotation.NonNull;

public class Mode123 extends ModePassthrough {
    @Override public int getId() { return MODE_123; }
    @Override @NonNull public String toString() { return "123"; }

    @Override public final boolean is123() { return true; }
    @Override public boolean isPassthrough() { return false; }

    @Override
    public boolean onNumber(int number, boolean hold, int repeat) {
        reset();
        suggestions.add((number == 0 && hold) ? "+" : String.valueOf(number));
        return true;
    }

    @Override
    public boolean onOtherKey(int key) {
        reset();

        int keyChar = new KeyEvent(KeyEvent.ACTION_DOWN, key).getUnicodeChar();
        if ((keyChar > 31 && key < 65) || (keyChar > 90 && keyChar < 97) || (keyChar > 122 && keyChar < 127)) {
            suggestions.add(String.valueOf((char) keyChar));
            return true;
        }

        return false;
    }

    @Override
    public void reset() {
        super.reset();
        autoAcceptTimeout = 0;
    }
}

That's the entire Mode123.java. It is working fine on both my phones and I hope it would work on yours too. If it does, I guess it would be safe to assume it would work on any phone. :smile:

With this the key code functionality can be dropped completely. I suppose, it wasn't such a good idea after all. And the other modes would also generate text in their onOtherKey(), but they will be allowed to generate letters as well.

owen-young commented 1 year ago

Thank you for pointing out this issue to me. I should have searched through existing issues before making my own, I apologize. That code you posted did work on my phone though! 123 mode is working well. Thanks a bunch!

sspanak commented 1 year ago

That's great, thanks for confirming. It's really hard to debug these when I don't have a Kyocera phone. I suppose I can add this in the next version or the one after.

owen-young commented 1 year ago

Thanks! One quirk I have noticed is when in 123 mode, the # key does a new line while also changing the mode. Not sure if this is intended or not, I can look into it more when I get home from work.

-------- Original Message -------- On Jun 26, 2023, 02:23, Dimo Karaivanov wrote:

That's great, thanks for confirming. It's really hard to debug these when I don't have a Kyocera phone. I suppose I can add this in the next version or the one after.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

sspanak commented 1 year ago

Definitely, it's not intended. Probably, the default action on your phone is new line and it is getting through instead of being suppressed. Have in mind, I wrote and tested the code above in 10 minutes or so. It is not fully tested, so I may have missed something. I'll double check when I get back to this issue. Thanks for reporting!

mcfrei commented 1 year ago

I'll get a pull request ready with that functionality. I assumed that sendDownUpKeyEvents could be crucial for some functionality on other phones, and I put more effort in long pressing * and # keys, I'll test your solution to see if it's native and it produces the same result - + on long press * and ;(pause) on long press #. What is the result of long key press of # and * keys on dialer on your devices, @sspanak ?

sspanak commented 1 year ago

What is the result of long key press of # and * keys on dialer on your devices

* is +, and # does nothing. But if it is a regular numeric field in the browser or some other app, holding these keys just types * or #. And the calculator handles them on its own and types mathematical characters. That's on the F21 Pro+. The other one is a touchscreen device, so key handling is a bit different there.

I don't think there is a standard across all phones and I don't like the idea of hardcoding some character. I'd rather pass the event to the system/app and let it decide what to do.

I assumed that sendDownUpKeyEvents could be crucial for some functionality on other phones

As I said, I wrote the code above and most of the previous suggestions in 10 minutes, so I am not 100% sure they are OK in all cases. Whenever someone opens a PR (including me), I would like to test thoroughly all modes in all apps. In other words, I will allow merging only if the code is very safe and looks good.

Edit: One of the reasons I added sendDownUpKeyEvents is this bug and because of the touchscreen support. Later, I realised it is not really mandatory for the latter. I have no idea about the bug though...

mcfrei commented 1 year ago

Thanks so much for providing your test results promptly, I am very grateful for them!

Apparently, , is a "pause slightly" functionality, and ; is a "wait for response and continue" functionality in phone numbers, although I don't know how popular it is outside Japan.

There is a E.123 standard article, where the most interesting info parts are that

There is also another link on the topic of ; usage in iPhones.

Additionally, I was able to verify that on modern android dialer, there is an "Add 2 sec pause" and "Add wait for input" options under the "three dots" menu in keypad that insert , and ; characters accordingly.

Considering all this, I would still prefer implementing long press on symbol characters # and * (as ; and + correspondingly) in both Dialer and 123 mode.

Here are my test results: On KYF33, using default preinstalled input iWnn IME long press is as follows:

Considering all that plus your information, I would suggest implementing + as a character for long pressing both 0 and * keys, to provide maximum compatibility for you, me and other users. Alternatively I could add options for "long press" in the "Keypad" area of the settings. consisting of "long press 0", "long press *" and "long press #" entries.

Considering the #225 bug, that is the reason why I implemented Mode123Alt in my commit as a separate mode, intending having an option for users to choose between it and original Mode123. Your wonderful idea about checking the original k9keboard could help to debug the issue for @lgexalter too, if they are still interested in it.

Regarding k9 keyboard, it seems that it uses inputConnection composing text feature extensively by calling setComposingText and finishComposing even in Dialer input.

mcfrei commented 1 year ago

I was able to check last version 4edda7c on KYF33 and here are the test results:

I worked on a pull request but encountered unexpected behaviour - on KYF33 when a * or # key is long pressed, there is no onKeyLongPress event in KeyPadHandler, and instead an onKeyDown event with event.isLongPress() == true is produced.

I was able to fix that hastily, but I don't like the way I did it in KYF33-2 branch (that is the branch that works though).

Is that an expected behavior? How can one deal with it, considering differences in processing logic in onKeyDown and onKeyLongPress? Have you seen stuff like this, @sspanak ?

sspanak commented 1 year ago

Apparently, , is a "pause slightly" functionality, and ; is a "wait for response and continue" functionality in phone numbers, although I don't know how popular it is outside Japan.

I've never had these on any phone, even in the old Nokia days. Neither have I ever seen them on a touchscreen dialer. I have had only Android phones though, not an iPhone. Indeed, carriers may not support such control codes in my part of the world.

Regarding k9 keyboard, it seems that it uses inputConnection composing text feature extensively by calling setComposingText and finishComposing even in Dialer input.

But this is causing double numbers on my phone every now and then. It seems that the system and the IME are in a constant race and sometimes they both win and type a number.

I worked on a pull request but encountered unexpected behaviour - on KYF33 when a * or # key is long pressed, there is no onKeyLongPress event in KeyPadHandler, and instead an onKeyDown event with event.isLongPress() == true is produced.

Ok, now I realized what the problem is. No matter what, * and # are always handled in KeyPadHandler.onKeyDown (we are returning true when they are pressed). But this would prevent the system onKeyLongPress from working, which is a problem on your phone, because there are actions assigned to long pressing these keys.

The above also means ModePassthrough is not passing through these keys, while its purpose is to provide maximum compatibility on all phones, so their "special" functions would work as expected. And this is where I would like to go. I'd rather allow the phone process key presses whenever possible, instead of writing code that does the same. Not only that, but have per-device implementations. This is what I said I don't like in my previous comment.

I admit I overlooked the case with * and # in ModePassthrough and it needs to be fixed. Since I am currently busy with adding new languages and you are on the topic, could you try one simple thing?

  1. Use the latest master
  2. Perhaps, use the new Mode123 that I suggested above.
  3. In KeyPadHandler.onKeyDown() find the if marked with this comment // start tracking key hold. Add Key.isPoundOrStar(keyCode) to the condition and see if this fixes the problem for you?

EDIT: This will probably fail too. I need some more time to concentrate on the problem and come up with a proper solution. But I am certain returning true for * and # in onKeyDown in all cases is wrong and could cause problems.

I see you did something very similar in your branch, but I think all else is not really necessary.

see also: hiragana, katakana and kanji

日本語を少し読めます :slightly_smiling_face:

sspanak commented 1 year ago

As for the AT and Microsoft command set, well... let's not go there. We are not implementing a software modem here, right? :slightly_smiling_face:

sspanak commented 1 year ago

@mcfrei, @owen-young, the latest version 23.7 fixes a lot of small bugs. Could you please try if it fixes the problem for you?

If you prefer to build from source, please use the mode-refactoring-again branch. And if the branch is gone by the time you read this, then it has been merged to master, so use master instead.

sspanak commented 1 year ago

345 is somewhat related to this. In the end, I have added proper support for all possible characters in 123 mode, depending on the input field (phone number or plain numeric).

I believe all outstanding problems discussed here will be fixed in the upcoming version 26.0, but I'll leave the issue open, for anyone affected to confirm, because I can't do it on my phone.

sspanak commented 8 months ago

If anyone is still experiencing this, please try out v27.0.

If no one comments after this, I'll consider the problem to be resolved.

github-actions[bot] commented 8 months ago

This issue was closed because it has been inactive for 14 days since being marked as stale.