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

The first letter is typing twice on Android #5493

Open
dudtjr913 opened this issue Jul 31, 2023 · 11 comments
Open

The first letter is typing twice on Android #5493

dudtjr913 opened this issue Jul 31, 2023 · 11 comments

Comments

@dudtjr913
Copy link

Description
When I type Korean in Android, the first letter is entered twice.
You can also check it on slate-react example.
You can check it by changing the device to Android in developer tools and then refreshing.

Recording
works fine on pc

default.mov

but doesn't work on android

default.mov

Steps
To reproduce the behavior:

  1. Go to slate-react-example
  2. open developer tools
  3. Change Android device
  4. Refresh
  5. Typing
  6. See the twice typing

Expectation
it should be typed only once.

Environment

  • Slate Version:
    "slate": "^0.94.1",
    "slate-history": "^0.93.0",
    "slate-hyperscript": "^0.77.0",
    "slate-react": "^0.98.0",
  • Operating System: [e.g. MAC]
  • Browser: [e.g. Android, Chrome]
@lauigi
Copy link
Contributor

lauigi commented Sep 4, 2023

I reproduce this behaviour when composing Chinese Characters in the same env

@rnike
Copy link

rnike commented Sep 21, 2023

We are also facing this problem

@rnike
Copy link

rnike commented Sep 28, 2023

This is also happening when compose in English

Screen.Recording.2023-09-28.at.12.24.48.PM.mov

@rnike
Copy link

rnike commented Sep 28, 2023

Digging into the source code

When the issue occur, the Range.isCollapsed(at) returns true at line 74 in text.ts below

if (Range.isCollapsed(at)) {
at = at.anchor
} else {
const end = Range.end(at)
if (!voids && Editor.void(editor, { at: end })) {
return
}
const start = Range.start(at)
const startRef = Editor.pointRef(editor, start)
const endRef = Editor.pointRef(editor, end)
Transforms.delete(editor, { at, voids })
const startPoint = startRef.unref()
const endPoint = endRef.unref()
at = startPoint || endPoint!
Transforms.setSelection(editor, { anchor: at, focus: at })
}

At this moment the offset of anchor and focus from at is both 1 which is unexpected, should be 0 for anchor and 1 for focus

// should be
{
  anchor: {
    path: [3, 0],
    offset: 0,
  },
  focus: {
    path: [3, 0],
    offset: 1,
  },
}

// but is 
{
  anchor: {
    path: [3, 0],
    offset: 1, <- this is incorrect
  },
  focus: {
    path: [3, 0],
    offset: 1,
  },
}

The variable at is from getDefaultInsertLocation

let { at = getDefaultInsertLocation(editor) } = options

Don't know if is something wrong with the normalization

@rnike
Copy link

rnike commented Sep 28, 2023

Update, I print out the apply operation from editor below

Screen Shot 2023-09-28 at 2 52 00 PM

it seems like there is missing the set_selection operation at the second try

@rnike
Copy link

rnike commented Oct 2, 2023

Update, I print out the apply operation from editor below

Screen Shot 2023-09-28 at 2 52 00 PM

it seems like there is missing the set_selection operation at the second try

Found out it might be an issue from AndroidInputManager, because the following if statement is false and will not trigger Transforms.select(editor, range) when it is going to have duplicated letters

if (!editor.selection || !Range.equals(editor.selection, range)) {
Transforms.select(editor, range)
}

@rnike
Copy link

rnike commented Oct 5, 2023

Maybe we need help from @BitPhinix, the author of AndroidInputManager

@rnike
Copy link

rnike commented Oct 5, 2023

Saw this workaround at handleDOMBeforeInput

// COMPAT: Swiftkey has a weird bug where the target range of the 2nd word
// inserted after a mark placeholder is inserted with an anchor offset off by 1.
// So writing 'some text' will result in 'some ttext'. Luckily all 'normal' insert
// text events are fired with the correct target ranges, only the final 'insertComposition'
// isn't, so we can adjust the target range start offset if we are confident this is the
// swiftkey insert causing the issue.
if (text && insertPositionHint && type === 'insertCompositionText') {
const hintPosition =
insertPositionHint.start + insertPositionHint.text.search(/\S|$/)
const diffPosition = diff.start + diff.text.search(/\S|$/)
if (
diffPosition === hintPosition + 1 &&
diff.end ===
insertPositionHint.start + insertPositionHint.text.length
) {
debug('adjusting swiftKey insert position using hint')
diff.start -= 1
insertPositionHint = null
scheduleFlush()
} else {
insertPositionHint = false
}

Seems like we need another workaround to modify the start and end position, for example

             } else if (type === 'insertCompositionText') {
              if (_text.length !== _diff.end - _diff.start + 1) {
                _diff.start = _diff.end - _text.length + 1

                if (_diff.start < 0) {
                  const offset = Math.abs(_diff.start);
                  _diff.start += offset;
                  _diff.end += offset;
                }

                insertPositionHint = null;
                scheduleFlush();
              } else {
                insertPositionHint = false;
              }
            } 

NOTE: Workaround above has side effects and cannot be used in production.

@enex
Copy link

enex commented Mar 27, 2024

Does this code change fix the behaviour? Will it be integrated into slate-react?

@Climbatize
Copy link

Facing the issue with Japanese IME too. First try of the fix above is not working. I'll investigate a bit more

@rnike
Copy link

rnike commented May 22, 2024

Sorry my bad, the workaround above has side effects and cannot be used in production, I'm just trying to declare the position issue from the editor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants