878012 - [keyboard] No change in keyboard Layout on changing keyboard langauge in keyboard setting.

Status: VERIFIED FIXED

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

Description

[Leo]

[TITLE]: No change in keyboard Langauge changing keyboard langauge in keyboard setting.
3.Action: Setting > keyboard > Default English > Select Arabic 
          Message > New Message > Change keypad layout to Arabic
          Go to Setting again
          Setting > keyboard > De select Arabic > Open meessage page> Select                    
          TEXT FIELD

4. Detailed Symptom (ENG.) : The keypad still shows in Arabic Layout.
                             This issue appears also for any other layout also.
5. GAIA version : d2ad0f1a5a6be8a2d6df721ba3c2b33e037c423b
6. Expected : The keypad should change automatically when keyboard langauge is changed in keyboard setting.
7.Reproducibility: Y
1)Frequency Rate : 100%
8.Comparison Results : 
1)Model Comparing : 
9. Attached files: 
1)Log : 
2)Test Contents : 
3)Video file :

Comment 1

[Leo]

Please check this issue.
This issue reproduces in Master Gaia and V1-Train both.

Comment 2

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

Yes, I can reproduce this issue now.
It is about keyboard layout settings, not about language.

Should be a regression.
Glad to have Mihai's help on this.

Please let me know if you guys have anything need me to dig into.

Comment 3

[Leo]

Hi Mihai

Following change in code is fixing the issue.

 function showKeyboard(state) {
 
--- if(keyboardName !== currentKeyboardName){
+++ if(keyboardName !== currentKeyboardName || enabledKeyboardNames.length === 1){


Please verify if its correct.

Thank you
Leo

Comment 4

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

(In reply to Leo from comment #3)
> Hi Mihai
> 
> Following change in code is fixing the issue.
> 
>  function showKeyboard(state) {
>  
> --- if(keyboardName !== currentKeyboardName){
> +++ if(keyboardName !== currentKeyboardName || enabledKeyboardNames.length
> === 1){
> 
> 
> Please verify if its correct.
> 
> Thank you
> Leo

Hi Leo, thanks for taking a look at this. Your solution will work only if you have only one keyboard layout active, for example if you have your language switched to German -> the otherlatins keyboard group is active by default (cannot be disabled) and it contains 6 keyboard layouts.

I'm almost done with my solution, currently testing it for any regression it might cause. Will post it in the upcoming hours.

Comment 5

[Jason Smith [:jsmith]]

Sounds like you got your answer in comment 2.

Comment 6

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Attachment: Pull Request #10134 - Reset kb if disabled from settings

Apparently when the keyboard settings were changed, keyboardName was not reset and, since setting currentKeyboardName depends on keyboardName, the check for (keyboardName !== currentKeyboardName) was false after following the STR -> the active keyboard layout was not updated when the keyboard was shown.

This patch fixes the issue by adding a check in handleNewKeyboards(), which is called every time keyboard settings change.

Comment 7

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

Comment on attachment 756569 [details]
Pull Request #10134 - Reset kb if disabled from settings

Hi Mihai,

Although your patch could fix this issue, but it seems the "keyboard.current" would not be updated for this case.

Could you please check what fix we need to make that happen?
Thank you.

--
I'll just clear the review flag here.

Comment 8

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Comment on attachment 756569 [details]
Pull Request #10134 - Reset kb if disabled from settings

Thanks for pointing that issue, Rudy, I initially thought that the 'keyboard.current' setting is set in renderKeyboard() (called from showKeyboard) -- as it was at some point in the code base.

I updated the patch to update the setting when resetting the keyboard name. Let me know if it looks good, thanks!

Comment 9

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

Comment on attachment 756569 [details]
Pull Request #10134 - Reset kb if disabled from settings

Hi Mihai,

Thanks for digging into this issue.
I have some suggestions to slightly move the "fallback" logic to the top of "showKeyboard()".
Please help take a look to see if it makes sense.

If not, please feel free to merge this with r=me.
Thanks.

Comment 10

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Thanks for the review, Rudy!

I have tested your suggestion and it doesn't seem to work without further changes (left the comment on GitHub). So, I left the solution as it was.

Landed on master:
https://github.com/mozilla-b2g/gaia/commit/6b82eb3ab24cd4b7dbc1c10cacada77f1875a9e2

Comment 11

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Ooops, reset some flags by accident with my previous comment...

Comment 12

[leo.bugzilla.gecko]

In triage

Comment 13

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 6b82eb3ab24cd4b7dbc1c10cacada77f1875a9e2 to:
v1-train: d9da86fe85e656e49d50df63b671f6c0e5838db9

Comment 14

[Deepa Subramanian]

The user is able to deselect Arabic in Settings-Keyboard. The user does not see the Arabic keyboard anymore.

This issue does not reproduce any more.

Environmental Variables
Build ID: 20130805071207
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/a2a9b89ef5ee
Gaia: 45f6a739b09292e16717fb21003386c914ca29c2
Platform Version: 18.1