Closed Bug 968929 Opened 7 years ago Closed 7 years ago
Client to production server URI
identity.fxaccounts.auth.uri sets the auth URI to use for various products. Firefox's URI was updated in bug 960332. Is this bug about updating Firefox OS's value (currently "https://api-accounts.dev.lcip.org/v1") or about changing the default in the case where the pref isn't set in (http://hg.mozilla.org/mozilla-central/annotate/d812f80a0f1d/services/fxaccounts/FxAccountsClient.jsm#l18)?
Component: Identity → FxA
This bug is about changing the default. It sounds like we should just read the same pref.
Per Gavin's comment, just use the pref that the Sync effort settled on
Comment on attachment 8374543 [details] [diff] [review] 968929-use-prod-uri.patch Review of attachment 8374543 [details] [diff] [review]: ----------------------------------------------------------------- ::: services/fxaccounts/FxAccountsClient.jsm @@ +14,5 @@ > Cu.import("resource://services-crypto/utils.js"); > Cu.import("resource://gre/modules/FxAccountsCommon.js"); > Cu.import("resource://gre/modules/Credentials.jsm"); > > +let _host = Services.prefs.getCharPref("identity.fxaccounts.auth.uri"); I think you want to keep the try/catch as it was, and make the new server url the default value of _host. The reason for this is that, in the event that a the identity.fxaccounts.auth.uri preference does not exists (either it's not in the build, or the user somehow deleted it), the getCharPref() call with throw an exception. As an experiment, go to Tools -> Web Developer -> Scratchpad. Then from the main menu, set Environment to Browser, and then run these two lines: Components.utils.import("resource://gre/modules/Services.jsm"); Services.prefs.getCharPref('i.like.pie'); It will throw an exception, which we do not want to have happen in FxAccountsClient.jsm. So you either need to do a try/catch as here, or check for the existence of the pref explicitly as in https://mxr.mozilla.org/mozilla-central/source/dom/identity/nsDOMIdentity.js#618; if there's a problem, fall back to the default. So basically I think all you need to do is change the initial value of _host (line 19) in the initial code to the new server url.
If you can't find the Scratchpad, you will need to pref devtools.chrome.enabled to true.
OK, got it. I was confused by the patch that Gavin pointed at -- I thought it meant that the pref in question would always be set. Feels a bit weird that the same crucial magic string is written in two places, but if that it the way it is done, so be it.
(In reply to Sam Penrose from comment #6) > Created attachment 8374572 [details] [diff] [review] > 968929-use-prod-uri.patch > > OK, got it. I was confused by the patch that Gavin pointed at -- I thought > it meant that the pref in question would always be set. Feels a bit weird > that the same crucial magic string is written in two places, but if that it > the way it is done, so be it. I agree, it's funky to have the same magic string in two places. But I still think we need something to fall back on in the case of a broken profile. Anyway, I think this should do it! Thanks for the fix! j
Comment on attachment 8374572 [details] [diff] [review] 968929-use-prod-uri.patch Ship it!
Attachment #8374572 - Flags: feedback?(jparsons) → review+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
(In reply to Jed Parsons (use needinfo, please) [:jedp, :jparsons] from comment #7) > I agree, it's funky to have the same magic string in two places. But I > still think we need something to fall back on in the case of a broken > profile. I don't think we need to support the "user manually removes default pref" case, since that can't happen in any reasonable situation (we already rely on default prefs always being there in many other places). That would suggest just removing the default fallback, and moving the other default from firefox.js/b2g.js into all.js.
You need to log in before you can comment on or make changes to this bug.