Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-25882] Android: return event is received twice from Symbol TC55/TC70 scanner. #9948

Merged
merged 5 commits into from Apr 6, 2018

Conversation

ypbnv
Copy link
Contributor

@ypbnv ypbnv commented Mar 20, 2018

JIRA: https://jira.appcelerator.org/browse/TIMOB-25882

NOTE: This PR relies on #9933 to be merged first.

Description:
As far as I understood/researched and tested the scanner simulates a keyboard for the input and sends an enter key pressed event. onActionEditor is called with actionID equal to TYPE_NULL and once for every keyEvent action - ACTION_UP and ACTION_DOWN. If the first call is marked as not consumed (the method call returns false) the second one is not triggered. I am not certain why it is handled that way and most likely it is bubbled up to something that prevents the second call. This seems to be the case regardless of the IME_ACTION for the TextField. I was not able to think of a cleaner solution that would cover the cases.

I don't think should be labeled as a bug or improvement, because it has the same behavior in native Android as well. We are doing some of the filtering of the events under the hood.

@ypbnv ypbnv added this to the 7.2.0 milestone Mar 20, 2018
@ypbnv ypbnv changed the title Prevent double onActionEditor call from simulated enter key. [TIMBO-25882] Android: return event is received twice from Symbol TC55/TC70 scanner. Mar 20, 2018
@build build added the android label Mar 20, 2018
@jquick-axway
Copy link
Contributor

I'm not sure if I understand.

Do you mean actual scanning software that reads texts via the mobile device's camera and outputs it to the app's TextField? And this issue only happens with Motorola Symbol phones? If so, then I would assume its simulating key events via software.

How were you able to test this?

@ypbnv
Copy link
Contributor Author

ypbnv commented Mar 21, 2018

I was not able to test the exact scenario as the one described in the ticket's comment.

Apparently these devices have an embedded scanner that can be set up by third party software.
https://developer.zebra.com/thread/31628
I assumed that from the link above.
What I tested was sending keyevents through adb and by setting an emulator to take advantage of the hardware keyboard of my laptop. By doing so I got the reported behavior in versions 6.3.0.GA through 7.1.0.GA. I combined this with the code we had in 6.3.0.GA (reported by the user to work for them)
https://github.com/appcelerator/titanium_mobile/blob/6_3_X/android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUIText.java#L473
and the following changes which "broke" that. This is the closest I could get to the actual scenario, bearing in mind that I have no clue what the actual events thrown by the scanner are.

I don't like what I did in this PR and IMO this is something that is hard to be universally solved, because different configuration of such scanners' input may result in different events and their handling. A better approach for me would be to propagate whatever the Android triggers to the JS, but that will most likely cause disparity for the event and we don't want that.

@jquick-axway
Copy link
Contributor

Thanks for the explanation. It looks like PR #9652 is the one that changed this behavior. Can you do a quick test with that PR's code as well? If it passes, then yeah, let's go with your change.

@ypbnv
Copy link
Contributor Author

ypbnv commented Mar 22, 2018

@jquick-axway Tried the test case from #9652 with software keyboard and bluetooth keyboard both on landscape and portrait modes - the event is fired only once as expected.

//one of the two callbacks, and the next checks deal with 'Next' and 'Done' callbacks, respectively.
//Refer to TiUIText.handleReturnKeyType(int) for a list of return keys that are mapped to EditorInfo.IME_ACTION_NEXT and EditorInfo.IME_ACTION_DONE.
if (actionId == EditorInfo.IME_ACTION_NEXT || actionId == EditorInfo.IME_ACTION_DONE) {
if (tv.getMaxLines() == 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use the this.field property like in the other PR.
The rest looks fine.

@ypbnv
Copy link
Contributor Author

ypbnv commented Mar 26, 2018

@jquick-axway Updated.

Copy link
Contributor

@jquick-axway jquick-axway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CR: Pass

@lokeshchdhry
Copy link
Contributor

FR Passed.

Studio Ver: 5.1.0
SDK Ver: 7.2.0 local build
OS Ver: 10.13.2
Xcode Ver: Xcode 9.3
Appc NPM: 4.2.12
Appc CLI: 7.0.3-master.27
Daemon Ver: 1.0.1
Ti CLI Ver: 5.1.0
Alloy Ver: 1.11.0
Node Ver: 8.9.1
NPM Ver: 5.5.1
Java Ver: 9.0.4
Devices: ⇨ google Nexus 5 --- Android 6.0.1
⇨ google Nexus 6P --- Android 8.0.0

@jquick-axway jquick-axway changed the title [TIMBO-25882] Android: return event is received twice from Symbol TC55/TC70 scanner. [TIMOB-25882] Android: return event is received twice from Symbol TC55/TC70 scanner. Apr 6, 2018
@build build added the android label Apr 6, 2018
@build
Copy link
Contributor

build commented Apr 6, 2018

Messages
📖

💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

@hansemannn hansemannn merged commit 6e08b14 into tidev:master Apr 6, 2018
hansemannn pushed a commit to hansemannn/titanium_mobile that referenced this pull request Apr 6, 2018
…5/TC70 scanner. (tidev#9948)

* Prevent double onActionEditor call from simulated enter key.

* Change check for TextField.

* Forgot the 'this'.
build pushed a commit to garymathews/titanium_mobile that referenced this pull request Apr 23, 2018
…5/TC70 scanner. (tidev#9948)

* Prevent double onActionEditor call from simulated enter key.

* Change check for TextField.

* Forgot the 'this'.
@sgtcoolguy sgtcoolguy modified the milestones: 7.2.0, 7.3.0 May 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants