Skip to content
This repository was archived by the owner on Feb 6, 2023. It is now read-only.

Commit ae25b8f

Browse files
sophiebitsfacebook-github-bot
authored andcommitted
Fix unlucky failures in character replacement
Summary: Fixes #1830 (bug introduced in #719; this is a proper fix for #398). * When we're allowing native insertion, we don't want to modify the selection because doing so blocks browser features like spellcheck from working * In all other cases when typing, we *do* want to force the selection – failing to do this is why the original issue occurred * We use the `insert-characters` change type for both native and non-native insertions * Instead of guessing in `EditorState.push` whether to force a selection, accept it as a parameter * Gate all changes by `draft_non_native_insertion_forces_selection` I verified with the feature flag disabled (same behavior as before): * Typing over a character works, and the cursor moves as expected (because of the `chars === currentlySelectedChars` check) * With editor contents "ABC\ndef", replacing "d", "e", or "f" with the character above *fails* to replace the char (because that check is buggy). And with the feature flag enabled (new behavior): * Typing over a character still works (but does so without the buggy check) * With editor contents "ABC\ndef", replacing "d", "e", or "f" with the character above *succeeds* at replacing the char. Reviewed By: flarnie Differential Revision: D9209691 fbshipit-source-id: f63551dbad689391aa9c2a69f1d6ba395b8bf1ac
1 parent 2d7ad18 commit ae25b8f

File tree

2 files changed

+30
-4
lines changed

2 files changed

+30
-4
lines changed

src/component/handlers/edit/editOnBeforeInput.js

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ const EditorState = require('EditorState');
2020
const UserAgent = require('UserAgent');
2121

2222
const getEntityKeyForSelection = require('getEntityKeyForSelection');
23+
const gkx = require('gkx');
2324
const isEventHandled = require('isEventHandled');
2425
const isSelectionAtLeafStart = require('isSelectionAtLeafStart');
2526
const nullthrows = require('nullthrows');
@@ -36,6 +37,10 @@ const FF_QUICKFIND_CHAR = "'";
3637
const FF_QUICKFIND_LINK_CHAR = '/';
3738
const isFirefox = UserAgent.isBrowser('Firefox');
3839

40+
const nonNativeInsertionForcesSelection = gkx(
41+
'draft_non_native_insertion_forces_selection',
42+
);
43+
3944
function mustPreventDefaultForCharacter(character: string): boolean {
4045
return (
4146
isFirefox &&
@@ -52,6 +57,7 @@ function replaceText(
5257
text: string,
5358
inlineStyle: DraftInlineStyle,
5459
entityKey: ?string,
60+
forceSelection: boolean,
5561
): EditorState {
5662
const contentState = DraftModifier.replaceText(
5763
editorState.getCurrentContent(),
@@ -60,7 +66,12 @@ function replaceText(
6066
inlineStyle,
6167
entityKey,
6268
);
63-
return EditorState.push(editorState, contentState, 'insert-characters');
69+
return EditorState.push(
70+
editorState,
71+
contentState,
72+
'insert-characters',
73+
forceSelection,
74+
);
6475
}
6576

6677
/**
@@ -124,7 +135,10 @@ function editOnBeforeInput(
124135
.getCurrentContent()
125136
.getPlainText()
126137
.slice(selectionStart, selectionEnd);
127-
if (chars === currentlySelectedChars) {
138+
if (
139+
!nonNativeInsertionForcesSelection &&
140+
chars === currentlySelectedChars
141+
) {
128142
editor.update(
129143
EditorState.forceSelection(
130144
editorState,
@@ -144,6 +158,7 @@ function editOnBeforeInput(
144158
editorState.getCurrentContent(),
145159
editorState.getSelection(),
146160
),
161+
true,
147162
),
148163
);
149164
}
@@ -158,6 +173,7 @@ function editOnBeforeInput(
158173
editorState.getCurrentContent(),
159174
editorState.getSelection(),
160175
),
176+
false,
161177
);
162178

163179
// Bunch of different cases follow where we need to prevent native insertion.
@@ -254,6 +270,11 @@ function editOnBeforeInput(
254270

255271
if (mustPreventNative) {
256272
e.preventDefault();
273+
if (nonNativeInsertionForcesSelection) {
274+
newEditorState = EditorState.set(newEditorState, {
275+
forceSelection: true,
276+
});
277+
}
257278
editor.update(newEditorState);
258279
return;
259280
}

src/model/immutable/EditorState.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,11 @@ import type {List, OrderedMap} from 'immutable';
2222
const BlockTree = require('BlockTree');
2323
const ContentState = require('ContentState');
2424
const EditorBidiService = require('EditorBidiService');
25-
const Immutable = require('immutable');
2625
const SelectionState = require('SelectionState');
2726

27+
const gkx = require('gkx');
28+
const Immutable = require('immutable');
29+
2830
const {OrderedSet, Record, Stack} = Immutable;
2931

3032
type EditorStateRecordType = {
@@ -340,12 +342,15 @@ class EditorState {
340342
editorState: EditorState,
341343
contentState: ContentState,
342344
changeType: EditorChangeType,
345+
forceSelection: boolean = true,
343346
): EditorState {
344347
if (editorState.getCurrentContent() === contentState) {
345348
return editorState;
346349
}
347350

348-
const forceSelection = changeType !== 'insert-characters';
351+
if (!gkx('draft_non_native_insertion_forces_selection')) {
352+
forceSelection = changeType !== 'insert-characters';
353+
}
349354
const directionMap = EditorBidiService.getDirectionMap(
350355
contentState,
351356
editorState.getDirectionMap(),

0 commit comments

Comments
 (0)