Closed
Bug 1339163
Opened 7 years ago
Closed 7 years ago
Make TPS check restmail for verification emails
Categories
(Firefox :: Sync, defect, P2)
Firefox
Sync
Tracking
()
RESOLVED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: tcsc, Assigned: tcsc)
Details
Attachments
(1 file)
This plan was the outcome of some email discussion and https://github.com/mozilla/fxa/issues/216.
Updated•7 years ago
|
Priority: -- → P2
Updated•7 years ago
|
Assignee: nobody → tchiovoloni
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8848614 [details] Bug 1339163 - Make TPS tests attempt to automatically verify fxa emails when using a restmail account https://reviewboard.mozilla.org/r/121506/#review124324 Looks great, thanks! Can you please ping Karl and see what steps he thinks we need to take to get this stood up? ::: services/sync/tps/extensions/tps/resource/auth/fxaccounts.jsm:60 (Diff revision 1) > + userData = this.getSignedInUser(); > + return userData && userData.verified; > + }, > + > + async _openVerificationPage(uri) { > + // Better way of doing this? nope - although Services.wm is probably preferred to the .getService() dance. ::: services/sync/tps/extensions/tps/resource/auth/fxaccounts.jsm:69 (Diff revision 1) > + let newtab = mainWindow.getBrowser().addTab(uri); > + let win = mainWindow.getBrowser().getBrowserForTab(newtab); > + let resolve; > + await new Promise(res => { > + resolve = res; > + win.addEventListener("loadend", resolve, true); You can use win.addEventListener("loadend", resolve, { once: true }); and skip the remove. ::: services/sync/tps/extensions/tps/resource/auth/fxaccounts.jsm:76 (Diff revision 1) > + win.removeEventListener("loadend", resolve); > + if (await this.shortWaitForVerification(10000)) { > + mainWindow.getBrowser().removeTab(newtab); > + return true; > + } > + return false; Can you add a comment saying why you don't close the tab - I guess we leave the tab open to help with diagnosis? ::: services/sync/tps/extensions/tps/resource/auth/fxaccounts.jsm:191 (Diff revision 1) > Async.promiseSpinningly(FxAccountsConfig.ensureConfigured()); > > let client = new FxAccountsClient(); > client.signIn(account["username"], account["password"], true).then(credentials => { > return fxAccounts.setSignedInUser(credentials); > - }).then(() => { > + }) I really prefer |)}.then...|, but I don't mind if you feel strongly the leading . is better :)
Attachment #8848614 -
Flags: review?(markh) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8848614 [details] Bug 1339163 - Make TPS tests attempt to automatically verify fxa emails when using a restmail account https://reviewboard.mozilla.org/r/121506/#review124324 > nope - although Services.wm is probably preferred to the .getService() dance. That was the part I meant. > You can use win.addEventListener("loadend", resolve, { once: true }); and skip the remove. I had recently learned I could do this, then promptly forgot, thanks. > Can you add a comment saying why you don't close the tab - I guess we leave the tab open to help with diagnosis? It should have always closed the tab, I think. Rewrote so that it will.
Pushed by tchiovoloni@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/81e0dfa05b19 Make TPS tests attempt to automatically verify fxa emails when using a restmail account r=markh
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/81e0dfa05b19
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in
before you can comment on or make changes to this bug.
Description
•