Closed
Bug 1085359
Opened 10 years ago
Closed 10 years ago
Merge the rendering of upper case layout and lower case layout
Categories
(Firefox OS Graveyard :: Gaia::Keyboard, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: timdream, Assigned: timdream)
References
Details
Attachments
(1 file)
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.
Assignee | ||
Comment 2•10 years ago
|
||
Comment on attachment 8508773 [details]
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.
Attachment #8508773 -
Flags: review?(rlu)
Attachment #8508773 -
Flags: review?(jlu)
Comment 3•10 years ago
|
||
Comment on attachment 8508773 [details]
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)
Attachment #8508773 -
Flags: review?(jlu) → feedback+
Comment 4•10 years ago
|
||
Comment on attachment 8508773 [details]
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.
Attachment #8508773 -
Flags: feedback+ → review+
Assignee | ||
Comment 5•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•10 years ago
|
Attachment #8508773 -
Flags: review?(rlu)
You need to log in
before you can comment on or make changes to this bug.
Description
•