Closed
Bug 837023
Opened 12 years ago
Closed 7 years ago
(simple-prefs) changing the default type of a simple-pref causes an error while loading addons
Categories
(Add-on SDK Graveyard :: General, defect, P2)
Add-on SDK Graveyard
General
Tracking
(Not tracked)
RESOLVED
INCOMPLETE
People
(Reporter: evold, Unassigned)
References
Details
Paul brought this up in the mailing list https://groups.google.com/forum/?hl=en&fromgroups=#!topic/mozilla-labs-jetpack/ivRxVwBnU2E
It looks like the error occurs because the setting must have already existed in your profile as a different type. So say a previous version of your add-on used pref `x` with default of true, which is type boolean, then in a new version of your add-on you declare `x` to have a default of `"A"`, this is what caused the error below..
Timestamp: 12/24/12 10:53:42 PM
Error: mind-the-time: An exception occurred.
Traceback (most recent call last):
File "resource://gre/modules/NetUtil.jsm", line 140, in null
aCallback(pipe.inputStream, aStatusCode, aRequest);
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/net/url.js", line 42, in null
resolve(data);
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 142, in resolve
while (pending.length) result.then.apply(result, pending.shift())
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 36, in then
return { then: function then(resolve) { resolve(value) } }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 116, in resolved
function resolved(value) { deferred.resolve(resolve(value)) }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 142, in resolve
while (pending.length) result.then.apply(result, pending.shift())
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 36, in then
return { then: function then(resolve) { resolve(value) } }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 116, in resolved
function resolved(value) { deferred.resolve(resolve(value)) }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 142, in resolve
while (pending.length) result.then.apply(result, pending.shift())
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 36, in then
return { then: function then(resolve) { resolve(value) } }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 116, in resolved
function resolved(value) { deferred.resolve(resolve(value)) }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 142, in resolve
while (pending.length) result.then.apply(result, pending.shift())
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 36, in then
return { then: function then(resolve) { resolve(value) } }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 116, in resolved
function resolved(value) { deferred.resolve(resolve(value)) }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 142, in resolve
while (pending.length) result.then.apply(result, pending.shift())
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 36, in then
return { then: function then(resolve) { resolve(value) } }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 116, in resolved
function resolved(value) { deferred.resolve(resolve(value)) }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 142, in resolve
while (pending.length) result.then.apply(result, pending.shift())
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 122, in then
else result.then(resolved, rejected)
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 36, in then
return { then: function then(resolve) { resolve(value) } }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 116, in resolved
function resolved(value) { deferred.resolve(resolve(value)) }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 54, in effort
try { return f(options) }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 116, in resolved
function resolved(value) { deferred.resolve(resolve(value)) }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 142, in resolve
while (pending.length) result.then.apply(result, pending.shift())
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 36, in then
return { then: function then(resolve) { resolve(value) } }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 116, in resolved
function resolved(value) { deferred.resolve(resolve(value)) }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/core/promise.js", line 54, in effort
try { return f(options) }
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/addon/runner.js", line 91, in onLocalizationReady
run(options);
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/addon/runner.js", line 120, in run
setDefaultPrefs(options.prefsURI);
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/addon/runner.js", line 58, in setDefaultPrefs
evaluate(sandbox, prefsURI);
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/toolkit/loader.js", line 174, in evaluate
: loadSubScript(uri, sandbox, encoding);
File "jar:file:///Users/paul/Library/Application%20Support/Firefox/Profiles/5p425v2u.Developer/extensions/jid0-HYNmqxA9zQGfJADREri4n2AHKSI@jetpack.xpi!/defaults/preferences/prefs.js", line 1, in null
pref("extensions.jid0-HYNmqxA9zQGfJADREri4n2AHKSI@jetpack.autoRefreshPref", "A");
File "resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/addon/runner.js", line 51, in null
branch.setCharPref(key, val);
[Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.setCharPref]" nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)" location: "JS frame :: resource://gre/modules/XPIProvider.jsm -> jar:file:///Users/paul/Library/Application%20Support/Firefox/Profiles/5p425v2u.Developer/extensions/jid0-HYNmqxA9zQGfJADREri4n2AHKSI@jetpack.xpi!/bootstrap.js -> resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/toolkit/loader.js -> resource://jid0-hynmqxa9zqgfjadreri4n2ahksi-at-jetpack/addon-sdk/lib/sdk/addon/runner.js :: <TOP_LEVEL> :: line 51" data: no]
Reporter | ||
Updated•12 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Reporter | ||
Comment 1•12 years ago
|
||
One possibility here is to first check if the pref exists, and that it is of the same type, then if it does exist and is not of the same type, delete the old pref, and recreate it..
Reporter | ||
Comment 2•12 years ago
|
||
We should match w/e the behavior is for old school add-ons though, and I'm not sure what that is atm.
Comment 3•12 years ago
|
||
While deleting the old pref and recreating it (as Erik mentions in Comment 1) nicely prevents the error... it would mean that users would lose their settings. With automatic background updates of add-ons, they would also probably lose them without knowing that they had lost them or why.
So for better UX, add-on devs would still need to deal with converting old prefs to new ones.
(Fortunately, it's probably not that often that prefs need to change type. I only need to do it because radio buttons weren't an option when I first added the prefs to my add-on.)
Priority: -- → P2
Reporter | ||
Comment 4•10 years ago
|
||
(In reply to Paul Morris from comment #3)
> While deleting the old pref and recreating it (as Erik mentions in Comment
> 1) nicely prevents the error... it would mean that users would lose their
> settings. With automatic background updates of add-ons, they would also
> probably lose them without knowing that they had lost them or why.
>
> So for better UX, add-on devs would still need to deal with converting old
> prefs to new ones.
So this is currently possible, the use case at issue here is when the add-on dev has not managed their
upgrade. This is something that should be caught by AMO reviewers, but cold slip by notice is some cases. With the suggestion I made, it would less likely that AMO reviewers will catch the issue, which is a problem with it.
Irakli what do you think should happen here?
Flags: needinfo?(rFobic)
Reporter | ||
Updated•10 years ago
|
Blocks: sdk/simple-prefs
Comment 5•10 years ago
|
||
i don't think there is much that we could do that would work for all (or even most) cases. the addon author is in the best position to migrate the the preference by reading the user's old value, resetting (clearing) it, and storing a new one.
the best we can do to enable this is to ignore/not throw when we come up on such a case, and let the addon author deal with it using the sdk/preferences/service module as described above.
btw, note that there are two distinct branches of preferences that cause this problem, the default one (which we use to set default preferences on addon startup) and the user one (stored only when user makes a change). because of that, i believe bug 1012231 is related to this one, and we should also fix it (somehow).
Comment 6•10 years ago
|
||
I think what we could do is at the bootstrap if it's for install / upgrade reason check if default prefs match existing prefs (if they do exist) & if they don't erase existing prefs, set back a new prefs.
Erik do you by a chance know how such conflicts are resolved with bootstrapped or old-school addons ?
Flags: needinfo?(rFobic)
Updated•10 years ago
|
Flags: needinfo?(evold)
Reporter | ||
Comment 7•10 years ago
|
||
(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #6)
> I think what we could do is at the bootstrap if it's for install / upgrade
> reason check if default prefs match existing prefs (if they do exist) & if
> they don't erase existing prefs, set back a new prefs.
>
> Erik do you by a chance know how such conflicts are resolved with
> bootstrapped or old-school addons ?
bootstrap add-ons don't handle default preferences, it has to be manually done, which is why we have our own implementation in the sdk. I just checked old school add-ons and they do either:
1. old default value/type is replaced with new default value/type
2. if a user setting is made, then an error is thrown, the old user set value is kept, and the new default value/type is ignored. So if the old type was a string, and the new type is an int, the new type is ignored.
This latter piece, when trying to set a new default type for an old pref that has a user setting, seems less than ideal, because it would leave the add-on in a hairy situation to deal with.
I think it would be better do wipe away the old user set value/type and replace it with the new default value/type.
This may be better implemented as a platform fix than in the sdk repo, since there currently isn't a way to delete a single pref, and one can only delete a pref branch. So in order to delete the pref we'd have to record all of the values in the branch, delete the branch, and reset the values that we didn't want to delete.
Flags: needinfo?(evold)
Comment 8•7 years ago
|
||
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INCOMPLETE
You need to log in
before you can comment on or make changes to this bug.
Description
•