Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Commit

Permalink
Merge pull request #20750 from RudyLu/keyboard/Bug1015541
Browse files Browse the repository at this point in the history
Bug 1015541 - choosing a word suggestion after moving the caret changes the wrong text
r=janjongboom
  • Loading branch information
RudyLu committed Jun 25, 2014
2 parents 0f2ac38 + a7e7aa4 commit 73b9c78
Show file tree
Hide file tree
Showing 3 changed files with 185 additions and 2 deletions.
45 changes: 44 additions & 1 deletion apps/keyboard/js/imes/latin/latin.js
Expand Up @@ -52,7 +52,8 @@
select: select,
dismissSuggestions: dismissSuggestions,
setLayoutParams: setLayoutParams,
setLanguage: setLanguage
setLanguage: setLanguage,
handleEvent: handleEvent
};

// This is the object that is passed to init().
Expand Down Expand Up @@ -129,6 +130,12 @@
*/
var inputSequencePromise = Promise.resolve();


// Flag to stop updating suggestions for selectionchange when we're going
// to do some actions that will cause selectionchange, such as sendKey()
// or replaceSurroundingText().
var pendingSelectionChange = 0;

// keyboard.js calls this to pass us the interface object we need
// to communicate with it
function init(interfaceObject) {
Expand Down Expand Up @@ -191,6 +198,10 @@
suggesting = (options.suggest && inputMode !== 'verbatim');
correcting = (options.correct && inputMode !== 'verbatim');

if (state.inputContext) {
state.inputContext.addEventListener('selectionchange', this);

This comment has been minimized.

Copy link
@timdream

timdream Jul 16, 2014

Contributor

I am very interested to know why a patch with addEventListener without corresponding removeEventListener can get a review+. @janjongboom ?

}

// Reset our state
lastSpaceTimestamp = 0;
autoCorrection = null;
Expand Down Expand Up @@ -346,6 +357,9 @@
function click(keyCode, upperKeyCode, repeat) {
// Wait for the previous keys have been resolved and then handle the next
// key.

pendingSelectionChange++;

var nextKeyPromise = inputSequencePromise.then(function() {
keyCode = keyboard.isCapitalized() && upperKeyCode ? upperKeyCode :
keyCode;
Expand Down Expand Up @@ -407,9 +421,12 @@
}

lastSpaceTimestamp = (keyCode === SPACE) ? Date.now() : 0;
pendingSelectionChange--;

}, function() {
// the previous sendKey or replaceSurroundingText has been rejected,
// No need to update the state.
pendingSelectionChange--;
});

// Need to return the promise, so that the caller could know
Expand Down Expand Up @@ -983,6 +1000,32 @@
return c === '.' || c === '?' || c === '!';
}

function handleEvent(evt) {
var type = evt.type;
switch (type) {
case 'selectionchange':
// We would get selectionchange event when the user type each char,
// or accept a word suggestion,so don't update suggestions in these
// cases.
if (cursor === evt.target.selectionStart ||
pendingSelectionChange > 0) {
return;
}

var newText = evt.target.textBeforeCursor + evt.target.textAfterCursor;
inputText = newText;
cursor = evt.target.selectionStart;
if (evt.target.selectionEnd > evt.target.selectionStart) {
selection = evt.target.selectionEnd;
} else {
selection = 0;
}

updateSuggestions();
break;
}
}

if (!('LAYOUT_PAGE_DEFAULT' in window))
window.LAYOUT_PAGE_DEFAULT = null;
}());
3 changes: 2 additions & 1 deletion apps/keyboard/js/keyboard.js
Expand Up @@ -1629,7 +1629,8 @@ function switchIMEngine(layoutName, mustRender) {
inputmode: inputContext.inputMode,
selectionStart: inputContext.selectionStart,
selectionEnd: inputContext.selectionEnd,
value: values[0]
value: values[0],
inputContext: inputContext
},
{
suggest: values[1].suggestionsEnabled && !isGreekSMS(),
Expand Down
139 changes: 139 additions & 0 deletions apps/keyboard/test/unit/latin_test.js
@@ -1,5 +1,7 @@
'use strict';

/* global Event, InputMethods */

requireApp('keyboard/test/unit/setup_engine.js');
requireApp('keyboard/js/imes/latin/latin.js');

Expand Down Expand Up @@ -329,6 +331,143 @@ suite('latin input method capitalization and punctuation', function() {
});
});

suite('> selectionchange', function() {
// Create a element as an event target
var inputContext = document.createElement('div');

var keyboardGlue = Object.create(defaultKeyboardGlue);
var _windowWorker;
var workers = [];

function activateIME() {
im.activate('en', {
type: 'text',
inputmode: '',
value: '',
selectionStart: 0,
selectionEnd: 0,
inputContext: inputContext
},{suggest: true, correct: true});
}

setup(function() {
// reset the output state
reset();

inputContext.textBeforeCursor = 'before';
inputContext.textAfterCursor = '';
inputContext.selectionStart = 0;
inputContext.selectionEnd = 0;

_windowWorker = window.Worker;
var worker = window.Worker = function() {
//workers.push(this);
};

worker.prototype.postMessage = function() {};
});

teardown(function() {
window.Worker = _windowWorker;
});

test('should listen to selectionchange', function() {
im.init(keyboardGlue);
activateIME();

var handleEventSpy = sinon.spy(im, 'handleEvent');
inputContext.dispatchEvent(new Event('selectionchange'));

sinon.assert.calledOnce(handleEventSpy);
});

test('wll clear the suggestions if selectionchange', function() {
im = InputMethods.latin;
keyboardGlue.sendCandidates = sinon.stub();
im.init(keyboardGlue);

activateIME();

// change the cursor position
inputContext.selectionStart = 4;
inputContext.selectionEnd = 4;
inputContext.dispatchEvent(new Event('selectionchange'));

// will clear the suggestions since cursor changed
sinon.assert.calledTwice(keyboardGlue.sendCandidates);
});

test('Do nothing if selectionchange wth the same cursor', function() {
im = InputMethods.latin;
keyboardGlue.sendCandidates = sinon.stub();
im.init(keyboardGlue);

activateIME();
inputContext.dispatchEvent(new Event('selectionchange'));

// Do nothing with the same cursor
sinon.assert.calledOnce(keyboardGlue.sendCandidates);
});

test('Do nothing if there is pending selection change', function() {
im = InputMethods.latin;
keyboardGlue.sendCandidates = sinon.stub();
im.init(keyboardGlue);

activateIME();

im.click('t'.charCodeAt(0));

// change the cursor position
inputContext.selectionStart = 4;
inputContext.selectionEnd = 4;
inputContext.dispatchEvent(new Event('selectionchange'));

// Do nothing with the same cursor
sinon.assert.calledOnce(keyboardGlue.sendCandidates);
});

test('Continue to listen to selectionchange after pending', function(done) {
im = InputMethods.latin;
keyboardGlue.sendCandidates = sinon.stub();
im.init(keyboardGlue);

activateIME();

im.click('t'.charCodeAt(0)).then(function() {
inputContext.selectionStart = 4;
inputContext.selectionEnd = 4;
inputContext.dispatchEvent(new Event('selectionchange'));

sinon.assert.calledTwice(keyboardGlue.sendCandidates);
done();
});
});

test('Continue to skip selectionchange if there are still' +
' pending actions', function(done) {
im = InputMethods.latin;
keyboardGlue.sendCandidates = sinon.stub();
im.init(keyboardGlue);

activateIME();

im.click('t'.charCodeAt(0)).then(function() {
console.log('hi hi');
inputContext.selectionStart = 4;
inputContext.selectionEnd = 4;

// send the event after the first key is resolved
inputContext.dispatchEvent(new Event('selectionchange'));
});

im.click('o'.charCodeAt(0)).then(function() {
sinon.assert.calledOnce(keyboardGlue.sendCandidates);
done();
});
});
});

function runtest(input, type, mode, statename, options) {
var modeTitle = '-' + (mode ? mode : 'default');
var optionsTitle = options ? '-' + JSON.stringify(options) : '';
Expand Down

0 comments on commit 73b9c78

Please sign in to comment.