[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

[Android] Fix TextInputType.none for devices with physical keyboard #49980

Merged
merged 12 commits into from
Feb 2, 2024
Merged

[Android] Fix TextInputType.none for devices with physical keyboard #49980

merged 12 commits into from
Feb 2, 2024

Conversation

bartcone
Copy link
Contributor

Description

This PR fixes an issue where keystrokes aren't received on Android devices with physical keyboards (e.g. rugged Zebra devices) when keyboardType is set to TextInputType.none on a TextField.

The logic in setTextInputClient and canShowTextInput created an inputTarget with InputTarget.Type.NO_TARGET which caused the input connection to short circuit and not be established.

Bug introduction PR: #26585

Related Issue

flutter/flutter#89983

Unit Test Notes

  • The existing showTextInput_textInputTypeNone() stays green after update.
  • inputConnection_textInputTypeNone() updated to assertNotNull. I would make this more specific, but this is my first venture into the Flutter engine and don't know enough about those connection attributes.

Demo

Video below with Zebra MC9300 device. This issue can also be reproduced in a standard android emulator. Simply add a TextField, configure keyboardType to be TextInputType.none and attempt to enter text after running and giving focus to textfield.

Before

before.mov

After

after.mov

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the [CLA].
  • All existing and new tests are passing.

@reidbaker reidbaker requested review from LongCatIsLooong and a team January 24, 2024 15:40
LongCatIsLooong
LongCatIsLooong approved these changes Jan 24, 2024
View reviewed changes
Copy link
Contributor
@LongCatIsLooong LongCatIsLooong left a comment

Choose a reason for hiding this comment

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

The change makes sense to me, modulo comments from other reviewers. Thanks for looking into this!

@bartcone
Copy link
Contributor Author
bartcone commented Jan 31, 2024 •
edited
Loading

@LongCatIsLooong @reidbaker is the [Linux linux_unopt] check historically flakey? I've seen it fail twice now as this PR has evolved, but "re-running" has turned it green. Just wanted to call it out in case we needed to look into something further.

e.g. https://github.com/flutter/engine/runs/21045950042

@reidbaker
Copy link
Contributor

@LongCatIsLooong @reidbaker is the [Linux linux_unopt] check historically flakey? I've seen it fail twice now as this PR has evolved, but "re-running" has turned it green. Just wanted to call it out in case we needed to look into something further.

e.g. https://github.com/flutter/engine/runs/21045950042

No it is not normally flakey for me but I think the issue might be flutter/flutter#138197

@reidbaker
Copy link
Contributor

Much cleaner after the refactor

@reidbaker reidbaker added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 31, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 31, 2024
@auto-submit Auto-submit
Copy link
Contributor
auto-submit bot commented Jan 31, 2024

auto label is removed for flutter/engine/49980, due to - The status or check suite Linux linux_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

@chinmaygarde
Copy link
Member

The presub is failing on a formatting error.

@bartcone bartcone force-pushed the bugfix/text-input-type-none-physical-keyboard branch from be15393 to 820865e Compare February 2, 2024 01:29
@bartcone
Copy link
Contributor Author
bartcone commented Feb 2, 2024

All green again @reidbaker @chinmaygarde

@johnmccutchan johnmccutchan merged commit e292632 into flutter:main Feb 2, 2024
25 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 2, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 2, 2024
@engine-flutter-autoroll
e29263212 [Android] Fix TextInputType.none for devices with physical …
5a84eb8 
…keyboard (flutter/engine#49980)
zanderso pushed a commit to flutter/flutter that referenced this pull request Feb 2, 2024
@engine-flutter-autoroll
Roll Flutter Engine from b35153d00b2e to e29263212bfd (2 revisions) (#…
10e6f4b 
…142799)

flutter/engine@b35153d...e292632

2024-02-02 cone.bart@gmail.com [Android] Fix TextInputType.none for
devices with physical keyboard (flutter/engine#49980)
2024-02-02 98614782+auto-submit[bot]@users.noreply.github.com Reverts
"Add the focus state related methods to the platform dispatcher"
(flutter/engine#50268)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC matanl@google.com,rmistry@google.com,zra@google.com on the
revert to ensure that a human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@kishankumawat03
Copy link

I have upgraded my Flutter version to 3.19.6. but I am still having the issue. I am unable to receive data from the physical keyboard when keyboardType is set to TextInputType.none on a TextField.

@bartcone
Copy link
Contributor Author

I have upgraded my Flutter version to 3.19.6. but I am still having the issue. I am unable to receive data from the physical keyboard when keyboardType is set to TextInputType.none on a TextField.

flutter/flutter#89983 (comment)

@kishankumawat03
Copy link
kishankumawat03 commented May 17, 2024 •
edited
Loading

Every thing working fine. For the first time I have to use a physical keyboard to take input. After that I am able to take input from the barcode scanning device. i am using Newland Barcode Scanner. i have updated flutter to 3.22.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johnmccutchan johnmccutchan johnmccutchan approved these changes

@reidbaker reidbaker reidbaker approved these changes

@LongCatIsLooong LongCatIsLooong LongCatIsLooong approved these changes

Assignees
No one assigned
Labels
platform-android
Projects
None yet
Milestone
No milestone
6 participants
@bartcone @reidbaker @chinmaygarde @kishankumawat03 @johnmccutchan @LongCatIsLooong