1250102 - Have ElementManager#find return a promise

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Andreas Tolfsen ❲:ato❳]

ElementManager#find currently takes success- and error callbacks.  We wrap this in a promise where it’s used and pass along resolve- and reject functions.

This is unnecessary and we can get rid of the callback functions altogether by having the ElementManager#find method return a promise directly.

This is similar to bug 1242655.

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: MozReview Request: Bug 1250102 - Correct exported symbol from testing/marionette/element.js; r?automatedtester

Review commit: https://reviewboard.mozilla.org/r/36055/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/36055/

Comment 2

[Andreas Tolfsen ❲:ato❳]

Attachment: MozReview Request: Bug 1250102 - Turn element keys into constants; r?automatedtester

Review commit: https://reviewboard.mozilla.org/r/36057/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/36057/

Comment 3

[Andreas Tolfsen ❲:ato❳]

Attachment: MozReview Request: Bug 1250102 - Rewrite element location to be promise-compatible; r?automatedtester

Element location is rewritten with this patch in order to make it
compatible for use with promises.  This makes consuming the API nicer
in the wider context of Marionette, since it no longer takes callbacks
and no longer has to be wrapped in external promises to be compatible
with the new dispatching technique.

Review commit: https://reviewboard.mozilla.org/r/36059/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/36059/

Comment 4

[Andreas Tolfsen ❲:ato❳]

Attachment: MozReview Request: Bug 1250102 - Employ new element location API; r?automatedtester

Review commit: https://reviewboard.mozilla.org/r/36061/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/36061/

Comment 5

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8722510 [details]
MozReview Request: Bug 1250102 - Correct exported symbol from testing/marionette/element.js; r?automatedtester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36055/diff/1-2/

Comment 6

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8722511 [details]
MozReview Request: Bug 1250102 - Turn element keys into constants; r?automatedtester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36057/diff/1-2/

Comment 7

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8722512 [details]
MozReview Request: Bug 1250102 - Rewrite element location to be promise-compatible; r?automatedtester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36059/diff/1-2/

Comment 8

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8722513 [details]
MozReview Request: Bug 1250102 - Employ new element location API; r?automatedtester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36061/diff/1-2/

Comment 9

[David Burns :automatedtester]

Comment on attachment 8722510 [details]
MozReview Request: Bug 1250102 - Correct exported symbol from testing/marionette/element.js; r?automatedtester

https://reviewboard.mozilla.org/r/36055/#review32929

Comment 10

[David Burns :automatedtester]

Comment on attachment 8722511 [details]
MozReview Request: Bug 1250102 - Turn element keys into constants; r?automatedtester

https://reviewboard.mozilla.org/r/36057/#review32931

Comment 11

[David Burns :automatedtester]

Comment on attachment 8722512 [details]
MozReview Request: Bug 1250102 - Rewrite element location to be promise-compatible; r?automatedtester

https://reviewboard.mozilla.org/r/36059/#review32939

Comment 12

[David Burns :automatedtester]

Comment on attachment 8722513 [details]
MozReview Request: Bug 1250102 - Employ new element location API; r?automatedtester

https://reviewboard.mozilla.org/r/36061/#review32941

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c73978b005c8
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a57362d5fca
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b7d91ef0326
https://hg.mozilla.org/integration/mozilla-inbound/rev/52b85b8b8c14

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c73978b005c8
https://hg.mozilla.org/mozilla-central/rev/9a57362d5fca
https://hg.mozilla.org/mozilla-central/rev/2b7d91ef0326
https://hg.mozilla.org/mozilla-central/rev/52b85b8b8c14

Comment 15

[Syd Polk :sydpolk]

This change is causing a call in Marionette of getElementsByTagName('video') to fail when trying to run Netflix tests. See Bug 1251763.

Comment 16

[Syd Polk :sydpolk]

So, should this bug be REOPEN'd? What are the next steps to get this fixed? Should I move the bug I originally filed (bug 1251763) into the Marionette component? Do we have an ETA on this work?

Comment 17

[Andreas Tolfsen ❲:ato❳]

The patches here introduced a bug with findElement using the tag name, name, class name, link text, or partial link text selectors.  It is fixed in bug 1251763.

Comment 18

[Syd Polk :sydpolk]

Automation confirms the fix:

http://pf-jenkins.qa.mtv2.mozilla.com:8080/view/mn%20media/job/mn-eme-netflix-nightly-win_32_64/173/