914459 - basicLayoutKey is not displayed correctly when it is defined by keyboard

Status: RESOLVED FIXED

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

Description

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: Arabic keyboard defines a basicLayoutKey, but alternateLayout displays it as "ABC".

The design of keyboard gives the author the ability to change the basic layout key contents, but it is not displayed correctly when it is defined by keyboard.

Attached file:

Arabic keyboard defines basicLayoutKey,

Expect result: In alternateLayout the basicLayoutKey should displays the content defined by Arabic keyboard 

Actual result: In alternateLayout the basicLayoutKey displays "ABC".

Comment 1

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: bug-914459-pull-request.html

When keyboard defined basicLayoutKey, it should be used in alternateLayout and symbolLayout.

Comment 2

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: bug-914459-pull-request.html

Re-attach the file in correct format.

Comment 3

[Luke Chang [:lchang] (inactive)]

Comment on attachment 802025 [details]
bug-914459-pull-request.html

It looks good to me. :)
However, I think it should be reviewed by the keyboard owner.

@Rudy, could you please review this patch, thanks.

Comment 4

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: bug-914459-pull-request-2.html

Correct the format and send pull request again.

Comment 5

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: bug-914459-pull-request-3.html

When a keyboard defines basicLayoutKey but no alternateLayout, it will use the base alternateLayout + basicLayoutKey. Need to clean the basicLayoutKey after it is assigned, otherwise the basicLayoutKey will be used by other keyboard which do not define an alternateLayout.

Comment 6

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

Comment on attachment 802817 [details]
bug-914459-pull-request-3.html

Hi Seinlin, 

I leave some comment on the pull reqeust, we might need to put the logic to replace basicLayoutKey in one place if possible.

Please help take a look and set the review flag back when you have an update.
Thanks.

Comment 7

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

Change the assignee, since Seinlin has a WIP patch.

Hi Seinlin,

BTW, you may update the code in the same pull request if you just pushed to the same branch.
You don't have to create another pull request. :)

Comment 8

[Kai-Zhen Li [:seinlin][:kli]]

(In reply to Rudy Lu [:rudyl] from comment #7)
> Change the assignee, since Seinlin has a WIP patch.
> 
> Hi Seinlin,
> 
> BTW, you may update the code in the same pull request if you just pushed to
> the same branch.
> You don't have to create another pull request. :)

Hi Rudy,

Thanks! I update change to the same pull request. Please have a look.

Comment 9

[Kai-Zhen Li [:seinlin][:kli]]

Comment on attachment 801986 [details]
Arabic keyboard defines a basicLayoutKey, but alternateLayout displays it as "ABC".

test

Comment 10

[Sein Lin]

feedback

Comment 11

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

Comment on attachment 802817 [details]
bug-914459-pull-request-3.html

Hi Jan,

Not sure if you are interested in reviewing this patch first?

Comment 12

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 802817 [details]
bug-914459-pull-request-3.html

r=me. The code works, but looks very strange if you see it in the codebase. Please add a comment that it's not possible to use |layout| variable here because it holds the alternative layout that doesn't have information about the alt key to be used; and that we need this info from the main layout object.

Please needinfo me if you added the comment and I'll land the patch :-) Thanks for your help!

Comment 13

[Kai-Zhen Li [:seinlin][:kli]]

Thanks, I added a comment and pull request is updated!

Comment 14

[Jan Jongboom [:janjongboom] (inactive)]

Hi, thanks a lot for a working on this. The patch has been landed in https://github.com/mozilla-b2g/gaia/commit/042b3f3bf882f2b3cb91207d54927b2490d65b9e.