Closed Bug 997045 Opened 6 years ago Closed 6 years ago

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

Categories

(Testing Graveyard :: JSMarionette, defect)

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rudyl, Assigned: rudyl)

References

Details

Attachments

(2 files, 1 obsolete file)

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.
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().
Assignee: nobody → rlu
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.
Attachment #8407475 - Flags: feedback?(evanxd)
WIP patch to make switchToApp will do switchToFrame() with focus = true.
As the above one, still need to add tests for this change.
Attachment #8407476 - Flags: feedback?(evanxd)
Attachment #8407476 - Flags: feedback?(evanxd) → feedback+
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.
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 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
Attachment #8407475 - Flags: review?(gaye)
Status: NEW → ASSIGNED
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.
Attachment #8407476 - Flags: review?(gaye)
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.
Attachment #8407475 - Flags: feedback?(evanxd) → review?(evanxd)
Comment on attachment 8407476 [details] [review]
Patch for marionette-apps

LGTM thanks :rudyl
Attachment #8407476 - Flags: review?(gaye) → review+
Comment on attachment 8407475 [details] [review]
Patch for marionette-js-client

Also a nice patch. Thanks :P
Attachment #8407475 - Flags: review?(gaye) → review+
Comment on attachment 8407475 [details] [review]
Patch for marionette-js-client

Hi Rudy,

r+, nice work!
Attachment #8407475 - Flags: review?(evanxd) → review+
Depends on: 998874
marionette-apps master,
https://github.com/mozilla-b2g/marionette-apps/commit/ea656ef4e6af74b95449ff27fabf35f25b3793de
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
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.
Attachment #8411749 - Flags: review?(evanxd)
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
Attachment #8411749 - Attachment is obsolete: true
Attachment #8411749 - Flags: review?(evanxd)
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
FYI - I am looking into why we're having these node extension compilation problems, and will report back if there's any update.
Depends on: 1001224
This should now be resolved as a part of: https://github.com/mozilla-b2g/gaia/commit/68e4ae3bc6af77091635e0b72d831748e6c2a620
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.