Closed Bug 1413413 Opened 7 years ago Closed 7 years ago

Remove support for extensions having their own prefs file

Categories

(Core :: Preferences: Backend, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: n.nethercote, Assigned: n.nethercote)

References

Details

Attachments

(2 files)

Now that legacy extensions are no longer supported, we don't need generic extension prefs support.
Comment on attachment 8924053 [details] Bug 1413413 (part 2) - Remove support for extensions having their own prefs file. . https://reviewboard.mozilla.org/r/195272/#review200332 This looks sane to me, but redirecting to Kris who knows more about the history here...
Attachment #8924053 - Flags: review?(aswan) → review?(kmaglione+bmo)
Thanks for redirecting, aswan, kmag, what I'm after from you is a general "yes, this functionality is dead" rather than a fine-grained review, if you wee what I mean. Thanks!
Blocks: 1413432
Ahem, if you *see* what I mean.
Comment on attachment 8924053 [details] Bug 1413413 (part 2) - Remove support for extensions having their own prefs file. . https://reviewboard.mozilla.org/r/195272/#review200334 ::: modules/libpref/Preferences.cpp:2572 (Diff revision 1) > // Debugging to see why we end up with very long strings here with > // some addons, see bug 836263. This comment can be removed now. ::: toolkit/xre/nsXREDirProvider.cpp (Diff revision 1) > - if (mProfileDir) { > - nsCOMPtr<nsIFile> overrideFile; > - mProfileDir->Clone(getter_AddRefs(overrideFile)); > - overrideFile->AppendNative(NS_LITERAL_CSTRING(PREF_OVERRIDE_DIRNAME)); > - > - bool exists; > - if (NS_SUCCEEDED(overrideFile->Exists(&exists)) && exists) > - directories.AppendObject(overrideFile); > - } Hmm... I wonder if anyone is still using this... If I had to guess, I'd say it's probably being used by malware distributors, and by a few users who will complain. But those probably aren't good enough reasons to keep it.
Attachment #8924053 - Flags: review?(kmaglione+bmo) → review+
Comment on attachment 8924052 [details] Bug 1413413 (part 1) - Remove unused "@mozilla.org/preferences;1" ID. . https://reviewboard.mozilla.org/r/195270/#review200346
Attachment #8924052 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8924053 [details] Bug 1413413 (part 2) - Remove support for extensions having their own prefs file. . https://reviewboard.mozilla.org/r/195272/#review200348 ::: commit-message-a7377:9 (Diff revision 1) > + > +Pieces removed include the following. > + > +- The "load-extension-default" observer notification. > + > +- The code for reading defaults/preferences/*.js. from addons. We still read the one from the app.
Attachment #8924053 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/943f22d90aca3ac389b97c9ac3b19452ec130f4a Bug 1413413 (part 1) - Remove unused "@mozilla.org/preferences;1" ID. r=glandium. https://hg.mozilla.org/integration/mozilla-inbound/rev/43c726ab7f71353f4b8d0c14bca27d65edc6ad99 Bug 1413413 (part 2) - Remove support for extensions having their own prefs file. r=glandium,kmag.
(Looks like there wouldn't be any harm done by assigning this bug.) Nicholas, could you advise us as to how to best adopt the removed functionality into TB: https://hg.mozilla.org/integration/mozilla-inbound/rev/43c726ab7f71353f4b8d0c14bca27d65edc6ad99#l1.83 doesn't appear to be a lot of high-maintenance complicated code. The problems are 1) where to call this code and 2) how to gain access to the 'static void' functions like PREF_InitParseState.
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Flags: needinfo?(n.nethercote)
Now my legacy extensions don't load the default preferences. How can I do it manually?
(In reply to Oriol Brufau [:Oriol] from comment #12) > Now my legacy extensions don't load the default preferences. How can I do it > manually? Honestly, if you're still using non-bootstrapped extensions, you're going to have much bigger problems soon. (In reply to Jorg K (GMT+2) from comment #10) > (Looks like there wouldn't be any harm done by assigning this bug.) > > Nicholas, could you advise us as to how to best adopt the removed > functionality into TB: > https://hg.mozilla.org/integration/mozilla-inbound/rev/ > 43c726ab7f71353f4b8d0c14bca27d65edc6ad99#l1.83 > doesn't appear to be a lot of high-maintenance complicated code. The > problems are 1) where to call this code and 2) how to gain access to the > 'static void' functions like PREF_InitParseState. The traditional route bootstrapped extensions have taken in the past is to just use the subscript loader to load the prefs.js files into a scope with pref and user_pref functions defined.
(In reply to Jorg K (GMT+2) from comment #10) > Nicholas, could you advise us as to how to best adopt the removed > functionality into TB: > https://hg.mozilla.org/integration/mozilla-inbound/rev/ > 43c726ab7f71353f4b8d0c14bca27d65edc6ad99#l1.83 > doesn't appear to be a lot of high-maintenance complicated code. The > problems are 1) where to call this code and 2) how to gain access to the > 'static void' functions like PREF_InitParseState. Also, I'd advise against trying to use any pref service internals here. This code was removed because the pref service is being massively refactored in preparation for a rewrite in Rust. If you write any code that relies on its implementation, it is pretty much guaranteed to break again soon.
Flags: needinfo?(n.nethercote)
Oh, you beat me to it, I was going to clear the NI. Philipp is in the process of preparing something in bug 1414398, please take a look. > Honestly, if you're still using non-bootstrapped extensions, you're going to have much bigger problems soon. I'd say most TB extension as non-bootstrapped since they overload part of the UI. I'm just a novice add-on author, I found that for simple things bootstrapped extensions are OK, but for heavy stuff it needs to be non-bootstrapped. Our calendar add-on Lightning is non-bootstrapped. What's on the books?
(In reply to Jorg K (GMT+2) from comment #15) > > Honestly, if you're still using non-bootstrapped extensions, you're going to have much bigger problems soon. > > I'd say most TB extension as non-bootstrapped since they overload part of > the UI. I'm just a novice add-on author, I found that for simple things > bootstrapped extensions are OK, but for heavy stuff it needs to be > non-bootstrapped. Our calendar add-on Lightning is non-bootstrapped. It's perfectly possible for bootstrapped add-ons to overload parts of the UI. We've had many heavyweight bootstrapped add-ons that do just that, in Firefox. Regardless, support for non-bootstrapped legacy extensions is being removed from the add-on manager and other parts of the platform. It's a huge maintenance burden, and we can't continue to support it just for the sake of Thunderbird. > What's on the books? Not sure what you mean.
(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #16) > > What's on the books? > Not sure what you mean. Umm, "What's on the books?" was enquiring about your plans. You've already answered that, thank you.
(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #13) > The traditional route bootstrapped extensions have taken in the past is to > just use the subscript loader to load the prefs.js files into a scope with > pref and user_pref functions defined. Thanks, I defined the following pref function and manually ran the prefs.js file in the browser console and now the extension works again: function pref(n, v) { var p = Components.classes["@mozilla.org/preferences-service;1"] .getService(Components.interfaces.nsIPrefService); switch (typeof v) { case "boolean": p.setBoolPref(n, v); return; case "number": p.setIntPref(n, v); return; case "string": p.setCharPref(n, v); return; } } But this doesn't seem the right approach, I think I should define these values as the defaults instead of as user values. But I didn't find any method to define the default.
Kris, coming back to your comment #16: Could you give us a summary of what is planned for legacy add-ons, maybe post to Thunderbird email developers <maildev@lists.thunderbird.net> In comment #16 you're saying that support for non-bootstrapped add-ons will be removed. When will that happen? Is there a bug we can follow? What about bootstrapped add-ons? For how long will they be supported? What about devtools? They are going to be turned onto a system add-on (bug 1369801). And just an ignorant question: What's a system add-on. There is also mention of system add-ons in the "Proposal to remove some preferences override support" thread on dev-platform.
Flags: needinfo?(kmaglione+bmo)
(In reply to Oriol Brufau [:Oriol] from comment #18) > But this doesn't seem the right approach, I think I should define these > values as the defaults instead of as user values. But I didn't find any > method to define the default. Use nsIPrefService.getDefaultBranch and operate on the default branch.
Depends on: 1416279
For the record: Thunderbird 58 beta building with this backed out via comm-beta patch: https://hg.mozilla.org/releases/comm-beta/rev/c1f5d46aa9613ef790fa73dbfdea95fd0f9a7536
Flags: needinfo?(kmaglione+bmo)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: