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)

x86
macOS
defect
Not set
normal

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: nobody → dflanagan
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.
Blocks: 876598
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)
See comment on GH
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 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-
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)
Attachment #764604 - Flags: review- → review?(janjongboom)
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+
Tests are bug 885678. Seems this issue already exists on master today, but noone noticed it.
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.