Fix block tree before/after comparison

Summary:
In some cases, we need to prevent native insertion because native insertion would leave the decorators in the wrong place (and critically – editOnInput's attempt to patch things up would just make things worse and process the insertion twice). This new logic should be correct.

Previously, typing '#' before 'f' in 'x #foo' would result in 'x ###foo' (extra '#' inserted).

Reviewed By: mitermayer

Differential Revision: D7941738

fbshipit-source-id: eeff55ff082fc80c19f76fe1ea915bcbf0e7d86e

src/component/handlers/edit/__tests__/editOnBeforeInput.test.js

@@ -14,11 +14,14 @@
 'use strict';
 
 jest.disableAutomock();
+jest.mock('gkx', () => name => name === 'draft_improved_decorator_fingerprint');
 
+const CompositeDraftDecorator = require('CompositeDraftDecorator');
 const ContentBlock = require('ContentBlock');
 const ContentState = require('ContentState');
 const EditorState = require('EditorState');
 const SelectionState = require('SelectionState');
+
 const onBeforeInput = require('editOnBeforeInput');
 
 const DEFAULT_SELECTION = {
@@ -40,20 +43,20 @@ const rangedSelectionBackwards = new SelectionState({
   isBackward: true,
 });
 
-const getEditorState = () => {
+const getEditorState = (text: string = 'Arsenal') => {
   return EditorState.createWithContent(
     ContentState.createFromBlockArray([
       new ContentBlock({
         key: 'a',
-        text: 'Arsenal',
+        text,
       }),
     ]),
   );
 };
 
 const getInputEvent = data => ({
   data,
-  preventDefault: () => {},
+  preventDefault: jest.fn(),
 });
 
 test('editor is not updated if no character data is provided', () => {
@@ -151,3 +154,72 @@ test('editor selectionstate is updated if new text matches current selection and
   const newEditorState = editor.update.mock.calls[0][0];
   expect(newEditorState.getSelection()).toMatchSnapshot();
 });
+
+const HASHTAG_REGEX = /#[a-z]+/g;
+function hashtagStrategy(contentBlock, callback, contentState) {
+  findWithRegex(HASHTAG_REGEX, contentBlock, callback);
+}
+
+function findWithRegex(regex, contentBlock, callback) {
+  const text = contentBlock.getText();
+  let matchArr, start;
+  while ((matchArr = regex.exec(text)) !== null) {
+    start = matchArr.index;
+    callback(start, start + matchArr[0].length);
+  }
+}
+
+function testDecoratorFingerprint(
+  text,
+  selection,
+  charToInsert,
+  shouldPrevent,
+) {
+  const editorState = EditorState.acceptSelection(
+    EditorState.set(getEditorState(text), {
+      decorator: new CompositeDraftDecorator([
+        {
+          strategy: hashtagStrategy,
+          component: null,
+        },
+      ]),
+    }),
+    new SelectionState({
+      ...DEFAULT_SELECTION,
+      anchorOffset: selection,
+      focusOffset: selection,
+    }),
+  );
+
+  const editor = {
+    _latestEditorState: editorState,
+    _latestCommittedEditorState: editorState,
+    props: {},
+    update: jest.fn(),
+  };
+
+  const ev = getInputEvent(charToInsert);
+  onBeforeInput(editor, ev);
+
+  expect(ev.preventDefault.mock.calls.length).toBe(shouldPrevent ? 1 : 0);
+}
+
+test('decorator fingerprint logic bails out of native insertion', () => {
+  const oldGetSelection = global.getSelection;
+  try {
+    global.getSelection = () => ({});
+
+    // Make sure we prevent native insertion in the right cases
+    testDecoratorFingerprint('hi #', 4, 'f', true);
+    testDecoratorFingerprint('x #foo', 3, '#', true);
+    testDecoratorFingerprint('#foobar', 4, ' ', true);
+
+    // but these are OK to let through
+    testDecoratorFingerprint('#foo', 4, 'b', false);
+    testDecoratorFingerprint('#foo bar #baz', 2, 'o', false);
+    testDecoratorFingerprint('#foo bar #baz', 7, 'o', false);
+    testDecoratorFingerprint('#foo bar #baz', 12, 'a', false);
+  } finally {
+    global.getSelection = oldGetSelection;
+  }
+});

src/component/handlers/edit/editOnBeforeInput.js

@@ -21,6 +21,7 @@ const EditorState = require('EditorState');
 const UserAgent = require('UserAgent');
 
 const getEntityKeyForSelection = require('getEntityKeyForSelection');
+const gkx = require('gkx');
 const isEventHandled = require('isEventHandled');
 const isSelectionAtLeafStart = require('isSelectionAtLeafStart');
 const nullthrows = require('nullthrows');
@@ -187,16 +188,73 @@ function editOnBeforeInput(
     }
   }
   if (!mustPreventNative) {
-    // Check the old and new "fingerprints" of the current block to determine
-    // whether this insertion requires any addition or removal of text nodes,
-    // in which case we would prevent the native character insertion.
-    const originalFingerprint = BlockTree.getFingerprint(
-      editorState.getBlockTree(anchorKey),
-    );
-    const newFingerprint = BlockTree.getFingerprint(
-      newEditorState.getBlockTree(anchorKey),
-    );
-    mustPreventNative = originalFingerprint !== newFingerprint;
+    if (gkx('draft_improved_decorator_fingerprint')) {
+      // Let's say we have a decorator that highlights hashtags. In many cases
+      // we need to prevent native behavior and rerender ourselves --
+      // particularly, any case *except* where the inserted characters end up
+      // anywhere except exactly where you put them.
+      //
+      // Using [] to denote a decorated leaf, some examples:
+      //
+      // 1. 'hi #' and append 'f'
+      // desired rendering: 'hi [#f]'
+      // native rendering would be: 'hi #f' (incorrect)
+      //
+      // 2. 'x [#foo]' and insert '#' before 'f'
+      // desired rendering: 'x #[#foo]'
+      // native rendering would be: 'x [##foo]' (incorrect)
+      //
+      // 3. '[#foobar]' and insert ' ' between 'foo' and 'bar'
+      // desired rendering: '[#foo] bar'
+      // native rendering would be: '[#foo bar]' (incorrect)
+      //
+      // 4. '[#foo]' and delete '#' [won't use this beforeinput codepath though]
+      // desired rendering: 'foo'
+      // native rendering would be: '[foo]' (incorrect)
+      //
+      // 5. '[#foo]' and append 'b'
+      // desired rendering: '[#foob]'
+      // native rendering would be: '[#foob]' (native insertion is OK here)
+      //
+      // It is safe to allow native insertion if and only if the full list of
+      // decorator ranges matches what we expect native insertion to give. We
+      // don't need to compare the content because the only possible mutation
+      // to consider here is inserting plain text and decorators can't affect
+      // text content.
+      const oldBlockTree = editorState.getBlockTree(anchorKey);
+      const newBlockTree = newEditorState.getBlockTree(anchorKey);
+      mustPreventNative =
+        oldBlockTree.size !== newBlockTree.size ||
+        oldBlockTree.zip(newBlockTree).some(([oldLeafSet, newLeafSet]) => {
+          // selectionStart is guaranteed to be selectionEnd here
+          const oldStart = oldLeafSet.get('start');
+          const adjustedStart =
+            oldStart + (oldStart >= selectionStart ? chars.length : 0);
+          const oldEnd = oldLeafSet.get('end');
+          const adjustedEnd =
+            oldEnd + (oldEnd >= selectionStart ? chars.length : 0);
+          return (
+            // Different decorators
+            oldLeafSet.get('decoratorKey') !== newLeafSet.get('decoratorKey') ||
+            // Different number of inline styles
+            oldLeafSet.get('leaves').size !== newLeafSet.get('leaves').size ||
+            // Different effective decorator position
+            adjustedStart !== newLeafSet.get('start') ||
+            adjustedEnd !== newLeafSet.get('end')
+          );
+        });
+    } else {
+      // Check the old and new "fingerprints" of the current block to determine
+      // whether this insertion requires any addition or removal of text nodes,
+      // in which case we would prevent the native character insertion.
+      const originalFingerprint = BlockTree.getFingerprint(
+        editorState.getBlockTree(anchorKey),
+      );
+      const newFingerprint = BlockTree.getFingerprint(
+        newEditorState.getBlockTree(anchorKey),
+      );
+      mustPreventNative = originalFingerprint !== newFingerprint;
+    }
   }
   if (!mustPreventNative) {
     mustPreventNative = mustPreventDefaultForCharacter(chars);