1017919 - Reset Firefox: Preserve extensions but disable all of them

Status: NEW

(Firefox :: Migration, enhancement, P2)

Description

[Matthew N. [:MattN]]

A part of the initial Reset Firefox work that didn't get implemented was preserving the extensions but disabling them.

Comment 1

[Verdi [:verdi] Best to slack me]

Yes. This is the last deterrent to using or recommending reset. If a user has a legitimate add-on that they use, a reset deletes it and all of its data. It would be great if we can do this.

Comment 2

[:Gijs (he/him)]

(In reply to Verdi [:verdi] from comment #1)
> Yes. This is the last deterrent to using or recommending reset. If a user
> has a legitimate add-on that they use, a reset deletes it and all of its
> data. It would be great if we can do this.

We would still be deleting all of the add-on's data because we remove prefs (except for the homepage pref, IIRC) and don't copy over extra sqlite tables that they might create, or any other data.

Keeping all the prefs would quite often lead to re-breaking the thing people use reset for. There is no way to really distinguish the add-on prefs, and some add-ons modify built-in Firefox prefs, where if we reset those but keep the add-on prefs and they keep track of "did we do first run stuff", we'll likely break that kind of mechanism. So I would prefer not to (try to) migrate add-on prefs. If people re-enable add-ons, they can/will then set up the same prefs they had in the add-ons, and/or could copy over files from the backed up profile.

Comment 3

[Verdi [:verdi] Best to slack me]

When Matt and I talked about this, I understood that there was a way to preserve an add-on's data. For example, if you were using something like Zotero we would be able to keep your research data.

Comment 4

[Matthew N. [:MattN]]

From the URL linked in the bug:
>        do we want to migrate their add-on prefs as well? – Asa
>            We don't know which add-on prefs are safe so I suggest not –– MattN

Refresh Firefox generally follows a minimalist approach to avoid carrying issues over to the fresh profile. The more we migrate, the bigger chance that a Refresh doesn't solve the user's problem.

We could theoretically provide a hook to tell extensions we are doing a refresh and let them stash some data or give us data to stash in the new profile. Then we could also notify them we just finished a refresh in the new profile so they should restore data. Add-ons can already use a Firefox Account/Sync to store data instead.

I also prefer that we don't migrate data (at least for an initial implementation) for the above reasons and perhaps this gets easier with WebExtensions in the future.

Comment 5

[Verdi [:verdi] Best to slack me]

Matt, I thought we worked out that it was possible in Bug 1053594.

Comment 6

[Matthew N. [:MattN]]

That doesn't really say much new other than reminding me about default/preferences/foo.js[1] in bug 1053594 comment 1 but I'm not that familiar with it and don't think it will address the problems we listed. I also already said a way we could implement migration of settings in comment 4 here.

I wonder what happens if default/preferences/*.js defines a new value for our built-in prefs…

I'm confused about your insistence on migrating settings when it will make reset less effective (depending on the extensions installed).

[1] https://developer.mozilla.org/en-US/Add-ons/Overlay_Extensions/XUL_School/Handling_Preferences#Adding_preferences_to_an_extension

Comment 7

[Verdi [:verdi] Best to slack me]

Matt and I talked and thought we should discard the requirement to migrate add-on data at this time. Let's look into that again with when we have a new add-on system.

Comment 8

[:Gijs (he/him)]

Dão, we talked about this bug last meeting - are you currently working on it? If not, I think I could take a look at it, but I don't want to steal what isn't mine. Could you let me know? Thanks! :-)

Comment 9

[Dão Gottwald [:dao]]

I haven't started, feel free to take this.

Comment 10

[:Gijs (he/him)]

22:36	Gijs_away	Mossop: is there documentation about how we store add-on enabled states in the profile?
22:36	Mossop	Gijs_away: Not really, why do you ask?
22:37	Mossop	Because it's pretty awful
22:37	Gijs_away	Mossop: we want to basically copy add-ons over for firefox refresh, but disable all of them
22:37	Gijs_away	(and I expect that really means: all of the non-system ones)
22:37	Gijs_away	Mossop: so fx refresh works by just creating a new profile, and then copying what we want, leaving out all the stuff we don't want
22:38	Mossop	Gijs_away: Oh, well that should be easy. Copy the extensions folder over but not the database and then they'll automatically be disabled on startup
22:38	Gijs_away	Mossop: copying the extensions/ dir is pretty easy...
22:38	Gijs_away	Mossop: oh. Um, OK.
22:38	Gijs_away	Mossop: how do we determine that they exist? Do we just check that dir on startup?
22:38	Gijs_away	won't they appear to be sideloaded?
22:38	Mossop	Hrm, I guess though we'll label them as foreignInstalls in that case. Maybe you do want to do the database and mangle it. It's just a json bloc
22:38	Mossop	s/bloc/blob/
22:39	Gijs_away	Mossop: I see a sqlite file in this one profile I have... is that just old?
22:39	Mossop	Yes
22:39	Gijs_away	ok
22:39	Gijs_away	the other thing is obviously not all add-ons will be in that folder
22:39	Mossop	You want to set userDisabled and active properties to true and false respectively
22:39	Gijs_away	Right, OK
22:40	Mossop	Oh
22:40	Mossop	Gijs_away: Except that will disable the default theme too! So don't do it for that one!
22:40	Gijs_away	lol
22:40	Gijs_away	yet another place where we get to hardcode the uuid, wheee
22:41	Mossop	I think we have safeguards to correct in that case, but I don't know how good they are
22:41	Gijs_away	Mossop: and I guess there's a pref for the theme in use...
22:41	Gijs_away	though we don't copy prefs
22:41	Gijs_away	and the default should be classic/1.0
22:41	Gijs_away	or whatever that is
22:41	Gijs_away	so that should be OK
22:41	Gijs_away	Mossop: lightweight themes? Where are those?
22:41	Mossop	Yeah, and there is also extensions.ini
22:41	Mossop	I think we rebuild if that is missing on startup though
22:42	Mossop	Gijs_away: They're in prefs
22:42	Gijs_away	Oh.
22:42	Mossop	Horribly
22:42	Gijs_away	So I guess I get to prod around prefs for those
22:42	Gijs_away	that's suboptimal
22:42	Gijs_away	also I bet we cache the images
22:42	Gijs_away	and so quite conceivably you get the cached images
22:42	Gijs_away	move on with life
22:43	Gijs_away	and 2 years later you refresh Firefox and the original lwt is gone for whatever reason
22:43	Gijs_away	hrmpf
22:44	Mossop	Gijs_away: We cache them as files in the profile IIRC
22:45	Gijs_away	oh, and we reuse the filenames...
22:45	* Gijs_away is reading https://dxr.mozilla.org/mozilla-ce...ghtweightThemeImageOptimizer.jsm
22:47	Gijs_away	gah
22:47	Gijs_away	this might need to be followup material :s
22:58	Mossop	Gijs_away: Hmm. Do you want experiments too? It's probably a bad idea to copy them over and they're in the extensions directory
22:59	Gijs_away	Mossop: not particularly, no...
22:59	Gijs_away	Mossop: I think we really only care about user-installed add-ons
23:00	Mossop	Then you'll want to filter the experiments out. The database will also contain updated system add-ons but it will auto-correct when it spots they are missing
23:00	Mossop	Gijs_away: Maybe you should needinfo me on the bug, I'm bound to think of a few problems given some time


--> ni for Mossop.

Comment 11

[Dave Townsend [:mossop]]

This is challenging because we have all kinds of restrictions in place to attempt to block other applications from injecting add-ons into a profile. In one aspect that is ideal, sideloaded add-ons are automatically marked as disabled. But we also flag them in the database as forever sideloaded and currently require a higher level of signing (though that is going away in bug 1245956) and it may cause other behaviours in the future. So I think we want to avoid that, but that means bypassing the sideloading detection code somehow. Since we periodically make changes to that to catch bad actors unless we have good testing in place for this feature we're likely to break it by accident. My understanding is that Firefox reset has no tests currently, so that's a big concern. We might be able to take care of it with an xpcshell test if we're really careful.

So there are three classes of extensions we care about. Those that come with the application (system add-ons and the default theme), those that are sideloaded on the user's system and those in the profile.

The first two are easy, when Firefox starts it will do the right thing with them (once bug 1249074 is fixed), that is install and enable those that come with the app and disable those that are sideloaded and mark them as such.

The profile add-ons are harder. We need to make Firefox think they aren't sideloaded (when appropriate) and yet make Firefox call the bootstrap install hook for those add-ons that are restartless. So we basically have to trick Firefox into thinking they are staged installs by copying them to the staging directory in the profile and write out their extended metadata as a json blob that Firefox will read to import additional data like compatibility info and we'll have to add support to that for making them disabled and copying over some of the properties like the sideloading state. This is a method of sideloading that cheats the system and I'd really like to get rid of, so there's that!

Comment 12

[:Gijs (he/him)]

Not currently working on this.

Comment 13

[Evan Tseng [:evanxd]]

Just like Comment 10, it works (I tried it).
> Copy the extensions folder over but not the database and then they'll automatically be disabled on startup

We could backup all xpi files in the extensions directory and put them back into the new profile's extensions directory during the Refresh Firefox process. The all add-on will be shown but disable on the about:addons page after reseting and restarting the Firefox.

And it seems we need to update the `CreateResetProfile` or `ProfileResetCleanup` methods in the `ProfileReset.cpp` file to change the behavior of Refresh Firefox process and fix the issue.

[1]: http://searchfox.org/mozilla-central/source/toolkit/xre/ProfileReset.cpp#37

Comment 14

[:Gijs (he/him)]

(In reply to Evan Tseng [:evanxd] from comment #13)
> Just like Comment 10, it works (I tried it).
> > Copy the extensions folder over but not the database and then they'll automatically be disabled on startup
> 
> We could backup all xpi files in the extensions directory and put them back
> into the new profile's extensions directory during the Refresh Firefox
> process. The all add-on will be shown but disable on the about:addons page
> after reseting and restarting the Firefox.
> 
> And it seems we need to update the `CreateResetProfile` or
> `ProfileResetCleanup` methods in the `ProfileReset.cpp` file to change the
> behavior of Refresh Firefox process and fix the issue.
> 
> [1]:
> http://searchfox.org/mozilla-central/source/toolkit/xre/ProfileReset.cpp#37

No, that's not the right place to fix it. You should update the FirefoxProfileMigrator.js file.

Comment 15

[Evan Tseng [:evanxd]]

Thanks for the tips, Gijs.

We will come back to work on this after we ensure we would like to deliver this at this stage (release 57) and this is a P1 bug.

Comment 17

[Fischer [:Fischer]]

Based on the Photon Toronto Workweek meeting, we are not going to do auto-referesh (at least having an opt-out checkbox during installation) and after 57 there should only be webextension addons to consider. This bug doesn't affect Photon onboarding so de-scope it from Photon onboarding.