[Android] Context manipulation is working incorrectly in Gmail app

MakaraSok commented 4 years ago

Describe the bug The character rotation feature does not give the expected outputs. It instead deletes character (s) before the character which has to be rotated leaving incomplete string of characters, i.e. gibberish.

To Reproduce Steps to reproduce the behavior:

Go to 'Gmail app'
Click to compose a new email
Start to type the character in the following orders using Khmer Angkor keyboard.
- (Type: sea) ស េ ា > ោ not the expected សោ
- (Type: seu) ស េ ុ > ុ not សុ
- (Type: suI) សុ ី > ស៊ី
- (Type kNjd) ក ណ ្ត > ណ្ដ not កណ្ដ
- (Type xEjm) ខ ែ ្ម > ្ម not ខ្មែ
See error

Expected behavior The rotation should not delete any character which does not belong in the rules.

Screenshots https://youtu.be/MpU7VAmhpB8

Android:

Device: [Galaxy S9+]
OS: [Android 9.0]
Keyman version [12.0.4205]
Target application: [Gmail for Android version 2019.09.15.270135155.release]

Keyboard

Keyboard name: [khmer_angkor]
Keyboard version: [1.0.1]
Language name: [Khmer]

Additional context

I've tested this behavior in Google Keep, and it works fine there.
Typing in Gmail subject field works fine

darcywong00 commented 4 years ago

Minimal keyboard for repro

store(&NAME) 'moo'
store(&VISUALKEYBOARD) 'moo.kvks'
store(&TARGETS) 'windows mobile'
store(&LAYOUTFILE) 'moo.keyman-touch-layout'
begin Unicode > use(main)

group(main) using keys

'p' + 'p' > 'ṗ'

'ṗ' + 'p' > 'p̂'

'p̂' + 'p' > 'p̂p̂'

'p̂p̂' + 'p' > 'p'

Link to moo.kmp

darcywong00 commented 4 years ago

Using gmail webview (Chrome browser) on the Android device

In all these fields, the rotas eventually delete characters:

To:
Cc/Bcc:
Subject:
Body:

Typing in web form input fields on the Android device

Typing on a website that has an input text field, the rotas delete characters

darcywong00 commented 4 years ago

Additional info from more investigation

Test Scenario

On an Android device, I can replicate the issue on an input field in Chrome
```
<form>
<input type='text'>
</form>
```

so it's not isolated to Gmail app.

Note: ṗ is two code points p 0070 Latin small latter p ̇ 0307 combining dot above

p̂ is two code points p 0070 Latin small letter p ̂ 0302 combining circumflex accent

Context	Type	Expected	Actual
	o	o	o
o	p	op	op
op	p	oṗ	oṗ
oṗ	p	op̂	p̂

Issue seen in these configs:

Keyman Android 10 and Keyman 11
Gmail web on Chrome (editing the body)

Issue not seen in these configs:

Gmail web on Firefox (editing the body)
On a Windows 10 desktop, using the keyboard on a KMW test page

The applicable Keyman code is KMManager for performing left-deletions

          // Perform left-deletions
          for (int i = 0; i < dn; i++) {
            CharSequence chars = ic.getTextBeforeCursor(1, 0);
            if (chars != null && chars.length() > 0) {
              char c = chars.charAt(0);
              SystemKeyboardShouldIgnoreSelectionChange = true;
              if (Character.isLowSurrogate(c)) {
                ic.deleteSurroundingText(2, 0);
              } else {
                ic.deleteSurroundingText(1, 0);
              }
            }
          }

From the documentation, deleteSurroundingText deletes 1 Java character, not code points.

deleteSurroundingTextInCodePoints should delete 1 code point, but it still deleted the entire ṗ

Reference

Chromium code review re: deleteSurroundingText

mcdurdin commented 4 years ago

This is definitely a bug in Chromium. This has been documented as a TODO but not listed as a bug by Chromium as of issue date.

However, we need a workaround because it will take a long time for this fix, if and when it lands, to filter down to all the devices we support.

Proposal:

Retrieve the text before cursor, CharSequence charsBackup = ic.getTextBeforeCursor(dn*2 + 16, 0); into a buffer. This ensures we collect enough characters for surrogate pairs + a long grapheme cluster at the start of the text.
Count the number of surrogate pairs in this buffer, backwards from the end until we reach dn codepoints (remember dn is in code points, not code units).
Update charsBackup to delete dn+numPairs from the right of the buffer.
Now tell the app to do the same (ic.deleteSurroundingText(dn+numPairs, 0);).
Now we run into the Chromium bug: it may have deleted more than we want it to, all the way up to a grapheme cluster boundary. The good news is we should just be able to put it back again because we've kept a copy in charsBackup.
So, getTextBeforeCursor() again. Slide this new buffer left until it matches the text in charsBackup. The dangling characters in charsBackup should be prepended to the text to be inserted in ic.commitText further down.

Does that make sense? There's some detail I've elided there but hoping that's enough to get teeth into it!

Finally, we should raise this as a bug with Chromium as well.

darcywong00 commented 4 years ago

Raised Chromium bug report 1024738

mcdurdin commented 1 year ago

Note, the Chromium fix landed in M81 for when we back this compat fix out in the future.

keymanapp / keyman