Closed
Bug 1037170
Opened 11 years ago
Closed 11 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•11 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•11 years ago
|
||
blocking-b2g: --- → 2.0?
Comment 3•11 years ago
|
||
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 4•11 years ago
|
||
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•11 years ago
|
||
Sloppy mistakes clean up. Thanks very much Jed.
Attachment #8454051 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 6•11 years ago
|
||
Error cleanup filed as bug 1037268
Comment 7•11 years ago
|
||
Keywords: checkin-needed
Comment 8•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S6 (18july)
Updated•11 years ago
|
blocking-b2g: 2.0? → 2.0+
Comment 9•11 years ago
|
||
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
•