Have ElementManager#find return a promise

RESOLVED FIXED in Firefox 47

Status

Testing
Marionette
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ato, Assigned: ato)

Tracking

({ateam-marionette-server})

unspecified
mozilla47
ateam-marionette-server
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(4 attachments)

(Assignee)

Description

2 years ago
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)

Updated

2 years ago
Keywords: ateam-marionette-server
(Assignee)

Updated

2 years ago
Assignee: nobody → ato
Status: NEW → ASSIGNED
(Assignee)

Comment 1

2 years ago
Created attachment 8722510 [details]
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/
Attachment #8722510 - Flags: review?(dburns)
(Assignee)

Comment 2

2 years ago
Created attachment 8722511 [details]
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/
Attachment #8722511 - Flags: review?(dburns)
(Assignee)

Comment 3

2 years ago
Created attachment 8722512 [details]
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/
Attachment #8722512 - Flags: review?(dburns)
(Assignee)

Comment 4

2 years ago
Created attachment 8722513 [details]
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/
Attachment #8722513 - Flags: review?(dburns)
(Assignee)

Comment 5

2 years ago
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/
(Assignee)

Comment 6

2 years ago
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/
(Assignee)

Comment 7

2 years ago
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/
(Assignee)

Comment 8

2 years ago
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

Comment 14

2 years ago
bugherder
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
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47

Comment 15

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

Comment 16

2 years ago
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
(Assignee)

Comment 17

2 years ago
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.