Closed
Bug 1250102
Opened 8 years ago
Closed 8 years ago
Have ElementManager#find return a promise
Categories
(Remote Protocol :: Marionette, defect)
Remote Protocol
Marionette
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 | ||
Updated•8 years ago
|
Keywords: ateam-marionette-server
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → ato
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•8 years ago
|
||
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•8 years ago
|
||
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•8 years ago
|
||
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•8 years ago
|
||
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•8 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•8 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•8 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•8 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 9•8 years ago
|
||
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+
Updated•8 years ago
|
Attachment #8722511 -
Flags: review?(dburns) → review+
Comment 10•8 years ago
|
||
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•8 years ago
|
||
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+
Updated•8 years ago
|
Attachment #8722513 -
Flags: review?(dburns) → review+
Comment 12•8 years ago
|
||
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•8 years ago
|
||
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•8 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
Closed: 8 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Comment 15•8 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•8 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)
Assignee | ||
Comment 17•8 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)
Comment 18•8 years ago
|
||
Automation confirms the fix: http://pf-jenkins.qa.mtv2.mozilla.com:8080/view/mn%20media/job/mn-eme-netflix-nightly-win_32_64/173/
Updated•1 year ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•