Closed
Bug 882954
Opened 11 years ago
Closed 11 years ago
[keyboard] autocorrect failures caused by bad nearby key data
Categories
(Firefox OS Graveyard :: Gaia::Keyboard, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: djf, Assigned: djf)
References
Details
Attachments
(1 file)
On gaia master (haven't checked v1-train yet) if I type 'lit' it autocorrects to kit, and if I type 'snarl' it auto-corrects to 'snark'. When investigating this, I've determined that the nearby key data thinks that the k key is closer to the l key than the l key is to itself! Basically for nearby key pairs, we have a weight saying how close they are. This weight is always supposed to be < 1. But for this k,l pair, the weight is 1.03. So when the prediction engine converts an l to a k it actually boosts the weight of the corrected word. If this is only on master, then I suspect that we're getting bad key data out of the new keyboard layout from render.js. (Maybe something related to the removal of the apostrophe key on master) If this bug also exists on v1-train, then it seems more likely that there is an error in the code I use to translate the data from render.js into the data used by predictions.js.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → dflanagan
Assignee | ||
Comment 1•11 years ago
|
||
As suspected, render.js is sending the wrong x coordinate for the L key on the keyboard. This appears to be a regression caused by bug 876598. That bug allows us to tap to the left of the A key or the right of the L key and still type an A or an L. But in the process it somehow also changes the reported X coordinate of the L key, so that render.js is telling keyboard.js that the K and L keys overlap. This messes up the nearby key calculations in the latin IM. Another regression that I'd guess is from the same bug: when the keyboard is in landscape mode, the A and L keys are narrower than the other ones.
Assignee | ||
Comment 2•11 years ago
|
||
Jan, I'm asking for your review on this bug because your patch in bug 876598 caused this regression and it is your code I'm modifying. It seems that the corrections made to the layout of the A and L keys affect the position of all of the keys in that middle row, so we can't compute the proper layout data of the keys in a row until the entire row has been processed. Without this patch, the data recorded in _keyArray[] says that the K and L keys overlap, and this causes auto-correction errors.
Attachment #764604 -
Flags: review?(janjongboom)
Comment 3•11 years ago
|
||
See comment on GH
Assignee | ||
Comment 4•11 years ago
|
||
Jan, If you just leave a comment on github and don't change any flags here on bugzilla, there is a good chance that I won't notice anything and the bug will just sit here. Better to give r- or use needinfo? to get my attention. I've responded on github with my hypothesis about why my patch fixes the bug. Using needinfo to be sure that you see this response.
Flags: needinfo?(janjongboom)
Comment 5•11 years ago
|
||
Comment on attachment 764604 [details]
link to patch on github
I still think the shifting of stuff is weird. But anyway, I have a question on GH. Lit now autocorrects to Lot which also doesn't sound right. Other than that I'm happy.
Attachment #764604 -
Flags: review?(janjongboom) → review-
Assignee | ||
Comment 6•11 years ago
|
||
I've responded on github about the lit->lot correction. That is just because lot is a much more common word than lit. Clearing the needinfo and resetting the r? flag for another review.
Flags: needinfo?(janjongboom)
Assignee | ||
Updated•11 years ago
|
Attachment #764604 -
Flags: review- → review?(janjongboom)
Comment 7•11 years ago
|
||
Comment on attachment 764604 [details]
link to patch on github
Good. Tested on GP Keon. I think code like this would very much benefit from tests to avoid regressions like this. I'll write some today.
Attachment #764604 -
Flags: review?(janjongboom) → review+
Comment 8•11 years ago
|
||
Tests are bug 885678. Seems this issue already exists on master today, but noone noticed it.
Assignee | ||
Comment 9•11 years ago
|
||
Fixed on gaia master: https://github.com/mozilla-b2g/gaia/commit/8d0102f400b9a0e7fbc2e129305c09aa6eef5cff
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•