Bug 1596920 - Don't set selection on dispatching key event if unnecessary. r=geckoview-reviewers,snorp

Actually we emulate key event (down, press and up) from replace transaction of
`Editable`. When dispatching key press, we always update current caret position.

Most situations is the following.
1. Dispatch keypress
2. Dispatch another keypress
3. Receive merged text/selection change result by 1. and 2.
4. Sync shadow (Java's Editable) text with 3.'s result. It means selection is
   correct position now.
5. Dispatch keypress with correct position.

When this issue occurs, the following situation occurs.
1. Dispatch keypress
2. Dispatch another keypress
3. Receive text/selection change result of 1.
4. Sync shadow (Java's Editable) text with 3.'s result. It means selection is
   old position now.
5. Dispatch another keypress with old position.
6. Receive text/selection change result of 2.
7. Receive text/selection change result of 5.

So when dispatching key press, we shouldn't always update selection if unnecessary.
Because selection range is already often set before dispatching key press.

Differential Revision: https://phabricator.services.mozilla.com/D71179
This commit is contained in:
Makoto Kato 2020-04-28 14:02:10 +00:00
parent 3b1e0f755c
commit fc29e1e8e1
2 changed files with 44 additions and 4 deletions

View File

@ -608,13 +608,39 @@ import android.view.inputmethod.EditorInfo;
// Because we get composition styling here essentially for free, // Because we get composition styling here essentially for free,
// we don't need to check if we're in batch mode. // we don't need to check if we're in batch mode.
if (!icMaybeSendComposition( if (icMaybeSendComposition(
action.mSequence, SEND_COMPOSITION_USE_ENTIRE_TEXT)) { action.mSequence, SEND_COMPOSITION_USE_ENTIRE_TEXT)) {
// Since we don't have a composition, we can try sending key events. mFocusedChild.onImeReplaceText(
sendCharKeyEvents(action); action.mStart, action.mEnd, action.mSequence.toString());
break;
}
// Since we don't have a composition, we can try sending key events.
sendCharKeyEvents(action);
// onImeReplaceText will set the selection range. But we don't
// know whether event state manager is processing text and
// selection. So current shadow may not be synchronized with
// Gecko's text and selection. So we have to avoid unnecessary
// selection update.
final int selStartOnShadow = Selection.getSelectionStart(mText.getShadowText());
final int selEndOnShadow = Selection.getSelectionEnd(mText.getShadowText());
int actionStart = action.mStart;
int actionEnd = action.mEnd;
// If action range is collapsed and selection of shadow text is
// collapsed, we may try to dispatch keypress on current caret
// position. Action range is previous range before dispatching
// keypress, and shadow range is new range after dispatching
// it.
if (action.mStart == action.mEnd && selStartOnShadow == selEndOnShadow &&
action.mStart == selStartOnShadow + action.mSequence.toString().length()) {
// Replacing range is same value as current shadow's selection.
// So it is unnecessary to update the selection on Gecko.
actionStart = -1;
actionEnd = -1;
} }
mFocusedChild.onImeReplaceText( mFocusedChild.onImeReplaceText(
action.mStart, action.mEnd, action.mSequence.toString()); actionStart, actionEnd, action.mSequence.toString());
break; break;
default: default:

View File

@ -877,7 +877,21 @@ bool GeckoEditableSupport::DoReplaceText(int32_t aStart, int32_t aEnd,
// the replaced text does not match our composition. // the replaced text does not match our composition.
textChanged |= RemoveComposition(); textChanged |= RemoveComposition();
#ifdef DEBUG_ANDROID_IME
{ {
nsEventStatus status = nsEventStatus_eIgnore;
WidgetQueryContentEvent selection(true, eQuerySelectedText, widget);
widget->DispatchEvent(&selection, status);
if (selection.mSucceeded) {
ALOGIME("IME: Current selection: { Offset=%u, Length=%u }",
selection.mReply.mOffset, selection.mReply.mString.Length());
}
}
#endif
// If aStart or aEnd is negative value, we use current selection instead
// of updating the selection.
if (aStart >= 0 && aEnd >= 0) {
// Use text selection to set target position(s) for // Use text selection to set target position(s) for
// insert, or replace, of text. // insert, or replace, of text.
WidgetSelectionEvent event(true, eSetSelection, widget); WidgetSelectionEvent event(true, eSetSelection, widget);