1716032 - Preferences set by users before enrolling into an experiment which uses the same value are deleted/reset at unenrollment

Status: VERIFIED FIXED

(Firefox :: Normandy Client, defect)

Description

[Maria Heres, inactive, contact Ecosystem QA]

[Affected Versions]:

• Firefox Beta 90.0b6 (Build ID: 20210610185818)
• Firefox Nightly 91.0a1 (Build ID: 20210610215038)

[Affected Platforms]:

• Windows 10
• Linux Mint 20
• macOS 11.1

[Prerequisites]:

• Have a Normandy default type recipe for an experiment that will create some preferences with values not default to Firefox.
  E.g. default type recipe for:
  “browser.topsites.experiment.ebay-2020-1” to “true”
• Have a user.js file that will enroll you in the experiment.
• Have Firefox open (not enrolled in study).

[Steps to reproduce]:

1. Navigate to “about:config”
2. Create preferences with the experiment values (i.e. “browser.topsites.experiment.ebay-2020-1” to “true”)
3. Restart the browser.
4. Open the “Profile Directory” folder and copy the user.js file from prerequisites there.
5. Restart Firefox.
6. Navigate to “about:studies” and unenroll from the study.
7. Restart Firefox.
8. Go to "about:config" and observe the preferences.

[Expected result]:

• Preferences set before enrolling are kept on user value after unenrollment.

[Actual result]:

• Preferences set before enrolling are deleted after unenrollment if they have the same value as the one used by that branch.

Comment 1

[Michael Cooper [:mythmon]]

The problem lies in /toolkit/components/normandy/lib/PreferenceExperiments.jsm#500-515. After detecting that the preference already has a user-set value, we unconditionally set a default value for it. Since Firefox doesn't save user prefs that are identical to default prefs at shut down, this causes the user pref to be erased.

Instead we should not set a default branch when it is the same as the user branch. We may also want to never change the default value if their is a user set value, but I'm less convinced about that.

Comment 2

[Maria Heres, inactive, contact Ecosystem QA]

Attachment: unenroll pref.gif

The issue is also reproducible for already existing preferences as long as between steps 5 and 6 the browser is restarted while enrolled in the study, as seen in the recording.

Comment 3

[Michael Cooper [:mythmon]]

Attachment: Bug 1716032 - Don't change default branch prefs to have match user branch prefs in Normandy experiments r=nanj

When the default and user branch values of a preference match, the user brach
value is effectively erased, and we keep no evidence that the user made a
choice. This changes makes normandy avoid that situation, so that user
preferences remain intact.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c57a4f31b1fe
Don't change default branch prefs to have match user branch prefs in Normandy experiments r=nanj

Comment 5

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/c57a4f31b1fe

Comment 6

[Julien Cristau [:jcristau]]

Is this something we'll need in 90?

Comment 7

[Maria Heres, inactive, contact Ecosystem QA]

I am reopening this issue as the fix seems to have only partially been effective. In the case of the original STR the issue seems fixed, while for the case described in comment 2, the issue is fully reproducible.
Tested using Firefox Nightly 91.0a1 (Build ID: 20210616214200) on Windows 10, Linux Mint 20 and macOS 11.1.

Comment 8

[Michael Cooper [:mythmon]]

(In reply to Julien Cristau [:jcristau] from comment #6)

Is this something we'll need in 90?

I think this is a fairly minor bug since the preferences we tend to do experiments on these days are dedicated preferences that shouldn't have pre-existing user-set values. Additionally, given QA's yellow sign off I think it is worth taking this slowly. Given that, I don't think we should uplift it for 90.

Comment 9

[Michael Cooper [:mythmon]]

Attachment: Bug 1716032 - Don't set any Normandy default branch experiment values if there is a user set value r=gijs

This should help with bugs where users lose user-set values after an experiment expires.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5931a0013981
Don't set any Normandy default branch experiment values if there is a user set value r=Gijs DONTBUILD

Comment 11

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/5931a0013981

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:mythmon, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 13

[Maria Heres, inactive, contact Ecosystem QA]

The latest fix does not seem to have modified the behavior. The exact steps in Comment 0 cannot reproduce the issue in either Firefox Nightly 92.0a1 (Build ID: 20210714215010) or Firefox Beta 91.0b2 (Build ID: 20210713163746) since the first fix. Bug 1720675 and Bug 1720677 have been opened for the behaviors encountered at different numbers of restarts after enrollment.
Considering this, I will mark this issue as verified fixed for the original behavior. Tested using Windows 10, Linux Mint 20 and macOS 11.1.

Comment 14

[Michael Cooper [:mythmon]]

The patch landed in nightly and beta is affected.
:mythmon, is this bug important enough to require an uplift?

I'm of the opinion this does not require an uplift.

If not please set status_beta to wontfix.

I don't see a status_beta flag on this bug. I'm going to assume there isn't one, since the bot also didn't change it.

Comment 15

[Julien Cristau [:jcristau]]

The bot meant status-firefox91. :)