Closed Bug 1708755 Opened 4 years ago Closed 3 years ago

Background update `about:preferences` toggle is not shown when per-installation pref has a user value

Categories

(Toolkit :: Application Update, defect)

defect

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox-esr78 --- unaffected
firefox88 --- unaffected
firefox89 --- disabled
firefox90 --- fix-optional

People

(Reporter: nalexander, Unassigned)

References

(Regression)

Details

(Keywords: regression)

This workaround with the default value of app.update.background.scheduling.enabled is complicated. From Anca on the QA team:

"When we were testing for the Nightly report, by setting app.update.background.experimental to true, implicit checking When Firefox/Nightly is not running option on the default profile, the settings stayed in place even after closing the default profile and opening another one (that become automatically the new default profile).

Now, with app.update.background.scheduling.enabled the new default profile does not inherit the settings, pref remains on false, and the UI is not displayed. The task is not removed from the Task Scheduler, and the background agent updates successfully if triggered immediately after the current (default) profile is closed. This looks like a bug since our previous understanding was that once the checkbox is checked (basically we enabled the feature per-installation, therefore it is shared among all profiles). Did something change on how are things handled with the current pref?"

I think we need to respect the pref or whether the per-installation pref has a user value. Or rework the UI display into a separate pref so that these two things aren't quite so coupled.

Set release status flags based on info from the regressing bug 1703302

(In reply to Nick Alexander :nalexander [he/him] from comment #0)

"When we were testing for the Nightly report, by setting app.update.background.experimental to true, implicit checking When Firefox/Nightly is not running option on the default profile, the settings stayed in place even after closing the default profile and opening another one (that become automatically the new default profile).

Now, with app.update.background.scheduling.enabled the new default profile does not inherit the settings, pref remains on false, and the UI is not displayed.

I don't really understand the change in behavior that you are describing. It sounds like you are saying that app.update.background.experimental somehow migrated its way into newly-created profiles. But I don't see how that would happen.

I also don't understand how app.update.background.scheduling.enabled is defaulting to false. The default on Nightly is true. Isn't the testing happening on Nightly?

The task is not removed from the Task Scheduler, and the background agent updates successfully if triggered immediately after the current (default) profile is closed.

I can understand how this would happen with the old mechanism that used caching to determine the task's current status. I don't really understand how this could happen with the current mechanism. I went to try to reproduce this problem, but it seems like it hinges on app.update.background.scheduling.enabled is defaulting to false, and I can't even reproduce that.

I think we need to respect the pref or whether the per-installation pref has a user value.

I think that you are suggesting showing UI based on the existence of a user value, but that sounds problematic. Like with regular Firefox preferences, we only store a user value if the value set is not equal to the default value. So I believe that this behavior would result in a visible UI disappearing when the setting is toggled (and about:preferences is refreshed). That seems undesirable to me.

Flags: needinfo?(nalexander)

Oh, I can see how changing the default profile to one with a different value of app.update.background.scheduling.enabled would result in that value being migrated to the per-installation pref. It seems to me that this is by design. If it didn't work this way, wouldn't we lose the ability to control this with Normandy?

I'm going to partially redirect this to Anca, to see if she can clarify what she was seeing. I'll leave the NI and try to think through this tomorrow (tricky, I have a full day) or Friday.

Flags: needinfo?(anca.soncutean)

Testing was done on Beta, the confusion on my part arose because the default value of app.update.background.scheduling.enabled is set on false. Although the user value (here setting the pref on true, and then implicit checking the “When Firefox is not running” checkbox option) should, as a general note, not be migrated to another profile, I suppose I’ve wrongly assume that somehow it should in this context, since the setting should be “per-installation”, not per-profile; and once it is enabled it should be enabled for all profiles.

Bellow, there are the exact steps I’ve used:

  1. Install Firefox (Beta)
  2. Go to ..\defaults\pref (installation folder) and change "app.update.channel" in order to prevent the update trigger
  3. Open Beta
  4. Confirm app.update.background.scheduling.enabled is set on false, “When Firefox is not running” option is not displayed in about:preferences
  5. Set the pref on true
  6. Go to about:preference and check the “When Firefox is not running” option checkbox
  7. Confirm that a task for the background update was created in Task Scheduler
  8. Close the browser and open it again, but with another profile
  9. Confirm app.update.background.scheduling.enabled is set on false, “When Firefox is not running” option is not displayed in about:preferences
  10. Close the browser
  11. Go to ..\defaults\pref (installation folder) and change "app.update.channel" back to “beta”
  12. Run the background agent task
  13. Verify the application.ini to see the build ID number

Actual result

  • Firefox is updated to the latest Beta version available (eg. from 89.0b3 and 89.0b6 to 89.0b8), but at this point, it seems to happen only after several manual trigger attempts. The logs show the same error even when the build ID is modified to the latest. see logs

Eventually this feature will be enabled by default, but until then is there something that could incommode our testing in Beta when using with multiple instances?

Running kind of similar steps on Nightly looks like the following:

Scenario 1

  1. Open Nightly with profile 1
  2. Confirm app.update.background.scheduling.enabled is set on true, “When Firefox is not running” option is checked
  3. Set app.update.background.scheduling.enabled on false, confirm “When Firefox is not running” option is no longer available
  4. Open Nightly with profile 2

Actual result

  • app.update.background.scheduling.enabled is set on true
  • “When Firefox is not running” option is checked

Scenario 2

  1. Open Nightly with profile 1
  2. Confirm app.update.background.scheduling.enabled is set on true, “When Firefox is not running” option is checked
  3. Uncheck “When Firefox is not running” option
  4. Open Nightly with profile 2

Actual result

  • app.update.background.scheduling.enabled is set on true
  • “When Firefox is not running” option is unchecked
  • background update task is not present in Task Scheduler
Flags: needinfo?(anca.soncutean)

(In reply to Anca Soncutean [:Anca], Desktop Release QA from comment #5)

Testing was done on Beta

Ah, I didn't realize we were testing on Beta right now. That clears up some of my questions.

the confusion on my part arose because the default value of app.update.background.scheduling.enabled is set on false. Although the user value (here setting the pref on true, and then implicit checking the “When Firefox is not running” checkbox option) should, as a general note, not be migrated to another profile, I suppose I’ve wrongly assume that somehow it should in this context, since the setting should be “per-installation”, not per-profile; and once it is enabled it should be enabled for all profiles.

Unless I am misunderstanding, in the situation that you have described, there is no user value on the per-installation pref. When app.update.background.scheduling.enabled is set to true, that changes the default value to true. Since there is no user value, this also changes the effective value to true. And since user values are only stored when the effective value doesn't match the default value, you will never have a stored user value of true while the default value is true.

When you switch to a different default profile with a different default value, the new default will get mirrored into the per-installation pref. This can potentially result in the profiles of different Windows users "fighting" over the default value, switching it back and forth. But, after talking to :nalexander about this, it sounds like that is the expected behavior.

Actual result

  • Firefox is updated to the latest Beta version available (eg. from 89.0b3 and 89.0b6 to 89.0b8), but at this point, it seems to happen only after several manual trigger attempts. The logs show the same error even when the build ID is modified to the latest. see logs

I'm not really sure what's going on there. This log isn't giving me a lot of useful information. Could you post the full log rather than a screenshot of it?

Eventually this feature will be enabled by default, but until then is there something that could incommode our testing in Beta when using with multiple instances?

It sounds to me like you will have an easier time of testing if you don't switch the default profile when you need to use a different profile. I'm not exactly clear on what you are doing now that causes this to happen but, if you let me know, I can advise further if that would be helpful.

Scenario 1

  1. Open Nightly with profile 1
  2. Confirm app.update.background.scheduling.enabled is set on true, “When Firefox is not running” option is checked
  3. Set app.update.background.scheduling.enabled on false, confirm “When Firefox is not running” option is no longer available
  4. Open Nightly with profile 2

Actual result

  • app.update.background.scheduling.enabled is set on true
  • “When Firefox is not running” option is checked

Are you switching the default profile to be profile 2? If so, this sounds like the expected result.

Scenario 2

  1. Open Nightly with profile 1
  2. Confirm app.update.background.scheduling.enabled is set on true, “When Firefox is not running” option is checked
  3. Uncheck “When Firefox is not running” option
  4. Open Nightly with profile 2

Actual result

  • app.update.background.scheduling.enabled is set on true
  • “When Firefox is not running” option is unchecked
  • background update task is not present in Task Scheduler

In this instance, a user value of false has been set on the per-installation pref (during step 3). So this sounds to me like the expected behavior.

Flags: needinfo?(nalexander)

Anca, could you answer the questions from comment 6?

Flags: needinfo?(anca.soncutean)

(In reply to Marco Castelluccio [:marco] from comment #7)

Anca, could you answer the questions from comment 6?

I was questioning what the expected behavior is, mostly because we were testing on Beta. But this behavior as described in comment 5, I don't think is relevant after all, since, for now, on Beta, the default value of app.update.background.scheduling.enabled is false. But, this scenario will not be encountered by the final users, once the feature will be enabled by default (as confirmed in comment 6 for the Nightly scenarios, that works as expected). As for the upcoming beta testing, I think the Normandy rollout will help us test this kind of scenario.

Flags: needinfo?(anca.soncutean)

To confirm, do I understand correctly that the issue you were seeing is related to flipping the pref manually in about:config, which is not how the normandy rollout will work, so this shouldn't be a problem in practice once we roll this out?

Flags: needinfo?(anca.soncutean)

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

To confirm, do I understand correctly that the issue you were seeing is related to flipping the pref manually in about:config, which is not how the normandy rollout will work, so this shouldn't be a problem in practice once we roll this out?

Yes, that is correct.

Flags: needinfo?(anca.soncutean)
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.