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

VERIFIED FIXED in 2.1 S6 (10oct)

Status

defect
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: rudyl, Assigned: mnjul)

Tracking

({regression})

unspecified
2.1 S6 (10oct)
x86
macOS
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.2 verified)

Details

(Whiteboard: [p=1])

Attachments

(1 attachment)

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.
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.
Flags: needinfo?(jlu)
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 :/
Flags: needinfo?(jlu)
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.
Attachment #8500912 - Flags: review?(timdream)
Attachment #8500912 - Flags: feedback?(rlu)
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 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.
Attachment #8500912 - Flags: review?(timdream)
Attachment #8500912 - Flags: feedback?(rlu)
Fixed by backing out bug 1075970: https://github.com/mozilla-b2g/gaia/commit/636fdf9ae94a491fa90aeff096d24bfe2a94da8e
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Assignee: nobody → jlu
Target Milestone: --- → 2.1 S6 (10oct)
Whiteboard: [p=1]
Duplicate of this bug: 1079424
Blocking 2.2+ for all fixed regressions.
blocking-b2g: 2.2? → 2.2+
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.