Rudy Lu [:rudyl] (inactive after 2015/7/3) Assignee
	Description • 10 years ago

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.

	Rudy Lu [:rudyl] (inactive after 2015/7/3) Assignee
	Comment 1 • 10 years ago

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().

	Rudy Lu [:rudyl] (inactive after 2015/7/3) Assignee
	Comment 2 • 10 years ago

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.

	Rudy Lu [:rudyl] (inactive after 2015/7/3) Assignee
	Comment 3 • 10 years ago

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

	Evan Tseng [:evanxd]
	Comment 4 • 10 years ago

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.

	Evan Tseng [:evanxd]
	Comment 5 • 10 years ago

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.

	Rudy Lu [:rudyl] (inactive after 2015/7/3) Assignee
	Comment 6 • 10 years ago

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

	Rudy Lu [:rudyl] (inactive after 2015/7/3) Assignee
	Comment 7 • 10 years ago

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.

	Rudy Lu [:rudyl] (inactive after 2015/7/3) Assignee
	Comment 8 • 10 years ago

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.

	Gareth Aye [:gaye] (back from PTO)
	Comment 9 • 10 years ago

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

LGTM thanks :rudyl

	Gareth Aye [:gaye] (back from PTO)
	Comment 10 • 10 years ago

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

Also a nice patch. Thanks :P

	Evan Tseng [:evanxd]
	Comment 11 • 10 years ago

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

Hi Rudy,

r+, nice work!

	Rudy Lu [:rudyl] (inactive after 2015/7/3) Assignee
	Comment 12 • 10 years ago

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

Evan, Gareth,

Thanks for the review.

	Rudy Lu [:rudyl] (inactive after 2015/7/3) Assignee
	Comment 13 • 10 years ago

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

	Rudy Lu [:rudyl] (inactive after 2015/7/3) Assignee
	Comment 14 • 10 years ago

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

	Rudy Lu [:rudyl] (inactive after 2015/7/3) Assignee
	Comment 15 • 10 years ago

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.

	Rudy Lu [:rudyl] (inactive after 2015/7/3) Assignee
	Comment 16 • 10 years ago

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

	Kevin Grandon :kgrandon
	Comment 17 • 10 years ago

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

	Kevin Grandon :kgrandon
	Comment 18 • 10 years ago

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

	Kevin Grandon :kgrandon
	Comment 19 • 10 years ago

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