964739 - [Keyboard][V1.4] Keyboard UI is broken while you rotate the device then switch keyboard

Status: VERIFIED FIXED

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

Description

[William Hsu [:whsu]]

Attachment: WP_20140128_009.mp4

* Description:
  This problem happened on v1.4 build (Mozilla Central).
  If you switch keyboard to third party keyboard then change the orientation of device, you will get a broken UI after you switch to built-in keyboard.
  Attaching the demo video.( WP_20140128_009.mp4 )

* Precondition:
  Install a third party keyboard

* Reproduction steps:
  1. Launch the browser app
  2. Tap "URL" bar to pop up keyboard
  3. Switch to third party keyboard
  4. Change the orientation of device.
  5. After keyboard is resized, switching keyboard to built-in keyboard

* Expected result:
  FxOS displays built-in keyboard and all UI are as expected

* Actual result:
  FxOS displays a broken UI

* Reproduction build:( Mozilla Central - V1.4 )
  - Gaia      f382061fe95750d584a9078175c421a36892afc9
  - Gecko     http://hg.mozilla.org/mozilla-central/rev/3f1dd2a8e972
  - BuildID   20140126040203
  - Version   29.0a1

Comment 1

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

Cannot reproduce this issue with v1.2, so this is a regression of current built-in keyboard.

Comment 3

[Jason Smith [:jsmith]]

This isn't a third-party keyboard bug. The dupe demonstrates a STR without using that feature.

Regression Window:

Last Working Environmental Variables:
Device: Buri v1.4 Mozilla RIL
BuildID: 20140109040203
Gaia: 47206ac66b084c6f6c4503a3b10d0e0760df2b6f
Gecko: 9409405e0739
Version: 29.0a1
Base Image: V1.2-device.cfg

First Broken Environmental Variables:
Device: Buri v1.4 Mozilla RIL
BuildID: 20140110040206
Gaia: f400efc804366c7b7cf5476d1d5d325e6651ee71
Gecko: 37516445a0b5
Version: 29.0a1
Base Image: V1.2-device.cfg

Comment 4

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

Traige: regression.

Rudy, could you take a look? Thanks.

Comment 5

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

Sure, this is on my plate.

Comment 6

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

Seems this regression was caused by the patch in bug 875963, adding the dependency so that we would know this should be fixed as well, if we want to uplift that patch to v1.3.

Comment 7

[Jason Smith [:jsmith]]

(In reply to Rudy Lu [:rudyl] from comment #6)
> Seems this regression was caused by the patch in bug 875963, adding the
> dependency so that we would know this should be fixed as well, if we want to
> uplift that patch to v1.3.

1.3 shouldn't be affected here. That patch never landed on 1.3.

Comment 8

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

Yes, Jason, thanks for pointing this out.
To make myself clear, I mean "if" we want to uplift that patch to v1.3T (or v1.3, but unlikely, I suppose).

Comment 9

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

Attachment: Patch V1 - pull request 16361

Hi Jan,

Could you please help review this simple patch that also cache the different keyboard UI per orientation, 

You may want to look at the last commit only, since I rebased my work on Ben's work of bug 972276,
https://github.com/RudyLu/gaia/commit/0728e64bdfdf12f460c8cef091fe4e9343bfa591

Thanks.

Comment 10

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 8377066 [details] [review]
Patch V1 - pull request 16361

r-'ing this, because using this approach we will have another instance of the keyboard in DOM, plus the overhead of generating it.

We should call resizeUI if we notice that the orientation changed since the last time the keyboard was visible, as it contains logic to handle this case already.

Comment 11

[Jan Jongboom [:janjongboom] (inactive)]

Attachment: Patch V2 - that calls resizeUI

I think this is cleaner and puts less of a burden on CPU.

Comment 12

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

Comment on attachment 8377209 [details] [review]
Patch V2 - that calls resizeUI

Jan,

Thanks for the counter patch. :)
r=me.

Comment 13

[Jan Jongboom [:janjongboom] (inactive)]

Waiting for https://bugzilla.mozilla.org/show_bug.cgi?id=972276 to land first...

Comment 14

[Jan Jongboom [:janjongboom] (inactive)]

https://github.com/mozilla-b2g/gaia/commit/18e305c4616e92df6f25bce619ca08b2ad5398d1

Comment 15

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

Uplifted to v1.3t, since this is regression caused by bug 875963, which is a 1.3t blocker,
https://github.com/mozilla-b2g/gaia/commit/2fa87ede3efd116d66c154108572b2e01108a71b

Comment 16

[William Hsu [:whsu]]

Verified it. Thanks for the help!

* Build information:
 - Gaia      8c9191df3c107df4073f3ca63816a1d36c51af5d
 - Gecko     https://hg.mozilla.org/mozilla-central/rev/923f1411f42f
 - BuildID   20140310080111
 - Version   30.0a1

* Test Result: Cannot reproduce