Description: Changing from one text field to another will often prevent the character selection pop-ups from displaying as the user types. Note: Long pressing a character like 'a' will bring up the list of Special Character options improperly above the keyboard on the left side (see attached screenshot). This issue is most noticable when switching from the Username to the Password fields of a log-in page, or when switching from a text field to the Rocketbar. This issue has been seen in the Facebook app, Rocketbar, Contacts, and inputting a WiFi password. Repro Steps: 1) Update a Flame device to BuildID: 20141001040205 2) Download and launch the Facebook app from the Marketplace 3) Input text into the Username/Email field of the log-in page. 4) Change to the Password field and input text. 5) Change back to the Username field and input more text 6) Repeat steps 4 and 5 several times. Actual: Keyboard stops displaying character selection indicator pop-ups Expected: Keyboard continues to display character selection indicator pop-ups properly Environmental Variables: Device: Flame 2.2 Master BuildID: 20141001040205 Gaia: 0e280591881d44b80f456bc27e12d9114c218868 Gecko: 14665b1de5ee Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf Version: 35.0a1 (2.2 Master) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0 Repro frequency: 2/5 See attached: screenshot, video clip, logcat ------------------------------------------------------- This issue does NOT occur in Flame 2.1. Keyboard character selection indicators are consistently displayed properly. Environmental Variables: Device: Flame 2.1 319MB BuildID: 20141001000202 (Full Flash) Gaia: 86a3626055ed58b39525424126870dc0a503e79f Gecko: c20912812877 Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf Version: 34.0a2 (2.1) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

[Blocking Requested - why for this release]: Regression of core functionality of keyboard app. Requesting a window.

Cause: Bug 1044525 seems to be the cause for this issue. B2g-inbound Regression Window Last Working Environmental Variables: Device: Flame 2.2 BuildID: 20140929035801 Gaia: 44c06869d9cf80d74157ea94b161b572f918a793 Gecko: f8237e7ef676 Version: 35.0a1 (2.2) Firmware Version: L1TC10011800 User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0 First Broken Environmental Variables: Device: Flame 2.2 BuildID: 20140929042800 Gaia: afeb7316826fb2172070e27632270224bc2ced13 Gecko: 59e54655f006 Version: 35.0a1 (2.2) Firmware Version: L1TC10011800 User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0 Last Working gaia / First Broken gecko - Issue does NOT occur Gaia: 44c06869d9cf80d74157ea94b161b572f918a793 Gecko: 59e54655f006 First Broken gaia / Last Working gekko - Issue DOES occur Gaia: afeb7316826fb2172070e27632270224bc2ced13 Gecko: f8237e7ef676 Gaia Pushlog: https://github.com/mozilla-b2g/gaia/compare/44c06869d9cf80d74157ea94b161b572f918a793...afeb7316826fb2172070e27632270224bc2ced13

broken by Bug 1044525 - can you take a look?

Sure. It does look like something that has to do with my patch. Keeping NI for reminding.

The root cause of this bug is that we can have multiple rendered keyboard layouts for a single layout, and that multiplicity comes from different input types, whether you allow candidate panels, and so forth [1]. When we switch back to already-drawn layouts, the renderer reuses rendered layouts, and bypasses my TargetObj/DomElem map-setting routines; so when you switch to (for example) another input type and back to the original input type, the reverse map (TargetObj to DOMElement) is still mapping to the "another" input type's rendered layout. As such, the reverse map (TargetObj to DOMElement) shall be able to map from one target object to multiple DOMElements (of different containers that can be indexed through keyboardClass as in [1]). In this patch, the value of the reverse WeakMap is now an object, holding multiple DOMElements for that target object key'ed by the keyboardClass decided in [1]. Then we need to keep track of the currently drawn keyboardClass such that the mapping can work correctly. [2] An alternative way to resolve this bug is to regenerate the whole reverse map when we reuse rendered keyboard layouts and bypass the actual rendering logics. Tim, could you see if this amends the architectural design reasonably? [1] https://github.com/mnjul/gaia/blob/b540d1c7b492555ba7ab8bd6a562413380107172/apps/keyboard/js/render.js#L131-L153 [2] On a second thought, I think we should use "currentlyDisplayedKeyboardClass" for that variable's name. That will be changed if not objected.

Comment on attachment 8499438 [details] Patch (PR @ Github) This is a sad band-aid, but I am sure we could dismantle render.js to make this part of keyboard app saner in the future.

TBPL testing before final landing: https://tbpl.mozilla.org/?rev=9845f86f0fc05f6c5ef839d94c4e6a7079e99253&tree=Gaia-Try

Master: https://github.com/mozilla-b2g/gaia/commit/1d153c6a9fbbdf50191673d92b3d2aa24d17cb31

Backed out for causing bug 1079091: https://github.com/mozilla-b2g/gaia/commit/636fdf9ae94a491fa90aeff096d24bfe2a94da8e I'll post a revised patch later.

So here's a better patch to address this bug and it should probably risk the overall architecture less (if any) against further regressions. As observed with this bug and bug 1079091, we have to ensure that a DOM element mapped to a target object can be mapped back to that very DOM element correctly. To achieve that, with this patch, when we insert an entry into the maps, we now generate a "reference stub" to the target object by |Object.create(targetObj)| and that "reference stub" is used for map look-up everywhere. So, each rendered DOM layout key has a reference stub unique from that key of another rendered DOM layout, and the mappings should work correctly. The created reference stubs are also |Object.freeze()|'en after being created to guard against unnecessary manipulation. Tim & Rudy, could you check if this looks good (and tests good if you'd like to test)?

Comment on attachment 8502247 [details] Patch v2 (PR @ GitHub) Looks simple and good if you could confirm this indeed fix the issues mentioned.

Master: https://github.com/mozilla-b2g/gaia/commit/8c3608c83ddc64cdfa5159ce216ad77f72cc83c9

Comment on attachment 8502247 [details] Patch v2 (PR @ GitHub) (Offline discussed with Rudy and he did not have concerns with this patch)

Adding qawanted to verify that this issue has been fixed.

Verified fixed on Flame 2.2 (319mb/full flash) Actual result: Keyboard continues to display character selection indicator pop-ups properly Device: Flame 2.2 BuildID: 20141012040203 Gaia: 717ad4e8b7fc10ab8248500d00ba5ba0977fa8ab Gecko: 44168a7af20d Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf Version: 35.0a1 (2.2) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Blocking 2.2+ for all fixed regressions.