1085359 - Merge the rendering of upper case layout and lower case layout

Status: RESOLVED FIXED

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

Description

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

The layout rendering of different cases is not the same, and it is distinguished by |flag.uppercase| we passed to IMERender.draw() function. In fact, this is only matters for layout definitions where |secondLayout| is true.

In this bug, I propose we merge the different rendering (a <div class="keyboard-type-container">) of different cases, that should allow us to remove special |secondLayout| handling in RenderingManager, and prevent |showUpperCase| from being needed in VisualHighlightManager.

This has a dependency to bug 985853 because of bug 985853 comment 10. If we keep using different set of DOM elements to render upper/lower case layouts, bug 985853 will not work in layout definitions where |secondLayout| is true.

Comment 1

[Mozilla Shepherd (This account is used for automation and doesn't receive bugmail)]

Attachment: mozilla-b2g:master PR#25374

Comment 2

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

Comment on attachment 8508773 [details] [review]
mozilla-b2g:master PR#25374

Rudy, John, could you review this? I think having one of you to review the patch would be enough.

With this patch I removed all |secondLayout| handling in business logic, and pass the responsibility to view, since this property affects representation only. Get rid of that simplifies business logic and avoid yet another special handling in bug 985853.

It is undesirable to put more code into render.js, but I want to block bug 985853 as soon as possible so I choose not to spend time factoring out KeyView etc. here. I also didn't pay attention to arguments passed to buildKey() because I want to keep the change at the minim.

Thanks.

Comment 3

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

Comment on attachment 8508773 [details] [review]
mozilla-b2g:master PR#25374

The patch itself looks good but unfortunately I'm experiencing a bug on Japanese IME with it.

STR:
1. Build with Japanese IME support (jp-kanji) and enable it.
2. Open Messages app. Start a new message. (You'll be on the recipient field)
3. Switch to jp-kanji IME
4. Press や (It is the third row of the third column)
5. Press ま (It is to the right of や, i.e. the second row of the second column)

At Step 5, adb logcat says
W/Built-in Keyboard( 1133): [JavaScript Error: "TypeError: cand is undefined" {file: "app://keyboard.gaiamobile.org/js/render.js" line: 544}]
And that IME basically becomes inoperable (further presses on the keys won't have any input effect, only highlights)

Comment 4

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

Comment on attachment 8508773 [details] [review]
mozilla-b2g:master PR#25374

After some experimentation, it appears that my claim on comment 3 is incorrect. It's an issue that arises from Japanese IME's implementation for candidate suggestion and is observable in master if reproduced with correct timing.

Comment 5

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

master: https://github.com/mozilla-b2g/gaia/commit/76d80e5b137dbe96dd41b132fc6a57deae8ea157