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

[Android/IME] Can't exit a list with the Enter key #12368

Closed
Mgsy opened this issue Aug 31, 2022 · 6 comments
Closed

[Android/IME] Can't exit a list with the Enter key #12368

Mgsy opened this issue Aug 31, 2022 · 6 comments
Assignees
Labels
browser:android domain:typing/ime This issue reports a problem with standard typing & IME (typing method for CJK languages). squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@Mgsy
Copy link
Member

Mgsy commented Aug 31, 2022

📝 Provide detailed reproduction steps (if any)

  1. Create a numbered list.
  2. Type "Test".
  3. Press Space.
  4. Press Enter twice.

✔️ Expected result

The second list item disappears and you exit the list.

❌ Actual result

New list items appear after pressing Enter.

📃 Other details

Branch: ck/11438-beforeinput-ime-research-vol1.1-android
Language: English
Keyboards: GBoard
Device: Nexus 10 emulator@Android v12


If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@Mgsy Mgsy added type:bug This issue reports a buggy (incorrect) behavior. domain:typing/ime This issue reports a problem with standard typing & IME (typing method for CJK languages). squad:core Issue to be handled by the Core team. browser:android labels Aug 31, 2022
@Reinmar
Copy link
Member

Reinmar commented Aug 31, 2022

I tested this on Chrome 104.0.5112.97 and it works for me.

Aug-31-2022.10-37-47.mp4

Could you run it in the manual test yarn manual --files typing -d typing, plug Chrome inspector, enable the "log native events" option (at the top of the manual test) and post here:

  • a screencast
  • the console logs

@Reinmar
Copy link
Member

Reinmar commented Aug 31, 2022

Ok, I managed to reproduce this.

The thing is that pressing the space (I need to do that twice in my case to see anything) inserts one space before the caret and one after the caret.

Since there's one space after the caret pressing Enter in what seems to be an empty line doesn't work because it's not an empty line. It's:

<li>[] </li>

@Reinmar
Copy link
Member

Reinmar commented Aug 31, 2022

An alternative scenario here is to:

  1. Create a list item
  2. Type "Test"
  3. Press Enter. Assuming that we're still in the composition mode (the "Test" is underlined) Chrome tends to send a beforeInput:insertText with \n string instead of beforeInput:insertParagraph. We add this \n to the model but it gets added after the selection. 
  4. Press Space. It does nothing visible. The space gets inserted into the model but again - after the selection.
  5. Press Space again. Now it works. Composition ends.
  6. Press Enter. Now it works (because we're not in composition and apparently normal beforeInput:insertParagraph is fired)
  7. Press Enter. It does not escape the list because there's content after the selection – ' \n'.

I reproduced the above in the all-features manual test but most likely it's reproducible everywhere.

@Reinmar Reinmar changed the title [IME] Can't exit a list with the Enter key [Android/IME] Can't exit a list with the Enter key Sep 7, 2022
@Reinmar
Copy link
Member

Reinmar commented Sep 7, 2022

Comment from the main Android thread: #12058 (comment)

@Reinmar
Copy link
Member

Reinmar commented Sep 7, 2022

Implementation idea: let's forward the beforeInput:insertCompositionText with 'Word\n' to an enter event and stop the original flow (so the default handler for insert text is not executed).

The idea is to do this only on Android and as early as possible, so still in the InputObserver's code. This heuristic will look for any insertCompositionText event which data ends with \n.

Theoretically, this specific event may be fired on Android in different scenarios than Enter key, so we might lose some text that the user wanted to insert. However, we don't know about any such scenarios right now, plus, it wouldn't work anyway.

Resolving this issue may also resolve:

@CKEditorBot CKEditorBot added the status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. label Sep 7, 2022
@niegowski niegowski self-assigned this Sep 7, 2022
@CKEditorBot CKEditorBot added status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. and removed status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. labels Sep 7, 2022
@Mgsy
Copy link
Member Author

Mgsy commented Sep 8, 2022

Fixed on ck/11438-beforeinput-ime-research-vol1.1-android.

@Mgsy Mgsy closed this as completed Sep 8, 2022
@Mgsy Mgsy added this to the iteration 57 milestone Sep 8, 2022
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Sep 8, 2022
@Reinmar Reinmar modified the milestones: iteration 57, upcoming Sep 13, 2022
@Reinmar Reinmar modified the milestones: upcoming, iteration 58 Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser:android domain:typing/ime This issue reports a problem with standard typing & IME (typing method for CJK languages). squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

4 participants