Closed Bug 901377 Opened 9 years ago Closed 9 years ago
Autocorrect doesn't update internal state on cursor movement, causing incorrect replacements to be made
The problems are in all text boxes. For normal textbox, type: Jan dkk bla Jump to the last 'k', type a 'g' Jump to last character Press space, 'bla' is replaced by 'dike' which makes no sense. Appears both in contenteditable and normal textboxes. Possible cause might be bug 890207.
Nominating for leo? because this is a regression that is a serious UX flow, causing text to be wrongly replaced. Possible fix: backporting onselectionchange to b2g18
I can reproduce this on 1.1. (Haven't tried master yet) This is a very serious regression that basically breaks autocorrection. Asking QA for help determining the regression range. Who can give this Leo+?
Yuan, Any idea what might be causing this? Do you want to take this bug? If not, please assign me.
Tested with latest master branch and the bug is caused by gaia commit 8ff99a4b7cc0ad47c310401467a96e94b2f63d99 of bug 874011. When cursor moves, the latin IME will get the new cursor position by the callback function of `activate(language, suggestionsEnabled, inputstate)` in latin.js. Then the callback function calls `updateSuggestions()` to update suggestion word list. With bug 874011 fixed, the callback function fails to call `updateSuggestions()` if the keyboard language hasn't been change since last call.
Assignee: nobody → xyuan
Triage - leo+'ing regression bug.
blocking-b2g: leo? → leo+
Pointer to Github pull-request
Yuan, Thank you for investigating this and patching it so quickly! I'm embarassed to find that it is my regression! I assumed that something had changed in forms.js that was preventing activations from being sent to the keyboard. I didn't stop to investigate whether my code in latin.js was ignoring those activations! I apologize for being quick to assume that the bug was in your code! I was just about to land your patch when I notice that activate() should probably also reset the correctionDiabled flag. If it is set and the user moves the cursor, we won't get autocorrection and the new location. While that is not technically part of this regression, I'd like to fix it here. (I'm also seeing something more ominous where after tapping around in my input a few times, the suggestion bar goes away entirely, and I'd like to investigate this some more, too.) So I'm going to take this bug and ask you to review my patch when I have it ready.
Assignee: xyuan → dflanagan
Yuan, I'm replacing your pull request with mine. Mine includes the fix you found, but also fixes another regression: When the user moved the cursor we did not reset the 'correctionDisabled' flag, so if the user made an autocorrection, reverted it with a backspace, and then moved the cursor, we'd remain in the disabled state until they typed a space. Also, your patch introduced another bug: when the language did not need to change, your patch calls updateSuggesetions() instead of setLanguage(). That fixes the bug reported here, but introduces a new problem because the idle timer never gets reset, so if the user dismisses the keyboard and then brings it up again, autocorrection stops working after about 30 seconds. This patch fixes that problem by moving the clearTimeout() code into activate() (where it should have been all along) instead of setLanguage(). Thank you again for your quick diagnosis of this bug!
Comment on attachment 785973 [details] link.html Setting feedback? for Jan and Mihai because Jan reported the bug and Mihai and I both worked on the bug that caused this regression. Jan and Mihai: no action is needed from you. I just wanted to let you know about this patch in case you are interested.
Comment on attachment 785973 [details] link.html The patch is great and r=me. It's a pleasant to work with you.
Attachment #785973 - Flags: review?(xyuan) → review+
Comment on attachment 785973 [details] link.html Thanks for taking care of these regressions, David -- curious that we missed them while working on those keyboard features.
Attachment #785973 - Flags: feedback?(mihai) → feedback+
Uplifted 42c4efb7550820b7b6d6086d419a32a9e0cad174 to: v1-train: dc96979e15d0eb562002d00c46167fe48655a0e1
Verified fixed on Build ID: 20130814041202 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/15813d776a69 Gaia: dd3959fa74e356a528daa76ffee14c2c728a4b56 Platform Version: 18.1 RIL Version: 01.01.00.019.190 Following steps on bug issue response correctly
You need to log in before you can comment on or make changes to this bug.