1428459 - Divorce system add-ons from app.update prefs

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, enhancement, P3)

Description

[David Durst [:ddurst]]

I recognize that this echoes bug 1307563 a bit, but as has been discussed via email, we want to untangle system add-on updates from app.update.enabled and app.update.auto.

Instead, we want system add-ons updates to be governed by their own (different) pref. This pref should not have UI.

For the app update side, we want to remove app.update.enabled -- and we don't want to break system add-on updates in the process.

This will allow us to treat app updates and system add-on updates separately, if need be, as well as decrease the potential for users to mistakenly orphan themselves from important updates.

Comment 1

[Andy McKay]

I think there's a undesirable edge case there we should fix, since try to keep system add-ons up to date is a reasonably good idea. For that reason moving from a P5 to P3.

Comment 2

[Andrew Swan [:aswan]]

what is the undesirable edge case?

Comment 3

[David Durst [:ddurst]]

The updater system team is actively trying to get orphaned users updated (and mitigate future orphaning), and the subset (however small) who have modified that pref are explicitly one of our targets.

I can't speak to the urgency of it, but I know that we ran in some circles the last time we needed to rely on a system add-on's uptake; minimizing that churn is another goal.

Comment 4

[Robin Steuber (they/them) [:bytesized]]

I will probably start working on this bug soon. I intend to make system addon updates depend on a new pref: `extensions.systemAddon.update.enabled`.

Comment 5

[Robin Steuber (they/them) [:bytesized]]

Attachment: Bug 1428459 - Divorce system add-ons from app.update prefs

Currently system addons will not be updated if app.update.enabled or app.update.auto are set to false. This patch introduces a new pref, extensions.systemAddon.update.enabled, which will control system addon updates. app.update.enabled and app.update.auto will no longer affect system addon updates.

Review commit: https://reviewboard.mozilla.org/r/243896/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/243896/

Comment 6

[Robert Helmer [:rhelmer]]

Comment on attachment 8975646 [details]
Bug 1428459 - Divorce system add-ons from app.update prefs

https://reviewboard.mozilla.org/r/243896/#review249796

This lgtm, but have you tried running tests? I think https://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/test/xpcshell/test_system_update_custom.js#92 needs to be changed too at least.

Comment 7

[Robin Steuber (they/them) [:bytesized]]

Comment on attachment 8975646 [details]
Bug 1428459 - Divorce system add-ons from app.update prefs

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/243896/diff/1-2/

Comment 8

[Robert Helmer [:rhelmer]]

Comment on attachment 8975646 [details]
Bug 1428459 - Divorce system add-ons from app.update prefs

https://reviewboard.mozilla.org/r/243896/#review250034

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e6013642ac9d
Divorce system add-ons from app.update prefs r=rhelmer

Comment 10

[Eliza Balazs [:ebalazs_]]

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

Comment 11

[marius.santa]

Is manual testing required on this bug? If Yes, please provide some STR and the proper webextension(if required), if No set the “qe-verify-“ flag.

Comment 12

[Robert Helmer [:rhelmer]]

Attachment: simple test add-on for QA

Comment 13

[Robert Helmer [:rhelmer]]

Attachment: update XML for QA

Comment 14

[Robert Helmer [:rhelmer]]

Steps for QA:

In about:config set:
- extensions.systemAddon.update.url to https://bug1428459.bmoattachments.org/attachment.cgi?id=8980719
- xpinstall.signatures.required to false
- extensions.logging.enabled to true

In Browser Console run:
ChromeUtils.import("resource://gre/modules/AddonManager.jsm"); AddonManagerPrivate.backgroundUpdateCheck()

Once "Background update check complete" is seen in the Browser Console, there should be a new icon on the toolbar and the extension "Fox Candy" should appear in about:support under "Nightly Features"

Comment 15

[Robin Steuber (they/them) [:bytesized]]

Thank you :rhelmer for the above help with QA.

The steps he gave will ensure that system addons can be installed with `extensions.systemAddon.update.enabled=true`. These additional steps should be followed to make sure that setting `extensions.systemAddon.update.enabled=false` works as expected:

With a fresh profile:
In about:config set:
- extensions.systemAddon.update.url to https://bug1428459.bmoattachments.org/attachment.cgi?id=8980719
- xpinstall.signatures.required to false
- extensions.logging.enabled to true
- extensions.systemAddon.update.enabled to false

In Browser Console run:
ChromeUtils.import("resource://gre/modules/AddonManager.jsm"); AddonManagerPrivate.backgroundUpdateCheck()

Wait until "Background update check complete" is printed to the Browser Console. The "Fox Candy" extension should not be installed. The icon should not be visible in the toolbar nor should about:support list "Fox Candy" under "Nightly Features".

Comment 16

[marius.santa]

Attachment: system addon not installed.gif

Tested with the pref extensions.systemAddon.update.enabled set to True and False, system add-ons are only installed when the pref is set to True.
Tested and verified in Firefox 62.0a1 (20180530100110) on Windows 10 64Bit and MacOS 10.13.2.