1687196 - Remove resetUserPrefs API as it is not reliable and only used in tests

Status: RESOLVED DUPLICATE of bug 1743079

(Core :: Preferences: Backend, defect)

Description

[Steven Penny]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:83.0) Gecko/20100101 Firefox/83.0

Steps to reproduce:

For example, if I set geo.enabled to false, previously I could call
Services.prefs.resetUserPrefs() to restore to default.

This is no longer working.

Comment 1

[Hani Yacoub, Desktop QA]

Managed to reproduce this issue only on MacOS 10.14, Windows 10 x64, Ubuntu 20.04 x64.
I'll provide a regression range as soon as possible.

Comment 2

[Mike Conley (:mconley) (:⚙️) (PTO - Sept 15 - 19)]

I am unable to reproduce this in the latest Nightly. Here are the steps I used:

1. In a new profile of Firefox Nightly, go to about:config
2. Search for geo.enabled, and toggle it to false
3. Use Cmd/Ctrl-Shift-J to open the Browser Console
4. In the console, type: Services.prefs.resetUserPrefs();

With these STR, I can see the geo.enabled pref being properly reset to true in about:config.

Am I missing a step somewhere, or misunderstanding how this bug is supposed to work?

Hani, if you're able to reproduce this reliably, would you be able to find us a regression range?

Comment 3

[Alice0775 White]

(In reply to Mike Conley (:mconley) (:⚙️) (Catching up on needinfos) from comment #2)

I am unable to reproduce this in the latest Nightly. Here are the steps I used:

1. In a new profile of Firefox Nightly, go to about:config
2. Search for geo.enabled, and toggle it to false
3. Use Cmd/Ctrl-Shift-J to open the Browser Console
4. In the console, type: Services.prefs.resetUserPrefs();

With these STR, I can see the geo.enabled pref being properly reset to true in about:config.

Am I missing a step somewhere, or misunderstanding how this bug is supposed to work?

Hani, if you're able to reproduce this reliably, would you be able to find us a regression range?

If I restarted the browser after step 2 and then did following steps 3 and 4, I was able to reproduce the problem.

Comment 4

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=59aec6da844c91cc1779a90a200a3a44ed61be54&tochange=4eaf01f13fbcdf3d99cfc96da10c79f577f3df25

Comment 5

[Hani Yacoub, Desktop QA]

A restart is needed after toggling "geo.enabled" to "false".
Thanks Alice0775 for the regression window.

Comment 6

[:Gijs (he/him)]

Kris, looks like your changes in bug 1471025 broke propagation of changes after a restart, are you able to take a look?

Comment 7

[Kris Maglione [:kmag]]

The problem is that ResetUserPrefs only iterates over the dynamic hashtable to find preferences that need to be reset, so it doesn't see any in the snapshot. It should be easy enough to fix by changing it to use for (auto& pref : PrefsIter(HashTable(), gSharedMap)) instead, but clearUserPrefs is also unused outside of one test, and untested, so I don't see it as particularly urgent.

Comment 8

[:Gijs (he/him)]

(In reply to Steven Penny from comment #0)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:83.0) Gecko/20100101 Firefox/83.0

Steps to reproduce:

For example, if I set geo.enabled to false, previously I could call
Services.prefs.resetUserPrefs() to restore to default.

This is no longer working.

Could you elaborate as to in what context this is happening, ie why and where are you calling Services.prefs.resetUserPrefs() ?

Comment 9

[Steven Penny]

Could you elaborate as to in what context this is happening, ie why and where
are you calling Services.prefs.resetUserPrefs() ?

I am opening up the Browser Console, and entering that command.

I dont think its unreasonable to want the ability to be able to reset preferences
I made have changed to the default. Why do I need to justify it? It seems the
use case, and the justification should be self evident.

Comment 10

[:Gijs (he/him)]

(In reply to Steven Penny from comment #9)

Could you elaborate as to in what context this is happening, ie why and where
are you calling Services.prefs.resetUserPrefs() ?

I am opening up the Browser Console, and entering that command.

I dont think its unreasonable to want the ability to be able to reset preferences
I made have changed to the default.

Right, but you don't technically need this API for that. Something like:

Services.prefs.getChildList("").filter(p => Services.prefs.prefHasUserValue(p)).forEach(p => Services.prefs.clearUserPref(p));

(or the for loop equivalent)

would work, though it's obviously less elegant and a bit less efficient than the dedicated API. That doesn't seem like something that would matter for your usecase, though. It also means you have a workaround.

Why do I need to justify it?

This isn't a courtroom or anything, so I don't think I would call it "justifying". I'm trying to understand what your usecase is, what other alternatives you have, and what the urgency of fixing this is - as Kris pointed out, there are no non-test users of this API in our own codebase, and no test coverage that ensures the API works, so it may make more sense for Firefox on its own to just remove the API outright rather than spend a bunch of time fixing both this bug and adding test coverage etc. You apparently have found some use for it, and I'm trying to figure out what that usecase is, and if it should change the decision we make in this bug.

It seems the use case, and the justification should be self evident.

It's not to me, at least! Why would you want to throw away every single preference you've customized? If you wanted to start off with a blank slate, a new profile or using Firefox Refresh would seem easier... Or if you're doing this in automation, you could just delete prefs.js from the profile directory.

It's also somewhat unusual for Firefox users to be running custom JS in the browser console. Its primary audience is browser developers, to be able to check/test/debug things quickly, so I'm curious why you find yourself using Firefox internals "manually" - perhaps your usecase, if it made sense outside of your situation, could be made easier by adding UI/UX for whatever it is you're trying to do.

Comment 11

[Clara Guerrero ( Need Info Brindusa Tot Please)]

Hi,
As per Comment 4 i'll update "Has regression range" accordingly.
Best,
Clara

Comment 12

[Steven Penny]

I finally tried out this comment [1], but it seems that prefHasUserValue is redundant, because if you run clearUserPref on something that is already default, it just does nothing. So it would only help I suppose in terms of speed. This seems to have same result:

for (let pref of Services.prefs.getChildList('')) {
   Services.prefs.clearUserPref(pref);
}

1. https://bugzilla.mozilla.org/show_bug.cgi?id=1687196#c10

Comment 13

[:Gijs (he/him)]

(In reply to Steven Penny from comment #12)

I finally tried out this comment [1], but it seems that prefHasUserValue is redundant, because if you run clearUserPref on something that is already default, it just does nothing. So it would only help I suppose in terms of speed.

Yeah, sorry for the confusion. In fact, though I haven't profiled it, I suspect it won't help much in terms of speed as the hashtable lookup is what's expensive, and you might as well do it just the once while also removing it, if present, rather than doing it twice for all the removed items...

This seems to have same result:

for (let pref of Services.prefs.getChildList('')) {
   Services.prefs.clearUserPref(pref);
}

Yep. Based on the comments so far, I think we should just remove the API. I'll update the summary accordingly.

Comment 14

[Gregory Pappas [:gregp]]

This removal was done in bug 1743079