910697 - [Keyboard][V1.2] FxOS shows up the previous keyboard layout before displaying the correct one.

Status: VERIFIED FIXED

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

Description

[William Hsu [:whsu]]

* Description:
  This case happened on V1.2 build. V1.1.0 didn't have this problem.
  If you hide the keyboard before switching the different types of keyboard layout, FxOS will temporarily display previous keyboard layout then display correct one.

* Reproduction steps:
  1. Launch the contact app
  2. Tap "+" to add a new contact
  3. Tap the "Name" field
  4. Tap the empty space or swipe down the keyboard to hide the keyboard.
  5. Tap the "Phone" field

* Expected result:
  FxOS displays the numeric keyboard layout.

* Actual result:
  FxOS displays the text keyboard layout then numeric keyboard layout.

* Reproduction build:(Mozilla-Central)
  + Mercurial-Information
    - Gecko revision="416075f77249"
  + Git-information
    - Gaia revision="94c3dde50cb8da6e9d9ed137bfbf895f8699f8e7"(From Rudy's repository)
  + Gecko version: 26.0a1


Thanks!

Comment 2

[Ivan Tsay (:ITsay)]

Visual issue. Need to fix

Comment 3

[Jan Jongboom [:janjongboom] (inactive)]

Stealing this. Ping me if that's not OK. I spent some time on this before.

Comment 4

[Jan Jongboom [:janjongboom] (inactive)]

Attachment: Patch

* Fix showIME and hideIME as they don't do anything at the moment. Uses visibility rather than display to ensure that we don't need another full reflow of all keys.
* Don't call showIME before creating the new keyboard is complete
* Don't call hideIME if a focus event comes directly after blur
* Test cases around all this

Comment 5

[David Flanagan [:djf]]

Comment on attachment 810575 [details] [review]
Patch

This looks good, but r- because now that the keyboard gets hidden, I can see the homescreen wallpaper show through when the keyboard is opening or when the candidate panel is closing, so I think you need to add an opaque background to the keyboard to accompany this change.

(To reproduce: In the messaging app tap on the message input and then on the address input and go back and forth. I see homescreen the first time that the keyboard opens and then when the candidate panel closes.  I also see this in the UITests keyboard tests panel.  Tap on input type="text", see the keyboard, then dismiss the keyboard and tap on input type="number". While the keyboard is switching, the homescreen shows through.)

Maybe that needs to be fixed in gecko, and if so, then just filing a followup bug is enough and I'll give r+ here.

I'd also be interested to know if you can call renderKeyboard() and showIME() before the input method is activated. That might be a bit more responsive. If you can't that seems like a failure of the current architecture.  I don't think it is refactoring for this bug, though, so I wouldn't recommend spending a lot of time on that.

Comment 6

[David Flanagan [:djf]]

I should mention that my testing was on a Unagi running a mozilla-aurora nightly build from today.

Comment 7

[Jan Jongboom [:janjongboom] (inactive)]

Hi David, work for background showing has been done in the other keyboard render bug marked koi+ that I asked you to review as well.

Comment 8

[Ivan Tsay (:ITsay)]

Hi Jan,

Would you please assign one of the target milestones (1.2 C3 ~ 1.2 C6) for this bug depending on time you need for resolving this one? We need the flag to check when this bug will be resolved. Thank you!

Ivan

Comment 9

[Jan Jongboom [:janjongboom] (inactive)]

Well that would depend on when someone has time to review this, the patch is all ready.

Comment 10

[Ivan Tsay (:ITsay)]

Understood about the review process. So the review time should be considered when assigning the target milestone.

Comment 11

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

Comment on attachment 810575 [details] [review]
Patch

Sorry about the delay to look at your patch.
Could you incorporate the patches that this one depends so that we could have a check at the same time?

Thank you.

Comment 12

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 810575 [details] [review]
Patch

The issue David saw was already landed in bug 912028. I rebased against master.

Comment 13

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

Comment on attachment 810575 [details] [review]
Patch

I have seen some UI issues when testing this patch, and that might not be caused by this patch exactly.
 - The keyboard will disappear and re-appear when you switch focus in contact app.
 - The homescreen might be seen when the keyboard height changes.

I'd like to loop in Gary to help to check the patch with Jan.

Gary, could you help to check this issue/patch and Bug 912033#c9?
Thank you.

Comment 14

[Jan Jongboom [:janjongboom] (inactive)]

Disappear and re-appear, see bug 931005 (patch attached).
Homescreen might be seen when keyboard height changes, see bug 931710. But is unrelated, is also on master.

Comment 15

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Comment on attachment 810575 [details] [review]
Patch

works fine on device and browser,

r=me, but still need to fix travis-ci issue and wait Rudy's comment.

thanks.

Comment 16

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

Comment on attachment 810575 [details] [review]
Patch

Looks good to me, but the unit tests keep failed on travis.

Comment 17

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

Comment on attachment 810575 [details] [review]
Patch

clear the review flag, since from my test on v1.2 1020 build, this seems to break the transition of sliding up.

--
I have a pull request trying to fix the unit tests.

Comment 18

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

Gary, do you have bandwidth to follow up for this issue since Jan is PTO right now?
Please refer to comment 17 and help testing this on v1.2 before Bug 932764 is fixed.

Comment 20

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Rudy, I am doing on this issue.
==== keep ni:gchen to remind myself ===

Comment 21

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Comment on attachment 810575 [details] [review]
Patch

clear the r+ flag, since from my test on v1.2 build, keyboard can not be shown up.

Gaia:     cb981e2f47bc644b4d178d54378c3676c946facf
Gecko:    http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/eec4da1b27eb
BuildID   20131103004003
Version   26.0

Comment 22

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Attachment: 2013-11-05-14-30-07.png

Keyboard can not be shown up.

Comment 23

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Comment on attachment 810575 [details] [review]
Patch

Hi Jan,
   Since bug 91550 have change permission name into 'role', so your patch need to rebase with gaia master. Sorry my bad.

As discuss with Rudy, the animation comes more shortly than previous version but looks good and also fixed the issue about 'previous' keyboard layout shown up, so I'll give r+ again after fixing travis failed.

Thanks for your work.

Comment 24

[Jan Jongboom [:janjongboom] (inactive)]

Pushed rebased version, let's see if travis is happy.

Comment 25

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

Comment on attachment 810575 [details] [review]
Patch

r+, if 

 1. please help address
    https://github.com/mozilla-b2g/gaia/pull/12473#discussion_r7302383

 2. the unit tests pass.

Jan,

Thank you.

Comment 26

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

Jan, do you have time to address review comment today? This needs to land this week.

Comment 27

[Jan Jongboom [:janjongboom] (inactive)]

Yes

Comment 28

[Jan Jongboom [:janjongboom] (inactive)]

Unit tests pass, but now UI tests break for some *&W*$ reaason and the pop up/down anims don't work anymore. So I'm not gonna get this in today.

Comment 29

[Jan Jongboom [:janjongboom] (inactive)]

Alright, I figured it all out. The thing is that we only show the keyboard now when it's fully rendered, which also means that (because of bug 875963) rendering the keyboard takes like 100 ms. of blocking time. So the keyboard doesn't pop up nicely most of the times because the animation is already finished. So I don't think we should land this as koi+ as long as bug 875963 would also land here, and that bug is koi-. So I think we should also koi- this (or up the other one).

ni?'ing Ivan as he koi+'ed this.

Comment 30

[Ivan Tsay (:ITsay)]

Jan,

If we don't land this, It will always happen when input needed in numeric field, right?

Comment 31

[Jan Jongboom [:janjongboom] (inactive)]

Yes, same behavior as on current master.

Text field, blur, focus on number field, text keyboard pops up, 200 ms. later -> number keyboard shows.

Comment 32

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

(In reply to Jan Jongboom [:janjongboom] from comment #29)
> Alright, I figured it all out. The thing is that we only show the keyboard
> now when it's fully rendered, which also means that (because of bug 875963)
> rendering the keyboard takes like 100 ms. of blocking time. So the keyboard
> doesn't pop up nicely most of the times because the animation is already
> finished. So I don't think we should land this as koi+ as long as bug 875963
> would also land here, and that bug is koi-. So I think we should also koi-
> this (or up the other one).
> 
> ni?'ing Ivan as he koi+'ed this.

Jan,

Could we just invoke updateTargetWindowHeight(hide) after the keyboard is rendered completely?
So that the animation would start after the keyboard is rendered and shown as the correct one, right?

Thank you.

Comment 33

[Jan Jongboom [:janjongboom] (inactive)]

Not at the moment, because we start the animation as soon as the focus event comes in (from keyboard_manager).

Comment 34

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

(In reply to Jan Jongboom [:janjongboom] from comment #33)
> Not at the moment, because we start the animation as soon as the focus event
> comes in (from keyboard_manager).

Can we change the keyboard manager so it would only animate the frame _after_ receiving the first resize event, and add a failsafe timeout to that if the keyboard app doesn't call resize?

I am fine if you guys tell me this cannot be done with in v1.2 timeframe.

Comment 35

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

Yes, I think we should do this as Comment 34 suggested.
I did not do a detailed review, but it seems the patch in bug 875963 is too big to land into v1.2.

Jan,

Do you agree with the above suggestion?
If not, I think I'm going to handle this bug with Gary.

Comment 36

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Attachment: pull request: https://github.com/mozilla-b2g/gaia/pull/13686

Hi Tim and Rudy,
   I've just modify KM flow as our discussion.
   please help to review my patch(efaa561), it is base on Jan's patch.

   thanks.

Comment 37

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

Comment on attachment 832052 [details] [review]
pull request: https://github.com/mozilla-b2g/gaia/pull/13686

Please see comments and re-request review (or discuss this in person)

Comment 38

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Comment on attachment 832052 [details] [review]
pull request: https://github.com/mozilla-b2g/gaia/pull/13686

Hi Tim,
   please help to review it again.
   I still try to figure out how to fix 'gaia_ui_tests' and 'marionette'.

Comment 39

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

(In reply to GaryChen [:GaryChen][:PYChen] from comment #38)
> Comment on attachment 832052 [details] [review]
> pull request: https://github.com/mozilla-b2g/gaia/pull/13686
> 
> Hi Tim,
>    please help to review it again.
>    I still try to figure out how to fix 'gaia_ui_tests' and 'marionette'.

marionette passed, but gaia_ui_tests still failed.
https://travis-ci.org/mozilla-b2g/gaia/builds/13960512

Comment 40

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

Comment on attachment 832052 [details] [review]
pull request: https://github.com/mozilla-b2g/gaia/pull/13686

Great, thanks!

Comment 41

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

(In reply to GaryChen [:GaryChen][:PYChen] from comment #39)
> marionette passed, but gaia_ui_tests still failed.
> https://travis-ci.org/mozilla-b2g/gaia/builds/13960512

You might want to verify the timing of the automation script *thinks* the keyboard is shown, in Python, tomorrow cause this patch essentially changed the timing of keyboard rendering/transition.

Also, pay attention to TPBL after this hits master -- you do not want to back out code on Friday night so this means it would be ideal if we could resolve the tests and land this patch by Friday noon.

Thanks!

Comment 42

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #41)
> (In reply to GaryChen [:GaryChen][:PYChen] from comment #39)
> > marionette passed, but gaia_ui_tests still failed.
> > https://travis-ci.org/mozilla-b2g/gaia/builds/13960512
> 
> You might want to verify the timing of the automation script *thinks* the
> keyboard is shown, in Python, tomorrow cause this patch essentially changed
> the timing of keyboard rendering/transition.
> 
> Also, pay attention to TPBL after this hits master -- you do not want to
> back out code on Friday night so this means it would be ideal if we could
> resolve the tests and land this patch by Friday noon.
> 
> Thanks!

Actually, I passed these 'gaia_ui_tests' tasks on my device and desktop build but still failed in travis-ci.
So I have no idea how to fix patch/test case.

Here are my result logs.

running webserver on http://10.247.27.240:60371/
TEST-START test_sms_contact_input_validation.py
test_sms_contact_validation (test_sms_contact_input_validation.TestContactValidation) ... ok

----------------------------------------------------------------------
Ran 1 test in 14.424s

OK

SUMMARY
-------
passed: 1
failed: 0
todo: 0

starting httpd
running webserver on http://10.247.27.240:60398/
TEST-START test_sms_contact_match.py
test_contact_match (test_sms_contact_match.TestContactMatch) ... ok

----------------------------------------------------------------------
Ran 1 test in 13.183s

OK

SUMMARY
-------
passed: 1
failed: 0
todo: 0

starting httpd
running webserver on http://10.247.27.240:60414/
TEST-START test_email_keyboard.py
test_basic_email_keyboard (test_email_keyboard.TestEmailKeyboard) ... ok

----------------------------------------------------------------------
Ran 1 test in 14.063s

OK

SUMMARY
-------
passed: 1
failed: 0
todo: 0


starting httpd
running webserver on http://10.247.27.240:60463/
TEST-START test_keyboard_predictive_key.py
test_keyboard_predictive_key (test_keyboard_predictive_key.TestKeyboardPredictiveKey) ... ok

----------------------------------------------------------------------
Ran 1 test in 11.498s

OK

SUMMARY
-------
passed: 1
failed: 0
todo: 0

Hi :rwoods,
   Can you help to clarify this issue? Or help to redirect to right person
   Thanks a lots.

Comment 43

[Robert Wood [:rwood]]

According to Github history for test_sms_contact_input_validation.py it looks like the author of the test is Andrei Hutusoru, but I couldn't find him in the address list. Zac perhaps someone on your team is familiar with this/these tests?

Comment 44

[Zac C (:zac)]

It is one of our tests, yes. You can ignore the test_sms_contact_input_validation but the other test failure looks really bad. It looks like the whole b2g process is crashing. 
I can't test this properly just yet but I had a go on the device. I agree with Tim that it is probably timing related.

Can you try adding into tests/python/gaia-ui-tests/gaiatest/apps/keyboard.app.py onto line 105:
self.wait_for_condition(lambda m: keybframe.location['y'] == 0)
Place it after the find_element but before the switch_to_frame, to wait for the keyboard to have slid into position. In debugging I found that we could switch into it before it has finished animating.

This is a bit of an educated guess. It may still not be enough of a wait. On line 107 we could wait for isKeyboardRendered == true aswell. Let's see how it goes.

PS nice patch from UX perspective!

Comment 45

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

Zac,

I don't think it make sense for Gaia UI tests to invoke keyboard (and tap every keys) for every test involve inputs. In Marionette JS tests the client simply send a comment to inject the text into the field. IMHO this kind of approach produces more noise than accuracy. We need to break-up that in order to address keyboard system fixes in a timely matter. Do you agree?

Comment 46

[Zac C (:zac)]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #45)
> Zac,
> 
> I don't think it make sense for Gaia UI tests to invoke keyboard (and tap
> every keys) for every test involve inputs. In Marionette JS tests the client
> simply send a comment to inject the text into the field. IMHO this kind of
> approach produces more noise than accuracy. We need to break-up that in
> order to address keyboard system fixes in a timely matter. Do you agree?

I agree, QA have discussed this and we don't use the keyboard for every input field. Perhaps only in about 12 tests, half of which are necessary cases (send_keys does not work or is buggy) and in the remainder where we are testing the keyboard integration, eg the test listed above where we are testing the email keyboard type and the .com key, etc.

Comment 47

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

(In reply to Zac C (:zac) from comment #44)
> Can you try adding into
> tests/python/gaia-ui-tests/gaiatest/apps/keyboard.app.py onto line 105:
> self.wait_for_condition(lambda m: keybframe.location['y'] == 0)
> Place it after the find_element but before the switch_to_frame, to wait for
> the keyboard to have slid into position. In debugging I found that we could
> switch into it before it has finished animating.

I pushed a branch to run the test:
https://travis-ci.org/timdream/gaia/jobs/14071264

Comment 48

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

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #47)
> I pushed a branch to run the test:
> https://travis-ci.org/timdream/gaia/jobs/14071264

Spend a few time hitting re-run coz Mozilla servers was being maintained.

It looks like waiting for keybframe.location does not work:

----------

OK

TEST-START test_keyboard_predictive_key.py

test_keyboard_predictive_key (test_keyboard_predictive_key.TestKeyboardPredictiveKey) ... ERROR

ERROR

======================================================================

ERROR: None

----------------------------------------------------------------------

Traceback (most recent call last):

File "/usr/local/lib/python2.7/dist-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette_test.py", line 143, in run

testMethod()

File "/home/travis/build/timdream/gaia/tests/python/gaia-ui-tests/gaiatest/tests/functional/keyboard/test_keyboard_predictive_key.py", line 34, in test_keyboard_predictive_key

keyboard.tap_first_predictive_word()

File "/home/travis/build/timdream/gaia/tests/python/gaia-ui-tests/gaiatest/apps/keyboard/app.py", line 286, in tap_first_predictive_word

self.marionette.find_element(*self._predicted_word_locator).tap()

File "/usr/local/lib/python2.7/dist-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette.py", line 83, in tap

return self.marionette._send_message('singleTap', 'ok', id=self.id, x=x, y=y)

File "/usr/local/lib/python2.7/dist-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette.py", line 577, in _send_message

self._handle_error(response)

File "/usr/local/lib/python2.7/dist-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette.py", line 638, in _handle_error

raise MarionetteException(message=message, status=status, stacktrace=stacktrace)

TEST-UNEXPECTED-FAIL | test_keyboard_predictive_key.py test_keyboard_predictive_key.TestKeyboardPredictiveKey.test_keyboard_predictive_key | MarionetteException: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMWindowUtils.sendMouseEvent]

======================================================================

ERROR: None

----------------------------------------------------------------------

Traceback (most recent call last):

File "/usr/local/lib/python2.7/dist-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette_test.py", line 173, in run

self.tearDown()

File "/home/travis/build/timdream/gaia/tests/python/gaia-ui-tests/gaiatest/gaia_test.py", line 728, in tearDown

MarionetteTestCase.tearDown(self)

File "/usr/local/lib/python2.7/dist-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette_test.py", line 318, in tearDown

self.marionette.set_context("content")

File "/usr/local/lib/python2.7/dist-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette.py", line 803, in set_context

return self._send_message('setContext', 'ok', value=context)

File "/usr/local/lib/python2.7/dist-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette.py", line 553, in _send_message

raise MarionetteException(message="Please start a session")

TEST-UNEXPECTED-FAIL | test_keyboard_predictive_key.py test_keyboard_predictive_key.TestKeyboardPredictiveKey.test_keyboard_predictive_key | MarionetteException: Please start a session

----------------------------------------------------------------------

Ran 1 test in 253.205s

Comment 49

[Zac C (:zac)]

That's a different test failing but the failure is identical; it looks like b2g crashed again.

When Marionette says "Please start a session" in the tearDown, the tearDown could not be run because b2g crashed and the last session was terminated.

I have say now that I don't think this is a problem with the test. Something when tapping on the keyboard is causing b2g to crash.

Comment 50

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Hi Zac and Tim,
   Thank for yours help last weekend.

   I found if we wait 1 second[1],[2] before keyboard re-show and do second action, then travis will pass.
   https://travis-ci.org/mozilla-b2g/gaia/jobs/14123875

   But I think using hard code to wait a second is not reasonable, and we have already use 'wait_for_element_displayed'[3] for waiting the end of keyboard's animation in gaia master.

   Do you have any suggestion, Zac?
   Thanks.


[1]https://github.com/mpizza/gaia/commit/22185e77d6ba845198cb116fa7bf5214edfdb074#diff-9f5b6925009ad18c8e32cd016fecc431R26

[2]https://github.com/mpizza/gaia/commit/22185e77d6ba845198cb116fa7bf5214edfdb074#diff-c2f03a9ad721c43403962798994aec1cR31

[3]https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/apps/keyboard/app.py#L284

Comment 51

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

I've try to use 'wait_for_condition(lambda m: keybframe.get_attribute('data-transitionIn') != 'true')' to make sure keyboard frame's animation is finished.

https://github.com/mpizza/gaia/commit/418069e3ce4d98e7310ac221fe383d399b10d564#diff-e38d43f83b6a98c801b6ef51a54bafcaR105

In my local test are passed, but travis server still failed. I have no idea what happened on there?

Comment 52

[Zac C (:zac)]

(In reply to GaryChen [:GaryChen][:PYChen] from comment #50)
> Hi Zac and Tim,
>    Thank for yours help last weekend.
> 
>    I found if we wait 1 second[1],[2] before keyboard re-show and do second
> action, then travis will pass.
>    https://travis-ci.org/mozilla-b2g/gaia/jobs/14123875
> 
>    But I think using hard code to wait a second is not reasonable, and we
> have already use 'wait_for_element_displayed'[3] for waiting the end of
> keyboard's animation in gaia master.
> 
>    Do you have any suggestion, Zac?
>    Thanks.
> 
> 
> [1]https://github.com/mpizza/gaia/commit/
> 22185e77d6ba845198cb116fa7bf5214edfdb074#diff-
> 9f5b6925009ad18c8e32cd016fecc431R26
> 
> [2]https://github.com/mpizza/gaia/commit/
> 22185e77d6ba845198cb116fa7bf5214edfdb074#diff-
> c2f03a9ad721c43403962798994aec1cR31
> 
> [3]https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-
> tests/gaiatest/apps/keyboard/app.py#L284

Sorry you got fooled by randomness there; I re-triggered the Travis run and this time it's failing exactly as it was before so the sleeps did nothing.

I'm having another look at it this morning. 

Are you using ubuntu64?

Comment 53

[Zac C (:zac)]

I have replicated this locally and I managed to observe it failing.
The keyboard just does not open, yet Marionette thinks it is open and tries to tap on the buttons anyway.

Going in with more debuggin.

Comment 54

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

(In reply to Zac C (:zac) from comment #52)
> (In reply to GaryChen [:GaryChen][:PYChen] from comment #50)
> > Hi Zac and Tim,
> >    Thank for yours help last weekend.
> > 
> >    I found if we wait 1 second[1],[2] before keyboard re-show and do second
> > action, then travis will pass.
> >    https://travis-ci.org/mozilla-b2g/gaia/jobs/14123875
> > 
> >    But I think using hard code to wait a second is not reasonable, and we
> > have already use 'wait_for_element_displayed'[3] for waiting the end of
> > keyboard's animation in gaia master.
> > 
> >    Do you have any suggestion, Zac?
> >    Thanks.
> > 
> > 
> > [1]https://github.com/mpizza/gaia/commit/
> > 22185e77d6ba845198cb116fa7bf5214edfdb074#diff-
> > 9f5b6925009ad18c8e32cd016fecc431R26
> > 
> > [2]https://github.com/mpizza/gaia/commit/
> > 22185e77d6ba845198cb116fa7bf5214edfdb074#diff-
> > c2f03a9ad721c43403962798994aec1cR31
> > 
> > [3]https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-
> > tests/gaiatest/apps/keyboard/app.py#L284
> 
> Sorry you got fooled by randomness there; I re-triggered the Travis run and
> this time it's failing exactly as it was before so the sleeps did nothing.
> 
> I'm having another look at it this morning. 
> 
> Are you using ubuntu64?

MAC OS

Comment 55

[Zac C (:zac)]

I have replicated this locally on desktopb2g and I have also replicated this manually on device. 

STR
1. Flash this Gaia change to device or boot up a desktopb2g with this change
2. Open UI tests app
3. Scroll down, open "Keyboard" page5.
4. Tap in input type="text"
5. Type a keyboard key
6. Very quickly tap on the white space to the left of the input field and then tap back inside the input field.

The field will have focus but the keyboard will remain closed.

I'll attach a screenshot of it in this state.


E/GeckoConsole(  830): [JavaScript Error: "Return value is not an object."]
E/GeckoConsole(  830): [JavaScript Error: "NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [inIDOMUtils.setContentState]" {file: "chrome://global/content/BrowserElementPanning.js" line: 465}]
E/GeckoConsole(  830): [JavaScript Error: "Return value is not an object."]
E/GeckoConsole(  830): [JavaScript Error: "Return value is not an object."]
E/GeckoConsole(  830): [JavaScript Error: "NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [inIDOMUtils.setContentState]" {file: "chrome://global/content/BrowserElementPanning.js" line: 465}]

Comment 56

[Zac C (:zac)]

Attachment: Screenshot of field focussed, keyboard closed

Comment 57

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

(In reply to Zac C (:zac) from comment #53)
> I have replicated this locally and I managed to observe it failing.
> The keyboard just does not open, yet Marionette thinks it is open and tries
> to tap on the buttons anyway.
> 
> Going in with more debuggin.

Have you click 'b2g-desktop' while the test case is running?
We found we need to click the 'b2g-desktop' then b2g will get focus and keyboard will shown.

Here is my video;
http://youtu.be/6a_s8PcEx4o

Comment 58

[Zac C (:zac)]

(In reply to GaryChen [:GaryChen][:PYChen] from comment #57)
> Have you click 'b2g-desktop' while the test case is running?
> We found we need to click the 'b2g-desktop' then b2g will get focus and
> keyboard will shown.
> 
> Here is my video;
> http://youtu.be/6a_s8PcEx4o

I saw something like that but I couldn't quite put my finger on what was causing it.. whether it was the window focus or mouseover focus or something else. Then I found the main issue comment #55.

Comment 59

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

(In reply to Zac C (:zac) from comment #58)
> (In reply to GaryChen [:GaryChen][:PYChen] from comment #57)
> > Have you click 'b2g-desktop' while the test case is running?
> > We found we need to click the 'b2g-desktop' then b2g will get focus and
> > keyboard will shown.
> > 
> > Here is my video;
> > http://youtu.be/6a_s8PcEx4o
> 
> I saw something like that but I couldn't quite put my finger on what was
> causing it.. whether it was the window focus or mouseover focus or something
> else. Then I found the main issue comment #55.


Is it easy to reproduce?
I can not reproduce on my desktop build.
http://youtu.be/jnHKzrYXYMc

Or should I tap more faster?
I'll try it with device in office tomorrow morning.

BTW, thanks for your help.

Comment 60

[Zac C (:zac)]

Try tapping a bit faster. I think it's easier on device because it is not as powerful.

Comment 61

[Alex Keybl [:akeybl]]

over to koi? given https://bugzilla.mozilla.org/show_bug.cgi?id=932764#c12

We may be backing this out given the perf regression

Comment 62

[Jason Smith [:jsmith]]

(In reply to Alex Keybl [:akeybl] from comment #61)
> over to koi? given https://bugzilla.mozilla.org/show_bug.cgi?id=932764#c12
> 
> We may be backing this out given the perf regression

I don't think you commented on the right bug here - nothing has landed here yet. bug 932764 is a gfx regression, which is unrelated to this bug.

Comment 63

[Alex Keybl [:akeybl]]

Ah, so a backout to resolve bug 932764 wouldn't be necessary. We just wouldn't ever land this?

Comment 64

[Jan Jongboom [:janjongboom] (inactive)]

Sorry I was out last week at VelocityConf. I think this is pretty hard to get completely right because we take so long to render the damn keyboard. If we can speed that up (or if someone would ever review my patch for that) this would be a non-discussion.

Comment 65

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Hi Jan,
   As comment 34, we want to animate the keyboard frame after rendering keyboard is finished.
   So I think no matter how long we take for rendering keyboard, we still can fix this bug just like my patch.
   But I am trying to fix Zac motioned issue (comment 60) in my patch.

   Looking forward if you have any suggestion.

Comment 66

[Tony Chung [:tchung]]

Triage reviewed this to be koi+'d.  the original comment doesnt say anything about typing fast, and shows its a clear bustage of the new feature on device.   High user impact.

Comment 67

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Hi Tim,
   can you reivew this patch again, I've add some code in keyboard manager to prevent tap quickly let keyboard break issue (comment 60).

https://github.com/mozilla-b2g/gaia/pull/13686/files#diff-869edc18f465b7f5cdda81d81cd58a71L574

and this patch passed in travis.
https://travis-ci.org/mozilla-b2g/gaia/jobs/14233254

Comment 68

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

That's great, thanks!

Comment 69

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

landed in gaia master:
https://github.com/mozilla-b2g/gaia/commit/606c7446add4af53c1f1e8849d98416c00b5b938

travis passed:
https://travis-ci.org/mozilla-b2g/gaia/builds/14233250

Comment 70

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

https://hg.mozilla.org/integration/b2g-inbound/rev/b1a75e4a07e1

Comment 71

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

https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=b1a75e4a07e1

Comment 72

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

Uplifted 606c7446add4af53c1f1e8849d98416c00b5b938 to:
v1.2: ce276842c9ac1746073271fb736dfdb626a89240

Comment 73

[William Hsu [:whsu]]

Hi, all,

Sorry for the late reply.
I cannot reproduce this bug on latest V1.2 build.
Thanks!

* Test Build:
 - Gaia:     0659f16b9790b1cf9eba4d80743fcc774d2ffe3a
 - Gecko:    http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/af2c7ebb5967
 - BuildID   20131205004003
 - Version   26.0