1079091 - The page switching key would be kept highlighted after switching back to default page

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Keyboard, defect)

Description

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

STR
===
1. Launch keyboard
2. Press [12&] to switch to symbol page.
3. Press [ABC] to switch back.

=>  the key [12&] would be wrongly highlighted.

Comment 1

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

I am afraid this is related to bug 1075970, reverting that patch could make this issue gone.

John, do you have bandwidth to take a look?
Thanks.

Comment 2

[John Lu [:mnjul][Please NI or I'll miss]]

Well, I took a look into the issue and this seemed yet another design weakness of bug 1044525 and bug 1075970. When |unHighlightKey| gets the element from the reverse map [1], the currentDrawnKeyboardClass is already assigned to the class of the next page. And the key in the old page wouldn't be retrieved and unhighlighted, resulting to the bug.

[1] https://github.com/mnjul/gaia/blob/88dd7fd0d13af5a0bee44339afcda43a38aaca41/apps/keyboard/js/render.js#L328

I can take a bug but I need to think a good way to resolve this :/

Comment 3

[John Lu [:mnjul][Please NI or I'll miss]]

Attachment: Proposed Patch (PR @ GitHub)

Discussed with Rudy for a few possible ways to fix the regression, but unfortunately all of them were quite hackish.

I decided to take the "if the key is a page switch key, unhighlight all DOMelements, of different keyboard classes, mapped by the key" route.

Another of the more viable ways is we inject a "currentPage" to switch key objects (during normalization, or in the hardcoded objects of updateCurrentPage) such that when we're to unhighlight a key, we can deduce that correct keyboardClass (to dig the reverse map from), which is different from IMERender.currentDisplayedKeyboardClass for the specific scenario of this bug.

Opinions? Thanks.

Comment 4

[John Lu [:mnjul][Please NI or I'll miss]]

Comment on attachment 8500912 [details] [review]
Proposed Patch (PR @ GitHub)

>https://github.com/mozilla-b2g/gaia/pull/24876

Gaia try did not start, I'm getting a new PR on https://github.com/mozilla-b2g/gaia/pull/24913 .

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8500912 [details] [review]
Proposed Patch (PR @ GitHub)

Discussed offline; let's ensure there will always be two way one-to-one mapping instead.

Comment 6

[John Lu [:mnjul][Please NI or I'll miss]]

Fixed by backing out bug 1075970: https://github.com/mozilla-b2g/gaia/commit/636fdf9ae94a491fa90aeff096d24bfe2a94da8e

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

regression

Comment 9

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Blocking 2.2+ for all fixed regressions.

Comment 10

[John Thomas [:Johnt]]

This issue is verified fixed on the Flame 2.2 Shallow Flash (319mb)

Result: Keyboard switch key is is not highlighted when switching back and forth.

Flame 2.2
Device: Flame 2.2 (319mb)(Kitkat Base)(Shallow Flash)
Build ID: 20141121040204
Gaia: 25388c6bce932657ebf93adedf31881bfaf88c15
Gecko: 3366c0fcf9c2
Version: 36.0a1 (2.2)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0