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

Fixing major Android editing issues #2035

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
83730c9
Much saner experience, but still with known issues that Im taking notes.
fabiomcosta Mar 2, 2019
939f935
Merge branch 'master' of https://github.com/facebook/draft-js into at…
fabiomcosta Mar 8, 2019
c831fcf
Improving code a bit
fabiomcosta Mar 8, 2019
1d55ab4
minor cleanup
fabiomcosta Mar 8, 2019
7322ff4
fixing flow issues
fabiomcosta Mar 8, 2019
7e67808
revert header changes
fabiomcosta Mar 8, 2019
314e3db
keeping draft_handlebeforeinput_composed_text experiment
fabiomcosta Mar 8, 2019
65120e8
Removing code added by accident
fabiomcosta Mar 8, 2019
8c3e6ab
Merge branch 'master' of https://github.com/facebook/draft-js into at…
fabiomcosta Mar 12, 2019
2f7ac3f
fixing more issues realted to composition, but I finaly noticed this …
fabiomcosta Mar 13, 2019
1800a0e
Detecting DOM changes using a mutation observer on composition events
fabiomcosta Mar 14, 2019
49ddeb5
removing console.logs and removing unecessary compositionUpdate handler
fabiomcosta Mar 15, 2019
b10bcc7
improving comments around the new onInput changes
fabiomcosta Mar 15, 2019
2be6cfb
fixing flow and lint errors and warnings
fabiomcosta Mar 16, 2019
2fbe986
v0.10.6-beta.0
fabiomcosta Mar 16, 2019
fbfb983
adding unit tests for getContentEditableContainer
fabiomcosta Mar 16, 2019
4b4c028
const editorState
fabiomcosta Mar 16, 2019
9ff84ce
better info on keeping the selection state.
fabiomcosta Mar 16, 2019
586ce9e
Fixes an issue that happens when the target textContent is an empty s…
fabiomcosta Mar 17, 2019
a560b8b
v0.10.6-beta.1
fabiomcosta Mar 17, 2019
9e0ac50
Making sure multiple mutations are properly applied to the editorStat…
fabiomcosta Mar 18, 2019
d3a3f98
v0.10.6-beta.2
fabiomcosta Mar 18, 2019
96d810d
reverting package name changes
fabiomcosta Mar 18, 2019
2bec631
adding unit tests for the editOnInput changes
fabiomcosta Mar 19, 2019
e4fdd89
cleanup unit tests and add "editor.update" check
fabiomcosta Mar 19, 2019
c11bce3
Adding unit tests for DraftEditorCompositionHandler. It is still miss…
fabiomcosta Mar 19, 2019
121997f
removing lines of code I was using just for testing and some unused r…
fabiomcosta Mar 19, 2019
2c95809
This change became unecessary after the new way of handling compositi…
fabiomcosta Mar 20, 2019
997edf7
Fixes style application issue on selection
fabiomcosta Mar 22, 2019
0c4bf1b
Merge branch 'master' of https://github.com/facebook/draft-js into at…
fabiomcosta Mar 22, 2019
6219a3f
Revert incorrect changes to the example file
fabiomcosta Mar 22, 2019
eef794e
Merge branch 'master' of https://github.com/facebook/draft-js into at…
fabiomcosta May 1, 2019
5a015a7
Improving invariant message
fabiomcosta May 1, 2019
5c70301
Merge branch 'master' of https://github.com/facebook/draft-js into at…
fabiomcosta May 7, 2019
2927e4f
updates based on CR
fabiomcosta May 7, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion meta/bundle-size-stats/Draft.js.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion meta/bundle-size-stats/Draft.min.js.json

Large diffs are not rendered by default.

136 changes: 136 additions & 0 deletions src/component/handlers/composition/DOMObserver.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @format
* @flow strict-local
* @emails oncall+draft_js
*/

'use strict';

const UserAgent = require('UserAgent');
const Immutable = require('immutable');

const findAncestorOffsetKey = require('findAncestorOffsetKey');
const invariant = require('invariant');
const nullthrows = require('nullthrows');

const {Map} = Immutable;

type MutationRecordT =
| MutationRecord
| {|type: 'characterData', target: Node, removedNodes?: void|};

// Heavily based on Prosemirror's DOMObserver https://github.com/ProseMirror/prosemirror-view/blob/master/src/domobserver.js

const DOM_OBSERVER_OPTIONS = {
subtree: true,
characterData: true,
childList: true,
characterDataOldValue: false,
attributes: false,
};
// IE11 has very broken mutation observers, so we also listen to DOMCharacterDataModified
const USE_CHAR_DATA = UserAgent.isBrowser('IE <= 11');

class DOMObserver {
observer: ?MutationObserver;
container: HTMLElement;
mutations: Map<string, string>;
onCharData: ?({target: EventTarget, type: string}) => void;

constructor(container: HTMLElement) {
this.container = container;
this.mutations = Map();
if (window.MutationObserver && !USE_CHAR_DATA) {
this.observer = new window.MutationObserver(mutations =>
this.registerMutations(mutations),
);
} else {
this.onCharData = e => {
invariant(
e.target instanceof Node,
'Expected target to be an instance of Node',
);
this.registerMutation({
type: 'characterData',
target: e.target,
});
};
}
}

start() {
if (this.observer) {
this.observer.observe(this.container, DOM_OBSERVER_OPTIONS);
} else {
/* $FlowFixMe(>=0.68.0 site=www,mobile) This event type is not defined
* by Flow's standard library */
this.container.addEventListener(
'DOMCharacterDataModified',
this.onCharData,
);
}
}

stopAndFlushMutations() {
const {observer} = this;
if (observer) {
this.registerMutations(observer.takeRecords());
observer.disconnect();
} else {
/* $FlowFixMe(>=0.68.0 site=www,mobile) This event type is not defined
* by Flow's standard library */
this.container.removeEventListener(
'DOMCharacterDataModified',
this.onCharData,
);
}
const mutations = this.mutations;
this.mutations = Map();
return mutations;
}

registerMutations(mutations: Array<MutationRecord>) {
for (let i = 0; i < mutations.length; i++) {
this.registerMutation(mutations[i]);
}
}

getMutationTextContent(mutation: MutationRecordT) {
const {type, target, removedNodes} = mutation;
if (type === 'characterData') {
// When `textContent` is '', there is a race condition that makes
// getting the offsetKey from the target not possible.
// These events are also followed by a `childList`, which is the one
// we are able to retrieve the offsetKey and apply the '' text.
if (target.textContent !== '') {
return target.textContent;
}
} else if (type === 'childList') {
// `characterData` events won't happen or are ignored when
// removing the last character of a leaf node, what happens
// instead is a `childList` event with a `removedNodes` array.
// For this case the textContent should be '' and
// `DraftModifier.replaceText` will make sure the content is
// updated properly.
if (removedNodes && removedNodes.length) {
return '';
}
}
return null;
}

registerMutation(mutation: MutationRecordT) {
const textContent = this.getMutationTextContent(mutation);
if (textContent != null) {
const offsetKey = nullthrows(findAncestorOffsetKey(mutation.target));
this.mutations = this.mutations.set(offsetKey, textContent);
}
}
}

module.exports = DOMObserver;
166 changes: 102 additions & 64 deletions src/component/handlers/composition/DraftEditorCompositionHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@ import type DraftEditor from 'DraftEditor.react';
const DraftModifier = require('DraftModifier');
const EditorState = require('EditorState');
const Keys = require('Keys');
const DraftOffsetKey = require('DraftOffsetKey');
const DOMObserver = require('DOMObserver');

const nullthrows = require('nullthrows');
const getEntityKeyForSelection = require('getEntityKeyForSelection');
const gkx = require('gkx');
const isEventHandled = require('isEventHandled');
const isSelectionAtLeafStart = require('isSelectionAtLeafStart');
const getDraftEditorSelection = require('getDraftEditorSelection');
const getContentEditableContainer = require('getContentEditableContainer');
const editOnSelect = require('editOnSelect');

/**
* Millisecond delay to allow `compositionstart` to fire again upon
Expand All @@ -42,19 +45,23 @@ const RESOLVE_DELAY = 20;
*/
let resolved = false;
let stillComposing = false;
let textInputData = '';
let domObserver = null;

const DraftEditorCompositionHandler = {
onBeforeInput: function(editor: DraftEditor, e: SyntheticInputEvent<>): void {
textInputData = (textInputData || '') + e.data;
},
function startDOMObserver(editor: DraftEditor) {
if (!domObserver) {
domObserver = new DOMObserver(getContentEditableContainer(editor));
domObserver.start();
}
}

var DraftEditorCompositionHandler = {
/**
* A `compositionstart` event has fired while we're still in composition
* mode. Continue the current composition session to prevent a re-render.
*/
onCompositionStart: function(editor: DraftEditor): void {
stillComposing = true;
startDOMObserver(editor);
},

/**
Expand All @@ -71,16 +78,18 @@ const DraftEditorCompositionHandler = {
* twice could break the DOM, we only use the first event. Example: Arabic
* Google Input Tools on Windows 8.1 fires `compositionend` three times.
*/
onCompositionEnd: function(editor: DraftEditor, e: SyntheticEvent<>): void {
onCompositionEnd: function(editor: DraftEditor): void {
resolved = false;
stillComposing = false;
setTimeout(() => {
if (!resolved) {
DraftEditorCompositionHandler.resolveComposition(editor, e);
DraftEditorCompositionHandler.resolveComposition(editor);
}
}, RESOLVE_DELAY);
},

onSelect: editOnSelect,
Copy link
Contributor Author

@fabiomcosta fabiomcosta Mar 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this here fixes an issue where the selection wouldn't be properly updated during composition.
This would cause, for example, inline-styles to not be applied during composition mode.


/**
* In Safari, keydown events may fire when committing compositions. If
* the arrow keys are used to commit, prevent default so that the cursor
Expand All @@ -92,7 +101,7 @@ const DraftEditorCompositionHandler = {
// 20ms timer expires (ex: type option-E then backspace, or type A then
// backspace in 2-Set Korean), we should immediately resolve the
// composition and reinterpret the key press in edit mode.
DraftEditorCompositionHandler.resolveComposition(editor, e);
DraftEditorCompositionHandler.resolveComposition(editor);
editor._onKeyDown(e);
return;
}
Expand Down Expand Up @@ -128,77 +137,106 @@ const DraftEditorCompositionHandler = {
* Resetting innerHTML will move focus to the beginning of the editor,
* so we update to force it back to the correct place.
*/
resolveComposition: function(
editor: DraftEditor,
event: SyntheticEvent<>,
): void {
resolveComposition: function(editor: DraftEditor): void {
if (stillComposing) {
return;
}

const mutations = nullthrows(domObserver).stopAndFlushMutations();
domObserver = null;
resolved = true;
const composedChars = textInputData;
textInputData = '';

const editorState = EditorState.set(editor._latestEditorState, {
let editorState = EditorState.set(editor._latestEditorState, {
inCompositionMode: false,
});

const currentStyle = editorState.getCurrentInlineStyle();
const entityKey = getEntityKeyForSelection(
editorState.getCurrentContent(),
editorState.getSelection(),
);

const mustReset =
!composedChars ||
isSelectionAtLeafStart(editorState) ||
currentStyle.size > 0 ||
entityKey !== null;
editor.exitCurrentMode();

if (mustReset) {
editor.restoreEditorDOM();
if (!mutations.size) {
editor.update(editorState);
return;
}

editor.exitCurrentMode();
// TODO, check if Facebook still needs this flag or if it could be removed.
// Since there can be multiple mutations providing a `composedChars` doesn't
// apply well on this new model.
Copy link
Contributor Author

@fabiomcosta fabiomcosta Mar 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claudiopro would you know if we can remove this code or do we need to keep it somehow working?
If we keep it working it will be buggy, because we are now applying multiple mutations (which leads to multiple composedChars) on compositionEnd, which is not compatible with the arguments that we provide to handleBeforeInput here.

If draft_handlebeforeinput_composed_text is still being used at FB, I'd suggest to provide the composedChars from the first mutation, completely ignoring the other mutations, which should be fine for the common case. My main suggestion is still to stop depending on this code/gkflag.

// if (
// gkx('draft_handlebeforeinput_composed_text') &&
// editor.props.handleBeforeInput &&
// isEventHandled(
// editor.props.handleBeforeInput(
// composedChars,
// editorState,
// event.timeStamp,
// ),
// )
// ) {
// return;
// }

let contentState = editorState.getCurrentContent();
mutations.forEach((composedChars, offsetKey) => {
const {blockKey, decoratorKey, leafKey} = DraftOffsetKey.decode(
offsetKey,
);

if (composedChars) {
if (
gkx('draft_handlebeforeinput_composed_text') &&
editor.props.handleBeforeInput &&
isEventHandled(
editor.props.handleBeforeInput(
composedChars,
editorState,
event.timeStamp,
),
)
) {
return;
}
// If characters have been composed, re-rendering with the update
// is sufficient to reset the editor.
const contentState = DraftModifier.replaceText(
editorState.getCurrentContent(),
editorState.getSelection(),
const {start, end} = editorState
.getBlockTree(blockKey)
.getIn([decoratorKey, 'leaves', leafKey]);

const replacementRange = editorState.getSelection().merge({
anchorKey: blockKey,
focusKey: blockKey,
anchorOffset: start,
focusOffset: end,
isBackward: false,
});

const entityKey = getEntityKeyForSelection(
contentState,
replacementRange,
);
const currentStyle = contentState
.getBlockForKey(blockKey)
.getInlineStyleAt(start);

contentState = DraftModifier.replaceText(
contentState,
replacementRange,
composedChars,
currentStyle,
entityKey,
);
editor.update(
EditorState.push(editorState, contentState, 'insert-characters'),
);
return;
}
// We need to update the editorState so the leaf node ranges are properly
// updated and multiple mutations are correctly applied.
editorState = EditorState.set(editorState, {
currentContent: contentState,
});
});

if (mustReset) {
editor.update(
EditorState.set(editorState, {
nativelyRenderedContent: null,
forceSelection: true,
}),
);
}
// When we apply the text changes to the ContentState, the selection always
// goes to the end of the field, but it should just stay where it is
// after compositionEnd.
const documentSelection = getDraftEditorSelection(
editorState,
getContentEditableContainer(editor),
);
const compositionEndSelectionState = documentSelection.selectionState;

editor.restoreEditorDOM();

const editorStateWithUpdatedSelection = EditorState.acceptSelection(
editorState,
compositionEndSelectionState,
);

editor.update(
EditorState.push(
editorStateWithUpdatedSelection,
contentState,
'insert-characters',
),
);
},
};

Expand Down
Loading