983012 - [Sora][Message][Input method]The virtual keyboard is not working after switching the keyboard layouts several times.

Status: VERIFIED FIXED

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

Description

[sync-1]

Firefox OS v1.3
 Mozilla build ID: 20140226004002
 
 Regression: can not reproduce on buri 1.1.
 
 DEFECT DESCRIPTION:
 ->tap the virtual keyboard, it's disable.
 
  REPRODUCING PROCEDURES:
 ->Enter"Message"to create a message;
 ->Long tap global key to switch the input method as pinyin;
 ->Then tap the virtual keyboard, it's disable.(ko)
 
 It is easy to reproduce this behavior.
 Download the MS and power on, enter text edit interface, switch to "pinyin" input method. The keyboard is disable, it can not input any charactors.
 
 note:Buri 1.1 is ok.
 
  EXPECTED BEHAVIOUR:
 ->Should input normally
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 
  For FT PR, Please list reference mobile's behavior:

Comment 1

[sync-1]

Other input method also cause have PR, just continuous switch input method via long tap global key.

Comment 2

[Jason Smith [:jsmith]]

Can we confirm reproduction on Buri 1.3 not using Pinyin?

Comment 3

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

It can reproduce on 1.3 not using Pinyin:

http://www.youtube.com/watch?v=TmBsy3yYHI4&feature=youtu.be

As you can see, what I did was just kept changing the input method, and after changing about 6 or 7 times, the keypad cannot work anymore. Get no response when tap the virtual keypad

Thanks

Vance

Comment 4

[Matthew Vaughan]

This issue looks to have started on the 09/25/13 1.3 build.

- Last Working -
Device: Buri v1.3 MOZ RIL
BuildID: 20130924040201
Gaia: a22ba4a3a9efd99f94adf9ece8a2b391d4df295b
Gecko: 1fda74e33e06
Version: 27.0a1
Firmware Version: V1.2-device.cfg

- First Broken -
Device: Buri v1.3 MOZ RIL
BuildID: 20130925174530
Gaia: 7e42b4d690049709c62e8783910f16ab20869f42
Gecko: fa0e6916f88c
Version: 27.0a1
Firmware Version: V1.2-device.cfg

*This looks to be a gaia issue*
last working gaia/first broken gecko = NO REPRO
Gaia: a22ba4a3a9efd99f94adf9ece8a2b391d4df295b
Gecko: fa0e6916f88c

first broken gaia/last working gecko = REPRO
Gaia: 7e42b4d690049709c62e8783910f16ab20869f42
Gecko: 1fda74e33e06

Push log: https://github.com/mozilla-b2g/gaia/compare/a22ba4a3a9efd99f94adf9ece8a2b391d4df295b...7e42b4d690049709c62e8783910f16ab20869f42


NOTE: This possibly could be reproduced all the way back to the first 1.3 from 09/19/13. However it looks like in order to reproduce this issue, the cursor has to remain in, and maintain focus of, the text box that is being used to bring up the keyboard. This was broken until 9/25/13 when bug 911129 was fixed.

Comment 5

[xiupinglong]

After I revert this patch for bug 911129, this issue can still be reproduced.

Comment 6

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Hi Jason, per Comment#5, looks like we still need someone to further check this issue, mind pointing me to the owner of the general keyboard issues?

Thanks

Vance

Comment 7

[Jason Smith [:jsmith]]

Rudy would be a good person to ask.

Comment 8

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

I could not reproduce this issue with v1.3 on buri.
I did not enable pinyin layout for my test, could someone confirm if pinyin is necessary or not?

--
Gaia      2ea2aab306bd1c941719160cdcb49ee9d755dc17
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/cf2042938526
BuildID   20140317164002
Version   28.0
ro.build.version.incremental=324
ro.build.date=Thu Dec 19 14:04:55 CST 2013

Comment 9

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

(In reply to Rudy Lu [:rudyl] from comment #8)
> I could not reproduce this issue with v1.3 on buri.
> I did not enable pinyin layout for my test, could someone confirm if pinyin
> is necessary or not?
> 
> --
> Gaia      2ea2aab306bd1c941719160cdcb49ee9d755dc17
> Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/cf2042938526
> BuildID   20140317164002
> Version   28.0
> ro.build.version.incremental=324
> ro.build.date=Thu Dec 19 14:04:55 CST 2013

No Pinyin is not necessary to reproduce this issue, I get a video here, could you check the video first? I think that will give you a better idea regarding how to reproduce this one, if you still cannot reproduce it, please let me know and I can show you how to

http://www.youtube.com/watch?v=TmBsy3yYHI4&feature=youtu.be


Thanks

Vance

Comment 10

[Matthew Vaughan]

Removing steps-wanted per comment 9.

Comment 11

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

Vance, I could not access that video via this link,
http://www.youtube.com/watch?v=TmBsy3yYHI4&feature=youtu.be
 - It would say "The video is private."

Could you please help check the permission of that video?
Thanks.

Comment 12

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Sorry, my bad, please try again, it should be public available now

Comment 13

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

Still could not reproduce after following similar steps shown in that video.
My setup:
 1. Only enable 3 layouts - English, Spanish, Portuguese.
 2. Using message app's message field as well.

If someone on this bug could reproduce this, please help comment on if the whole system is still working, like if pressing Home button can bring you to homescreen, etc.

Vance,

I'll try to sync up with you on this bug tomorrow.

Comment 14

[xiupinglong]

(In reply to Rudy Lu [:rudyl] from comment #13)

> If someone on this bug could reproduce this, please help comment on if the
> whole system is still working, like if pressing Home button can bring you to
> homescreen, etc.
> 

Dear Rudy,

When this issue happened, the system is still working. But the keyboard can not input any charactors in all apps.

Comment 15

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

(In reply to Rudy Lu [:rudyl] from comment #13)
> Still could not reproduce after following similar steps shown in that video.
> My setup:
>  1. Only enable 3 layouts - English, Spanish, Portuguese.
>  2. Using message app's message field as well.
> 
> If someone on this bug could reproduce this, please help comment on if the
> whole system is still working, like if pressing Home button can bring you to
> homescreen, etc.
> 
> Vance,
> 
> I'll try to sync up with you on this bug tomorrow.

Sure, I am in office now, just ping me if you want to sync up

Comment 16

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

Still could not find an easy way to reproduce this on buri, so would like to call for help from QA.

William,

Do you recently test v1.3 keyboard?
Did you see any issues when switching keyboard layouts with long press?

Thanks.

Comment 17

[William Hsu [:whsu]]

Good Job, Vance! :)
=======================================================
Hi, Rudy,

I also can reproduce this bug on V1.3 build (Buri Device)
There had something wrong on Gecko while I reproduced it.

-------------------------------------------------------
E/GeckoConsole(  136): [JavaScript Error: "NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [inIDOMUtils.setContentState]" {file: "chrome://global/content/BrowserElementPanning.js" line: 484}]
E/GeckoConsole(  136): [JavaScript Error: "NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [inIDOMUtils.setContentState]" {file: "chrome://global/content/BrowserElementPanning.js" line: 484}]
E/GeckoConsole(  136): [JavaScript Error: "NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [inIDOMUtils.setContentState]" {file: "chrome://global/content/BrowserElementPanning.js" line: 484}]
E/GeckoConsole(  136): [JavaScript Error: "NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [inIDOMUtils.setContentState]" {file: "chrome://global/content/BrowserElementPanning.js" line: 484}]
E/GeckoConsole(  136): [JavaScript Error: "NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [inIDOMUtils.setContentState]" {file: "chrome://global/content/BrowserElementPanning.js" line: 484}]
E/GeckoConsole(  136): [JavaScript Error: "NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [inIDOMUtils.setContentState]" {file: "chrome://global/content/BrowserElementPanning.js" line: 484}]
E/GeckoConsole(  136): [JavaScript Error: "NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [inIDOMUtils.setContentState]" {file: "chrome://global/content/BrowserElementPanning.js" line: 484}]
E/GeckoConsole(  136): [JavaScript Error: "NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [inIDOMUtils.setContentState]" {file: "chrome://global/content/BrowserElementPanning.js" line: 484}]
--------------------------------------------------------

Does it relate to canvas because I saw a related discussion on stackoverflow.
- http://stackoverflow.com/questions/8660867/firefox-throws-error-0x80004005-when-working-with-canvas

* Build Information:
 - Gaia      da50b2b62ed96993cf4df22e3d69c435b28506bd
 - Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/3c39d1e487d9
 - BuildID   20140226004002
 - Version   28.0

* Result:
 - Can be reproduce

Thanks.

Comment 18

[Jason Smith [:jsmith]]

So far there's three people that can reproduce this, which makes me think the STR is correct. I don't know why Rudy can't reproduce. At this point, I think you need to go talk to William or Vance directly this to find out why you can't reproduce.

Comment 19

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

Attachment: dump_log

When this issue occurs, we got isShowingAlternativesMenu set as true.
https://github.com/mozilla-b2g/gaia/blob/808394920b64ee7f1bfc7bf84a1f2339e1eb1701/apps/keyboard/js/keyboard.js#L1081

So, keyboard app would ignore any new touch events.

From the log, isShowingAlternativesMenu is set as true before movePress() was invoked after you switch to any keyboard layout.
Will try to see how to avoid this.

Comment 20

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

Attachment: pull request 17374

The caused has been stated in the previous comment.
This patch tries to remove event listeners for the touched keys after you switch the layouts.

Tim, please help review this one.
Thanks.

Comment 21

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

Modify the summary since this has nothing to do with pinyin.

Comment 22

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

Comment on attachment 8394042 [details] [review]
pull request 17374

Can we test the patch?

Comment 23

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #22)
> Comment on attachment 8394042 [details] [review]
> pull request 17374
> 
> Can we test the patch?

I will test later and ask partner to verify as well

Comment 24

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Patch verified OK by partner

Thanks

Vance

Comment 25

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

Comment on attachment 8394042 [details] [review]
pull request 17374

Tim,

Could you help review again so that we can land this?
Thank you.

Comment 26

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

Comment on attachment 8394042 [details] [review]
pull request 17374

Patch updated to add unit test and address review comments.
Will send review after travis is green.

Comment 27

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

Comment on attachment 8394042 [details] [review]
pull request 17374

Patch upated for: 
 - Add unit test, note that the test for jspinyin has been removed, since it may conflict with the newly added tests, and since it has been disabled anyway.

Tim, please help review it again.
Thanks.

Comment 28

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

Comment on attachment 8394042 [details] [review]
pull request 17374

You should not remove JSPinyin tests since it's enabled by default for engineering build so we should be able to run them.

Thank you for adding a test. I do not know we have no any test at first place. I think we should now concentrate the test script effort to the new keyboard instead.

Comment 29

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

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #28)
> Comment on attachment 8394042 [details] [review]
> pull request 17374
> 
> You should not remove JSPinyin tests since it's enabled by default for
> engineering build so we should be able to run them.
> 

What I meant by "disabled" os for jspinyin tests themselves, please see
https://github.com/mozilla-b2g/gaia/blob/f5132420bef0d4dc27dfcb32bb9c5df2ab80530a/apps/keyboard/test/unit/jspinyin_test.js#L9

Without removing them, the tests would conflict with the newly added tests.
Could you confirm we can remove jspinyin tests for now?

Thanks.

Comment 30

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

Yes, the test is disabled on 1.3. But I don't understand why keeping the files in the tree can result conflict in any way ....?

Comment 31

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

This is because jspinyin would still require some files which would define some global objects, like 'window.InputMethods' with this line,
requireApp('keyboard/test/unit/setup_engine.js');

However, we defined this in keyboard.js, which would cause conflict because const could be defined more than once.
const InputMethods = {}.

Comment 32

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

Landed to v1.3,
https://github.com/mozilla-b2g/gaia/commit/b38047ff76a532cfd8300928b4d75e037aa03c00

Will have separate patches for v1.4 and master.

Comment 33

[Ryan VanderMeulen [:RyanVM]]

I don't see any v1.3 approval here?
https://wiki.mozilla.org/Release_Management/B2G_Landing#Landing_Procedure_3

Please get approval or revert ASAP. This policy change took effect over a month ago.

Comment 34

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

Sorry that I forgot to ask for approval first, patch reverted,
https://github.com/mozilla-b2g/gaia/commit/b789780c53adaac199787f51615375f721edfef4

Comment 35

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

Comment on attachment 8394042 [details] [review]
pull request 17374

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): N/A, might be a hidden bug for a long while.
[User impact] if declined: The keyboard would be totally unusable if this occurs.
[Testing completed]: Yes, manual tests and unit tests included.
[Risk to taking this patch] (and alternatives if risky): Should be low, just remove the event listener when the keyboard is hidden or switched.
[String changes made]: N/A.

Comment 36

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

Landed.

Gaia master,
https://github.com/mozilla-b2g/gaia/commit/77dab58bc492c26441132ee7641cfcda26f3ca90

v1.4,
https://github.com/mozilla-b2g/gaia/commit/e849783b647035552e9bb289e8cb8b8ddfa22aaf

v1.3,
https://github.com/mozilla-b2g/gaia/commit/812838ad0fabf51fa14435af562ddac6d26fa936

Comment 37

[William Hsu [:whsu]]

Verified it. Thanks.

* Build Information:
 - Gaia      4c3b2f57f4229c5f36f0d8fd399e65f4db88f104
 - Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/3aaca223b673
 - BuildID   20140330160202
 - Version   30.0a2

Comment 38

[William Hsu [:whsu]]

Thanks Robert! :)
Added.
- https://moztrap.mozilla.org/manage/case/12019/