924893 - [Keyboard] Chrome Event inputmethod-contextchange doesn't have info for the value selector

Status: RESOLVED FIXED

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

Description

[Jan Jongboom [:janjongboom] (inactive)]

When investigating bug 906096's failing UI tests I noticed that we indeed break the value selectors. This is because we switch from using mozKeyboard.onfocuschange to a chrome event with the name 'inputmethod-contextchange'. However we discard a lot of information that is currently send. We should make sure that the chrome event has the same info as the old onfocuschange (or change the architecture of value selector).

I have kept the old workaround in bug 906096 with a comment to this bug.

Comment 1

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

Jan, do you need help for the bug or it still managable for now?

Comment 2

[Jan Jongboom [:janjongboom] (inactive)]

No, I know what to do, just have to make time.

Comment 3

[Jan Jongboom [:janjongboom] (inactive)]

Attachment: Gecko patch

Here's the gecko part

Comment 4

[Jan Jongboom [:janjongboom] (inactive)]

Attachment: Gaia patch

Comment 5

[Jan Jongboom [:janjongboom] (inactive)]

checkin-needed for Gecko part.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d020cdb22780

Comment 8

[Jan Jongboom [:janjongboom] (inactive)]

Reopening as the gaia part didn't land yet.

Comment 9

[Yuan Xulei [:yxl] (Leave from May 18)]

Jan, please add "[leave open]" to the whiteboard if the bug should not be resolved fixed after the merge to m-c.

Comment 10

[Evelyn Hung]

Comment on attachment 817864 [details] [review]
Gaia patch

Most changes happen in System, I think it's appropriate to have Tim's review. :)

Comment 11

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

Comment on attachment 817864 [details] [review]
Gaia patch

It's good to have a second pair of eyes, but Rudy should review this.

Comment 12

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

Comment on attachment 817864 [details] [review]
Gaia patch

Looks good to me.
I tested this on device and FF nightly.

Jan, thanks for taking care of this.
:)

Comment 13

[Jan Jongboom [:janjongboom] (inactive)]

Landed in https://github.com/mozilla-b2g/gaia/commit/4fb5a545dc2dc6d89e6f912f787e019a770d907c

Comment 14

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

Uplifted 4fb5a545dc2dc6d89e6f912f787e019a770d907c to:
v1.2: 47c3ccfc7682d7caeeb8a921153caf8cefc5b480

Comment 15

[Jason Smith [:jsmith]]

This broke the select picker. This needs to be backed out.

John & Ryan - Can you backout the Gaia & Gecko patches here?

In other news - why isn't there an automated test preventing the date picker from breaking? Please get an integration test covering this.

Comment 16

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

I backed out the Gaia portion with:

[v1.2 276838e] Revert "Merge pull request #12892 from comoyo/value_selector_inpu

Comment 17

[Jason Smith [:jsmith]]

(In reply to John Ford [:jhford] -- please use 'needinfo?' instead of a CC from comment #16)
> I backed out the Gaia portion with:
> 
> [v1.2 276838e] Revert "Merge pull request #12892 from
> comoyo/value_selector_inpu

Can you back this out on master as well?

Comment 18

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

(In reply to Jason Smith [:jsmith] from comment #17)
> (In reply to John Ford [:jhford] -- please use 'needinfo?' instead of a CC
> from comment #16)
> > I backed out the Gaia portion with:
> > 
> > [v1.2 276838e] Revert "Merge pull request #12892 from
> > comoyo/value_selector_inpu
> 
> Can you back this out on master as well?

[master a8606f7] Revert "Merge pull request #12892 from comoyo/value_selector_in

Comment 19

[[:fabrice] Fabrice Desré]

Backed out:
https://hg.mozilla.org/integration/b2g-inbound/rev/862fbd85f847

Comment 20

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

Fabrice, I guess the value selector breakage Bug 931351 is because the following commit was not uplifted to aurora, i.e. our v1.2 depends on,
https://hg.mozilla.org/mozilla-central/rev/d020cdb22780

Do you think we should not backout this from m-c and do the uplift again?

Comment 21

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

Can someone help to uplift this patch to aurora for FxOS v1.2?
https://hg.mozilla.org/mozilla-central/rev/d020cdb22780

Thank you.

Comment 22

[Jason Smith [:jsmith]]

(In reply to Rudy Lu [:rudyl] from comment #20)
> Fabrice, I guess the value selector breakage Bug 931351 is because the
> following commit was not uplifted to aurora, i.e. our v1.2 depends on,
> https://hg.mozilla.org/mozilla-central/rev/d020cdb22780
> 
> Do you think we should not backout this from m-c and do the uplift again?

That's fine. We can reland the patch on inbound then. We just need to be careful on landing these patches in the right order - gecko first, then gaia for each branch.

Comment 23

[[:fabrice] Fabrice Desré]

Re-landed on b2g-inbound:
https://hg.mozilla.org/integration/b2g-inbound/rev/8301158fba85

Note to sheriff: no need to backout and reland on m-c, but this needs to be uplifted to aurora.

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8301158fba85

Comment 25

[Jason Smith [:jsmith]]

Reopen to land Gaia patch.

Comment 26

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

(In reply to Jason Smith [:jsmith] from comment #25)
> Reopen to land Gaia patch.

The Gecko part needs to uplift to aurora before landing Gaia patch everywhere. Any idea how this could happen?

Comment 27

[Ryan VanderMeulen [:RyanVM]]

It needs uplifting to B2G26, not Aurora. B2G26 is currently closed due to various perma-fails since the branch was commissioned. I will land it on B2G26 once it is reopened.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/6e5acf11e24c

Please resolve the bug when the Gaia patch hits master and set status-b2g-v1.2 to fixed when uplifted to v1.2.

Comment 29

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

Rudy, the Gaia patch should be ready for landing right? If so I could do it.

Comment 30

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

Yes, it is.
Thanks for your help.

Comment 31

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

master: a8606f77a25fab9497c7130eac6cf7a29a2783ac
v1.2: df049e3177ced0ca493ff0d192c65f18392bb462

Comment 32

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #31)
> master: a8606f77a25fab9497c7130eac6cf7a29a2783ac
> v1.2: df049e3177ced0ca493ff0d192c65f18392bb462

Tim, was something missed in the tests here?  It seems we are getting gaia-ui-test failures on v1.2 as of these commits?

  v1.2:    https://travis-ci.org/mozilla-b2g/gaia/jobs/13300770

Comment 33

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

(In reply to Ben Kelly [:bkelly] from comment #32)
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from
> comment #31)
> > master: a8606f77a25fab9497c7130eac6cf7a29a2783ac
> > v1.2: df049e3177ced0ca493ff0d192c65f18392bb462
> 
> Tim, was something missed in the tests here?  It seems we are getting
> gaia-ui-test failures on v1.2 as of these commits?
> 
>   v1.2:    https://travis-ci.org/mozilla-b2g/gaia/jobs/13300770

Would it be possible that Travis CI is not running with b2gdesktop containing the Gecko patch?
If you could rule out that we certainly backout the patch and figure out that later.

(it's midnight here so do whatever you see fit here please, thanks)

Comment 34

[Ben Kelly [:bkelly, not reviewing]]

James, is v1.2 running against mozilla_b2g26_v1_2 branch?  This travis failure may be due to missing a gecko patch.

Comment 35

[Anthony Ricaud (:rik)]

From bug 929062, it seems this is running against aurora. This should probably be changed to mozilla_b2g26_v1_2.

Comment 36

[James Lal [:lightsofapollo]]

I will do this today- Rik did most of the work but I will address the review comments myself and land this today.