Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Applied upstream v0.12.5 patches to fix IME bug #15

Merged
merged 4 commits into from
Nov 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,9 @@
"saucie": "^3.3.2",
"testem": "^2.17.0"
},
"main": "dist/commonjs/mobiledoc-kit/index.js"
"main": "dist/commonjs/mobiledoc-kit/index.js",
"volta": {
"node": "10.22.1",
"yarn": "1.22.5"
}
}
2 changes: 2 additions & 0 deletions src/js/editor/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@ class Editor {
this._callbacks = new LifecycleCallbacks(values(CALLBACK_QUEUES));
this._beforeHooks = { toggleMarkup: [] };

this._isComposingOnBlankLine = false;

DEFAULT_TEXT_INPUT_HANDLERS.forEach(handler => this.onTextInput(handler));

this.hasRendered = false;
Expand Down
74 changes: 73 additions & 1 deletion src/js/editor/event-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,15 @@ import SelectionManager from 'mobiledoc-kit/editor/selection-manager';
import Browser from 'mobiledoc-kit/utils/browser';

const ELEMENT_EVENT_TYPES = [
'keydown', 'keyup', 'cut', 'copy', 'paste', 'keypress', 'drop'
'keydown',
'keyup',
'cut',
'copy',
'paste',
'keypress',
'drop',
'compositionstart',
'compositionend',
];

export default class EventManager {
Expand Down Expand Up @@ -146,6 +154,13 @@ export default class EventManager {
event.preventDefault();
}

// Handle carriage returns
if (!key.isEnter() && key.keyCode === 13) {
_textInputHandler.handleNewLine();
editor.handleNewline(event);
return;
}

_textInputHandler.handle(key.toString());
}

Expand All @@ -166,6 +181,10 @@ export default class EventManager {
let range = editor.range;

switch(true) {
// Ignore keydown events when using an IME
case key.isIME(): {
break;
}
// FIXME This should be restricted to only card/atom boundaries
case key.isHorizontalArrowWithoutModifiersOtherThanShift(): {
let newRange;
Expand Down Expand Up @@ -210,6 +229,59 @@ export default class EventManager {
this._updateModifiersFromKey(key, {isDown:false});
}

// The mutation handler interferes with IMEs when composing
// on a blank line. These two event handlers are for suppressing
// mutation handling in this scenario.
compositionstart(event) { // eslint-disable-line
let { editor } = this;
// Ignore compositionstart if not on a blank line
if (editor.range.headMarker) {
return;
}
this._isComposingOnBlankLine = true;

if (editor.post.isBlank) {
editor._insertEmptyMarkupSectionAtCursor();
}

// Stop listening for mutations on Chrome browsers and suppress
// mutations by prepending a character for other browsers.
// The reason why we treat these separately is because
// of the way each browser processes IME inputs.
if (Browser.isChrome()) {
editor.setPlaceholder('');
editor._mutationHandler.stopObserving();
} else {
this._textInputHandler.handle(' ');
}
}

compositionend(event) {
let { editor } = this;

// Ignore compositionend if not composing on blank line
if (!this._isComposingOnBlankLine) {
return;
}
this._isComposingOnBlankLine = false;

// Start listening for mutations on Chrome browsers and
// delete the prepended character introduced by compositionstart
// for other browsers.
if (Browser.isChrome()) {
editor.insertText(event.data);
editor.setPlaceholder(editor.placeholder);
editor._mutationHandler.startObserving();
} else {
let startOfCompositionLine = editor.range.headSection.toPosition(0);
let endOfCompositionLine = editor.range.headSection.toPosition(event.data.length);
editor.run(postEditor => {
postEditor.deleteAtPosition(startOfCompositionLine, 1, { unit: 'char' });
postEditor.setRange(endOfCompositionLine);
});
}
}

cut(event) {
event.preventDefault();

Expand Down
3 changes: 3 additions & 0 deletions src/js/utils/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,8 @@ export default {
},
isWin() {
return (typeof window !== 'undefined') && window.navigator && /Win/.test(window.navigator.platform);
},
isChrome() {
return (typeof window !== 'undefined') && ('chrome' in window);
}
};
10 changes: 2 additions & 8 deletions testem-ci.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,17 @@ module.exports = {
"args": ["-b", "microsoftedge", "-v", "15", "--no-connect", "-u"],
"protocol": "tap"
},
"SL_IE_11": {
"exe": "saucie",
"args": ["-b", "internet explorer", "-v", "11", "--no-connect", "-u"],
"protocol": "tap"
},
"SL_Safari_Current": {
"exe": "saucie",
"args": ["-b", "safari", "-v", "9", "--no-connect", "-u"],
"args": ["-b", "safari", "-v", "latest", "--no-connect", "-u"],
"protocol": "tap"
}
},
"launch_in_ci": [
"Chrome",
"Firefox",
"SL_Safari_Current",
"SL_MS_Edge",
"SL_IE_11"
"SL_MS_Edge"
],
"browser_args": {
"Chrome": [
Expand Down
93 changes: 93 additions & 0 deletions tests/acceptance/editor-ime-handler-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import Keycodes from 'mobiledoc-kit/utils/keycodes';
import Browser from 'mobiledoc-kit/utils/browser';
import Helpers from '../test-helpers';

let editor, editorElement;

const { test, module } = Helpers;

module('Acceptance: editor: IME Composition Event Handler', {
beforeEach() {
editorElement = $('#editor')[0];
},
afterEach() {
if (editor) { editor.destroy(); }
}
});

['Enter', 'Tab', 'Backspace'].forEach((key) => {
test(`ignore ${key} keydowns when using an IME`, (assert) => {
let { post: expected } = Helpers.postAbstract.buildFromText('你好');
editor = Helpers.editor.buildFromText('你好', { element: editorElement });

Helpers.dom.moveCursorTo(editor, editorElement.firstChild, 1);

Helpers.dom.triggerKeyEvent(editor, 'keydown', {
key,
keyCode: Keycodes.IME,
charCode: Keycodes[key.toUpperCase()]
});

assert.postIsSimilar(editor.post, expected);
});
});

test('ignore horizontal arrow keydowns when using an IME', (assert) => {
editor = Helpers.editor.buildFromText("안녕하세요", { element: editorElement });

Helpers.dom.moveCursorTo(editor, editorElement.firstChild);

Helpers.dom.triggerKeyEvent(editor, 'keydown', {
key: 'ArrowRight',
keyCode: Keycodes.IME,
charCode: Keycodes.RIGHT
});

assert.positionIsEqual(editor.range.head, editor.post.headPosition());

Helpers.dom.moveCursorTo(editor, editorElement.firstChild, 1);

Helpers.dom.triggerKeyEvent(editor, 'keydown', {
key: 'ArrowLeft',
keyCode: Keycodes.IME,
charCode: Keycodes.LEFT
});

assert.positionIsEqual(editor.range.head, editor.post.tailPosition());
});

// There doesn't seem to be a way to directly test the usage
// of an OS-level IME, however this test roughly simulates
// how the IME inputs text into the DOM.
test('test handling of IME composition events', (assert) => {
let done = assert.async();

editor = Helpers.editor.buildFromText("", { element: editorElement });

Helpers.dom.moveCursorTo(editor, editorElement);

editor.element.dispatchEvent(
new CompositionEvent('compositionstart', { 'data': 'n' })
);

Helpers.wait(() => {
if(Browser.isChrome()) {
editorElement.firstChild.innerHTML = "こんにちは";
} else {
editorElement.firstChild.innerHTML += "こんにちは";
}

Helpers.wait(() => {
editor.element.dispatchEvent(
new CompositionEvent('compositionend', { 'data': 'こんにちは' })
);

Helpers.wait(() => {
assert.positionIsEqual(editor.range.head, editor.post.tailPosition());
assert.hasElement('#editor p:contains(こんにちは)');

done();
});
});
});
});