997045 - Cannot focus on input field when doing element.click()

Status: RESOLVED FIXED

(Testing Graveyard :: JSMarionette, defect)

Description

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

When doing Gaia keyboard related testing, we found that sometimes on Travis, when we click on the input field in the tested app (not keyboard - the app with input field), we could not focus on it to trigger the Gaia keyboard.

We have done some experiments here like,
 - add pref 'focusmanager.testmode' as suggested in bug#952443 comment 11.

But it still caused a high failure rate.

Comment 1

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

In the end, we found that marionette python client would do switch_to_frame with focus parameter, which is default to true. 
http://dxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/marionette.py#902

And we don't have this "focus" parameter in both apps.switchToApp() and client.switchToFrame().

Comment 2

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

Attachment: Patch for marionette-js-client

WIP patch to add a parameter "focus" to switchToFrame, so that switchToApp may pass the info into this function.

I have not added tests for this change, but want to send this out for feedback.

Comment 3

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

Attachment: Patch for marionette-apps

WIP patch to make switchToApp will do switchToFrame() with focus = true.
As the above one, still need to add tests for this change.

Comment 4

[Evan Tseng [:evanxd]]

Hi Rudy,

Nice work!

f+,
but for marionette-app's patch, we should make sure the gaia's marionette tests could work well with it.

Thanks.

Comment 5

[Evan Tseng [:evanxd]]

For marionette-client's patch, we should use the `options` param instead of `focus` param, see the comment at https://github.com/mozilla-b2g/marionette-js-client/pull/111/files#r11717395.

Comment 6

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

Comment on attachment 8407475 [details] [review]
Patch for marionette-js-client

Hi Gareth, Evan,

Thanks for your feedback.
I've updated both patches and also added the unit tests, so I think this is ready to review.

--
BTW, I've created a pull request to Gaia to check this change won't break any tests that we have. 
https://github.com/mozilla-b2g/gaia/pull/18419

Comment 7

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

Comment on attachment 8407476 [details] [review]
Patch for marionette-apps

The marionette-js tests are all passed with this trying,
https://travis-ci.org/mozilla-b2g/gaia/builds/23210447

Gareth,

Could you please help review this?
Thanks.

Comment 8

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

Comment on attachment 8407475 [details] [review]
Patch for marionette-js-client

Patch updated again to address the review comments and as our discussion.

Evan,

Please help review it again.
Thanks.

Comment 9

[Gareth Aye [:gaye] (back from PTO)]

Comment on attachment 8407476 [details] [review]
Patch for marionette-apps

LGTM thanks :rudyl

Comment 10

[Gareth Aye [:gaye] (back from PTO)]

Comment on attachment 8407475 [details] [review]
Patch for marionette-js-client

Also a nice patch. Thanks :P

Comment 11

[Evan Tseng [:evanxd]]

Comment on attachment 8407475 [details] [review]
Patch for marionette-js-client

Hi Rudy,

r+, nice work!

Comment 12

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

marionette-js-client master,
https://github.com/mozilla-b2g/marionette-js-client/commit/84dfa6962db4a0ceaa796538992ec9bdd0e5e02e

Evan, Gareth,

Thanks for the review.

Comment 13

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

marionette-apps master,
https://github.com/mozilla-b2g/marionette-apps/commit/ea656ef4e6af74b95449ff27fabf35f25b3793de

Comment 14

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

gaia-node-modules updated,
https://github.com/mozilla-b2g/gaia-node-modules/commit/3a8ed55fca2d056543573b29463d0fb12902f7ea

Comment 15

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

Attachment: pull request to lock gaia's dependency on marionette-apps

We have to lock the dependency of marionette-apps to 0.3.3 or it would fail when you do "npm install" in gaia.
This is because we have published apps 0.3.4, which would "peer-depends" on client 1.1.6 while gaia itself depends 1.1.5.

We would update these when we make sure gaia-node-modules are correctly updated.

Evan,

Please help take a look.
Thanks.

Comment 16

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

Comment on attachment 8411749 [details] [review]
pull request to lock gaia's dependency on marionette-apps

Update the gaia-node-modules dependency in Gaia master,
https://github.com/mozilla-b2g/gaia/commit/ce0dc5e9c9fd2d3a5b4a8f949ae34a2293d2a83a

Comment 17

[Kevin Grandon :kgrandon]

Reverted in gaia for suspicion of breaking integration tests. I think this might be some sockit-to-me compilation issue, as we've seen this in the past. https://tbpl.mozilla.org/php/getParsedLog.php?id=38432781&tree=B2g-Inbound#error17

https://github.com/mozilla-b2g/gaia/commit/c5a8c3352c9ec3985674547a29e7fa047f7d6343

Comment 18

[Kevin Grandon :kgrandon]

FYI - I am looking into why we're having these node extension compilation problems, and will report back if there's any update.

Comment 19

[Kevin Grandon :kgrandon]

This should now be resolved as a part of: https://github.com/mozilla-b2g/gaia/commit/68e4ae3bc6af77091635e0b72d831748e6c2a620