Closed
Bug 1037170
Opened 9 years ago
Closed 9 years ago
Fire oncancel() when user cancels Sign Up/Sign In
Categories
(Firefox OS Graveyard :: FxA, defect)
Tracking
(blocking-b2g:2.0+, firefox31 wontfix, firefox32 fixed, firefox33 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)
People
(Reporter: spenrose, Assigned: spenrose)
References
Details
Attachments
(1 file, 1 obsolete file)
1.44 KB,
patch
|
Details | Diff | Splinter Review |
Intercept "DIALOG_CLOSED_BY_USER" and redirect to the cancel path.
Assignee | ||
Comment 1•9 years ago
|
||
Hi Jed -- another small one. My bad for not verifying all the flows and noting that the error object format changes depending on the dialog cancelled.
Attachment #8454051 -
Flags: review?(jed+bmo)
Assignee | ||
Comment 2•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=b8fed8d5937d
blocking-b2g: --- → 2.0?
Comment on attachment 8454051 [details] [diff] [review] 1037170-signin-oncancel.patch Review of attachment 8454051 [details] [diff] [review]: ----------------------------------------------------------------- Grody that our flows send error or error.error. That should be fixed, or maybe filed as a Good First Bug for someone to enjoy. From a quick scan through fxaccounts and toolkit/identity, this seems like the only place where we look into the error object, so this is fine as a no-risk, short-term fix with me. Also ok if you want to change Credentials.jsm logger, as mentioned below. r=me Thanks, Sam! ::: toolkit/identity/FirefoxAccounts.jsm @@ +12,5 @@ > Cu.import("resource://gre/modules/XPCOMUtils.jsm"); > Cu.import("resource://gre/modules/Services.jsm"); > Cu.import("resource://gre/modules/identity/LogUtils.jsm"); > > + Unimportant, but newline accidentally introduced. @@ +31,5 @@ > } catch (e) { > this.LOG_LEVEL = Log.Level.Error; > } > > +let log = Log.repository.getLogger("FirefoxAccounts"); If you want to change this from Identity.FxAccounts to FirefoxAccounts, you should also change the logger in Credentials.jsm. If you want to add the change to Credentials.jsm to this patch, I'm fine with that. @@ +190,4 @@ > // Cancellation is passed through an error channel; here we reroute. > + if ((error.error && (error.error.details == "DIALOG_CLOSED_BY_USER")) > + || (error.details == "DIALOG_CLOSED_BY_USER") > + ) { Microscopic nit: The placement of the braces and the || here is not quite in tune with Moz style. Style guide likes line breaks after || or &&, and the close paren and curly brace should be on the same line (above).
Comment on attachment 8454051 [details] [diff] [review] 1037170-signin-oncancel.patch Whoops - forgot to change the flag
Attachment #8454051 -
Flags: review?(jed+bmo) → review+
Assignee | ||
Comment 5•9 years ago
|
||
Sloppy mistakes clean up. Thanks very much Jed.
Attachment #8454051 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 6•9 years ago
|
||
Error cleanup filed as bug 1037268
Comment 7•9 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/d86018462663
Keywords: checkin-needed
Comment 8•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d86018462663
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S6 (18july)
Updated•9 years ago
|
blocking-b2g: 2.0? → 2.0+
Comment 9•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/e100e6b0efc1
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
status-firefox31:
--- → wontfix
status-firefox32:
--- → fixed
status-firefox33:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•