Skip to content

Commit

Permalink
Fix IME bugs
Browse files Browse the repository at this point in the history
* Suppress mutations when using an IME on a blank line
* Ignore control keydowns when IME is active
* Handle carriage return keypresses
  • Loading branch information
ruiokada committed Jul 5, 2020
1 parent 3d96e5a commit e40114d
Show file tree
Hide file tree
Showing 4 changed files with 173 additions and 1 deletion.
2 changes: 2 additions & 0 deletions src/js/editor/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,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
76 changes: 75 additions & 1 deletion src/js/editor/event-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,17 @@ import TextInputHandler from 'mobiledoc-kit/editor/text-input-handler'
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']
const ELEMENT_EVENT_TYPES = [
'keydown',
'keyup',
'cut',
'copy',
'paste',
'keypress',
'drop',
'compositionstart',
'compositionend',
]

export default class EventManager {
constructor(editor) {
Expand Down Expand Up @@ -143,6 +153,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 @@ -169,6 +186,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 @@ -215,6 +236,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) {
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.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,7 @@ export default {
isWin() {
return typeof window !== 'undefined' && window.navigator && /Win/.test(window.navigator.platform)
},
isChrome() {
return typeof window !== 'undefined' && 'chrome' in window
},
}
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 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();
});
});
});
});

0 comments on commit e40114d

Please sign in to comment.