Closed Bug 1250102 Opened 7 years ago Closed 7 years ago

Have ElementManager#find return a promise

Categories

(Testing :: Marionette, defect)

defect
Not set
normal

Tracking

(firefox47 fixed)

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: ato, Assigned: ato)

References

Details

(Keywords: pi-marionette-server)

Attachments

(4 files)

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.
Assignee: nobody → ato
Status: NEW → ASSIGNED
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/
Attachment #8722512 - Flags: review?(dburns)
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 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 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 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 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
Attachment #8722510 - Flags: review?(dburns) → review+
Attachment #8722511 - Flags: review?(dburns) → review+
Comment on attachment 8722511 [details]
MozReview Request: Bug 1250102 - Turn element keys into constants; r?automatedtester

https://reviewboard.mozilla.org/r/36057/#review32931
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
Attachment #8722512 - Flags: review?(dburns) → review+
Attachment #8722513 - Flags: review?(dburns) → review+
Comment on attachment 8722513 [details]
MozReview Request: Bug 1250102 - Employ new element location API; r?automatedtester

https://reviewboard.mozilla.org/r/36061/#review32941
This change is causing a call in Marionette of getElementsByTagName('video') to fail when trying to run Netflix tests. See Bug 1251763.
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?
Flags: needinfo?(ato)
Depends on: 1251763
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.
Flags: needinfo?(ato)
You need to log in before you can comment on or make changes to this bug.