Fix Hangul output in iOS and in Safari#2454
Conversation
| if (deleteSize > 0) { | ||
| add(DeleteSurroundingTextCommand(deleteSize, 0)) | ||
| } else if (deleteSize == 0) { | ||
| add(BackspaceCommand()) |
There was a problem hiding this comment.
As I remember from the logs you shared from iPad, only deleteContentBackward was dispatched there for this type of input.
Is it necessary to add this check (deleteSize == 0) here too?
The name of the events suggests that deleteContentBackward can indeed have no selection - so just delete like a backspace would do.
But insertReplacementText hints that a selection must be present to replace something.
There was a problem hiding this comment.
good catch! - this is fallacy of tuning something to test - (in this case, for SafariCompositeInput::input hangul-hol`) let me fix test instead
| // The system first tells us to delete the old text, | ||
| // and then it would send the "insertText" event. | ||
| val deleteSize = deleteContentBackwardSize | ||
| val deleteSize = deleteContentBackwardSize ?: 0 |
There was a problem hiding this comment.
in this case it will go to else if branch and execute Backspace command.
There was a problem hiding this comment.
@eymar shouldn't we treat the null (deleteContentBackwarSize was not set at all) like zero?
This leads to Removing Safari:: `input hangul-hol` test - unfortunately we can not change state of html input which is needed for this test to behave properly Other possible solution would be not to set deleteContentBackwardSize if it somehow came from event already but this will mean we leak testing logic into application. This price would be too high
2 participants
Correct behaviour for Hangul input under certain circumstances
Fixes [https://youtrack.jetbrains.com/issue/CMP-8773]
Testing
./gradlew testWebRelease Notes
Fixes - Web