Closed Bug 1420514 Opened 2 years ago Closed Last year

Remove or hide the update features in the preferences

Categories

(Firefox :: Preferences, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 63
Tracking Status
relnote-firefox --- 63+
firefox63 --- fixed

People

(Reporter: Sylvestre, Assigned: bytesized)

References

Details

(Whiteboard: [ux feature][To know how to disable updates, please see comment #c137])

User Story

Rationale:
The "Never check for Updates" option is easy to enable and forget about. Once enabled this exposes users to severe security issues and it seems unwise to expose this feature in the preferences? This also contributes to orphaned users.

Requirements:
- Remove "Never check for Updates" option
- Keep "Check for updates but let me chose to install them" so users can remain in control of what gets installed on their devices.
- Updates can be disabled via the policy engine

Attachments

(7 files)

We should remove (or move under an advanced option) the options to disable updates.

I don't see any good in this option as we fix critical issues in every release of Firefox.

Of course, the options should remain available under a pref but not available to lambda users.
Jeff, what do you think?
Flags: needinfo?(jgriffiths)
Summary: Remove or hide the update features → Remove or hide the update features in the preferences
See Also: → 1420570
Punting to RT to look into. I don't know what to think, I'd need to see data to better understand if this is worth doing. 

I'm mildly more interested in hiding the pref under advanced.
Flags: needinfo?(jgriffiths) → needinfo?(rtestard)
(In reply to Jeff Griffiths (:canuckistani) (:⚡︎) from comment #2)
> I'm mildly more interested in hiding the pref under advanced.

Side note, we don't have an "Advanced" in Prefs anymore, though we could create one.
Priority: -- → P3
(In reply to Jeff Griffiths (:canuckistani) (:⚡︎) from comment #2)
> Punting to RT to look into. I don't know what to think, I'd need to see data
> to better understand if this is worth doing. 
> 
> I'm mildly more interested in hiding the pref under advanced.

I updated the user story to clarify what's needed here.
I'll discuss this with Ddurst team in Q1 planning.
User Story: (updated)
Flags: needinfo?(rtestard)
Severity: normal → major
Priority: P3 → P1
Severity: major → normal
Priority: P1 → P3
Whiteboard: [ux feature]
User Story: (updated)
Assignee: nobody → ksteuber
Attachment #8979379 - Flags: review?(felipc)
Attachment #8979379 - Flags: review?(ato)
Attachment #8979643 - Flags: review?(robert.strong.bugs)
Attachment #8979377 - Flags: review?(chutten)
Attachment #8979644 - Flags: review?(ato)
Attachment #8979645 - Flags: review?(felipc)
Attachment #8979646 - Flags: review?(robert.strong.bugs)
Comment on attachment 8979377 [details]
Bug 1420514 - Remove telemetry probes tracking app.update.enabled

https://reviewboard.mozilla.org/r/245538/#review251932

LGTM
Attachment #8979377 - Flags: review?(chutten) → review+
Comment on attachment 8979378 [details]
Bug 1420514 - Remove the UI for app.update.enabled in about:preferences

https://reviewboard.mozilla.org/r/245540/#review252238
Attachment #8979378 - Flags: review?(jaws) → review+
Depends on: 1460082
Comment on attachment 8979644 [details]
Bug 1420514 - Replace app.update.enabled with app.update.disabledForTesting in the test harness

https://reviewboard.mozilla.org/r/245788/#review252864

::: js/src/tests/user.js:1
(Diff revision 1)
> -user_pref("app.update.enabled", false);
> +user_pref("app.update.disabledForTesting", true);

I can’t review changes to this file.

::: testing/geckodriver/src/prefs.rs:19
(Diff revision 1)
>  
>          // Disable automatic downloading of new releases
>          ("app.update.auto", Pref::new(false)),
>  
>          // Disable automatically upgrading Firefox
> -        ("app.update.enabled", Pref::new(false)),
> +        ("app.update.disabledForTesting", Pref::new(true)),

geckodriver is a program used out-of-tree by end-users and to
maintain backwards compatibility with earlier Firefoxen, we need
to keep the old app.update.enabled preference around until 62 reaches
stable.

Can you add app.update.enabled back in with a comment that it can
be removed when 62 becomes stable?

::: testing/geckodriver/src/prefs.rs:20
(Diff revision 1)
>          // Disable automatic downloading of new releases
>          ("app.update.auto", Pref::new(false)),
>  
>          // Disable automatically upgrading Firefox
> -        ("app.update.enabled", Pref::new(false)),
> +        ("app.update.disabledForTesting", Pref::new(true)),
> +        ("security.turn_off_all_security_so_that_viruses_can_take_over_this_computer", Pref::new(true)),

Where did this come from?  We removed this in the past because it
got removed from Gecko.

::: testing/marionette/client/marionette_driver/geckoinstance.py:505
(Diff revision 1)
>  
>  
>  class DesktopInstance(GeckoInstance):
>      desktop_prefs = {
>          # Disable application updates
> -        "app.update.enabled": False,
> +        "app.update.disabledForTesting": True,

For the same reasons as with geckodriver, the Marionette Python
client is used for Firefox update tests and needs to be backwards
compatible with earlier Firefoxen back to ESR.

Can you add app.update.enabled back with a note saying it can be
removed when 62 becomes ESR?

::: testing/marionette/components/marionette.js:68
(Diff revision 1)
>    // Disable automatically upgrading Firefox.
>    //
>    // This should also be set in the profile prior to starting Firefox,
>    // as it is picked up at runtime.
> -  ["app.update.enabled", false],
> +  ["app.update.disabledForTesting", true],
> +  ["security.turn_off_all_security_so_that_viruses_can_take_over_this_computer", true],

Why is this needed?

::: testing/raptor/test/test_raptor.py:26
(Diff revision 1)
>      assert isinstance(raptor.profile, BaseProfile)
>      if app != 'firefox':
>          return
>  
> -    # This pref is set in mozprofile
> -    firefox_pref = 'user_pref("app.update.enabled", false);'
> +    # These prefs are set in mozprofile
> +    firefox_prefs = [

For testing/raptor you should request r? from jmaher, as I’m not a
peer.

::: testing/talos/talos/unittests/test_talosconfig_browser_config.json:1
(Diff revision 1)
> -{'deviceroot': '', 'dirs': {}, 'repository': 'http://hg.mozilla.org/releases/mozilla-release', 'buildid': '20131205075310', 'results_log': 'pathtoresults_log', 'symbols_path': None, 'bcontroller_config': 'pathtobcontroller', 'host': '', 'browser_name': 'Firefox', 'sourcestamp': '39faf812aaec', 'remote': False, 'child_process': 'plugin-container', 'branch_name': '', 'browser_version': '26.0', 'extra_args': '', 'develop': True, 'preferences': {'browser.display.overlaynavbuttons': False, 'extensions.getAddons.get.url': 'http://127.0.0.1/extensions-dummy/repositoryGetURL', 'dom.max_chrome_script_run_time': 0, 'network.proxy.type': 1, 'extensions.update.background.url': 'http://127.0.0.1/extensions-dummy/updateBackgroundURL', 'network.proxy.http': 'localhost', 'plugins.update.url': 'http://127.0.0.1/plugins-dummy/updateCheckURL', 'dom.max_script_run_time': 0, 'extensions.update.enabled': False, 'browser.safebrowsing.keyURL': 'http://127.0.0.1/safebrowsing-dummy/newkey', 'media.navigator.permission.disabled': True, 'app.update.enabled': False, 'extensions.blocklist.url': 'http://127.0.0.1/extensions-dummy/blocklistURL', 'browser.EULA.override': True, 'extensions.checkCompatibility': False, 'talos.logfile': 'pathtofile', 'browser.safebrowsing.gethashURL': 'http://127.0.0.1/safebrowsing-dummy/gethash', 'extensions.hotfix.url': 'http://127.0.0.1/extensions-dummy/hotfixURL', 'dom.disable_window_move_resize': True, 'network.proxy.http_port': 80, 'browser.dom.window.dump.enabled': True, 'extensions.update.url': 'http://127.0.0.1/extensions-dummy/updateURL', 'browser.chrome.dynamictoolbar': False,  'browser.link.open_newwindow': 2, 'browser.cache.disk.smart_size.first_run': False, 'security.turn_off_all_security_so_that_viruses_can_take_over_this_computer': True, 'dom.disable_open_during_load': False, 'extensions.getAddons.search.browseURL': 'http://127.0.0.1/extensions-dummy/repositoryBrowseURL', 'browser.cache.disk.smart_size.enabled': False, 'extensions.getAddons.getWithPerformance.url': 'http://127.0.0.1/extensions-dummy/repositoryGetWithPerformanceURL', 'hangmonitor.timeout': 0, 'dom.send_after_paint_to_content': True, 'security.fileuri.strict_origin_policy': False, 'media.capturestream_hints.enabled': True, 'extensions.update.notifyUser': False, 'extensions.blocklist.enabled': False, 'browser.bookmarks.max_backups': 0, 'browser.shell.checkDefaultBrowser': False, 'media.peerconnection.enabled': True, 'dom.disable_window_flip': True, 'security.enable_java': False, 'browser.warnOnQuit': False, 'media.navigator.enabled': True, 'browser.safebrowsing.updateURL': 'http://127.0.0.1/safebrowsing-dummy/update', 'dom.allow_scripts_to_close_windows': True, 'extensions.webservice.discoverURL': 'http://127.0.0.1/extensions-dummy/discoveryURL'}, 'test_timeout': 1200, 'title': 'qm-pxp01', 'error_filename': 'pathtoerrorfile', 'webserver': 'localhost:15707', 'browser_path':ffox_path, 'port': 20701, 'browser_log': 'browser_output.txt', 'process': 'firefox.exe', 'xperf_path': 'C:/Program Files/Microsoft Windows Performance Toolkit/xperf.exe', 'extensions': ['pathtopageloader'], 'fennecIDs': '', 'env': {'NO_EM_RESTART': '1'}, 'init_url': 'http://localhost:15707/getInfo.html', 'browser_wait': 5}
> +{'deviceroot': '', 'dirs': {}, 'repository': 'http://hg.mozilla.org/releases/mozilla-release', 'buildid': '20131205075310', 'results_log': 'pathtoresults_log', 'symbols_path': None, 'bcontroller_config': 'pathtobcontroller', 'host': '', 'browser_name': 'Firefox', 'sourcestamp': '39faf812aaec', 'remote': False, 'child_process': 'plugin-container', 'branch_name': '', 'browser_version': '26.0', 'extra_args': '', 'develop': True, 'preferences': {'browser.display.overlaynavbuttons': False, 'extensions.getAddons.get.url': 'http://127.0.0.1/extensions-dummy/repositoryGetURL', 'dom.max_chrome_script_run_time': 0, 'network.proxy.type': 1, 'extensions.update.background.url': 'http://127.0.0.1/extensions-dummy/updateBackgroundURL', 'network.proxy.http': 'localhost', 'plugins.update.url': 'http://127.0.0.1/plugins-dummy/updateCheckURL', 'dom.max_script_run_time': 0, 'extensions.update.enabled': False, 'browser.safebrowsing.keyURL': 'http://127.0.0.1/safebrowsing-dummy/newkey', 'media.navigator.permission.disabled': True, 'app.update.disabledForTesting': True, 'extensions.blocklist.url': 'http://127.0.0.1/extensions-dummy/blocklistURL', 'browser.EULA.override': True, 'extensions.checkCompatibility': False, 'talos.logfile': 'pathtofile', 'browser.safebrowsing.gethashURL': 'http://127.0.0.1/safebrowsing-dummy/gethash', 'extensions.hotfix.url': 'http://127.0.0.1/extensions-dummy/hotfixURL', 'dom.disable_window_move_resize': True, 'network.proxy.http_port': 80, 'browser.dom.window.dump.enabled': True, 'extensions.update.url': 'http://127.0.0.1/extensions-dummy/updateURL', 'browser.chrome.dynamictoolbar': False,  'browser.link.open_newwindow': 2, 'browser.cache.disk.smart_size.first_run': False, 'security.turn_off_all_security_so_that_viruses_can_take_over_this_computer': True, 'dom.disable_open_during_load': False, 'extensions.getAddons.search.browseURL': 'http://127.0.0.1/extensions-dummy/repositoryBrowseURL', 'browser.cache.disk.smart_size.enabled': False, 'extensions.getAddons.getWithPerformance.url': 'http://127.0.0.1/extensions-dummy/repositoryGetWithPerformanceURL', 'hangmonitor.timeout': 0, 'dom.send_after_paint_to_content': True, 'security.fileuri.strict_origin_policy': False, 'media.capturestream_hints.enabled': True, 'extensions.update.notifyUser': False, 'extensions.blocklist.enabled': False, 'browser.bookmarks.max_backups': 0, 'browser.shell.checkDefaultBrowser': False, 'media.peerconnection.enabled': True, 'dom.disable_window_flip': True, 'security.enable_java': False, 'browser.warnOnQuit': False, 'media.navigator.enabled': True, 'browser.safebrowsing.updateURL': 'http://127.0.0.1/safebrowsing-dummy/update', 'dom.allow_scripts_to_close_windows': True, 'extensions.webservice.discoverURL': 'http://127.0.0.1/extensions-dummy/discoveryURL'}, 'test_timeout': 1200, 'title': 'qm-pxp01', 'error_filename': 'pathtoerrorfile', 'webserver': 'localhost:15707', 'browser_path':ffox_path, 'port': 20701, 'browser_log': 'browser_output.txt', 'process': 'firefox.exe', 'xperf_path': 'C:/Program Files/Microsoft Windows Performance Toolkit/xperf.exe', 'extensions': ['pathtopageloader'], 'fennecIDs': '', 'env': {'NO_EM_RESTART': '1'}, 'init_url': 'http://localhost:15707/getInfo.html', 'browser_wait': 5}

For testing/talos you should also request review from jmaher.

::: testing/tps/tps/testrunner.py:59
(Diff revision 1)
>          'MOZ_NO_REMOTE': '1',
>          'XPCOM_DEBUG_BREAK': 'warn',
>      }
>  
>      default_preferences = {
> -        'app.update.enabled': False,
> +        'app.update.disabledForTesting': True,

Please request review from whimboo on testing/tps.

::: toolkit/components/telemetry/tests/marionette/harness/telemetry_harness/testcase.py
(Diff revision 1)
> -    def toggle_update_pref(self):
> -        value = self.marionette.get_pref('app.update.enabled')
> -        self.marionette.enforce_gecko_prefs({'app.update.enabled': not value})

I’m not a peer of Telemetry, so I can’t review this.
Attachment #8979644 - Flags: review?(ato) → review-
Comment on attachment 8979645 [details]
Bug 1420514 - Replace app.update.enabled with app.update.disabledForTesting in enterprise policy tests

https://reviewboard.mozilla.org/r/245790/#review253064
Attachment #8979645 - Flags: review?(felipc) → review+
Comment on attachment 8979379 [details]
Bug 1420514 - Remove app.update.enabled from prefs files

https://reviewboard.mozilla.org/r/245542/#review253808

::: browser/app/profile/firefox.js
(Diff revision 3)
>  // user of the failure. User initiated update checks always notify the user of
>  // the failure.
>  pref("app.update.backgroundMaxErrors", 10);
>  
> -// Whether or not app updates are enabled
> -#ifdef MOZ_UPDATER

Please add a comment in bug 1427471 about this being removed.

::: mobile/android/app/mobile.js
(Diff revision 3)
>  pref("app.update.autodownload", "wifi");
>  pref("app.update.url.android", "https://aus5.mozilla.org/update/4/%PRODUCT%/%VERSION%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/%MOZ_VERSION%/update.xml");
>  
>  #ifdef MOZ_UPDATER
>  /* prefs used specifically for updating the app */
> -pref("app.update.enabled", false);

Android has its own update mechanism and it may resuse this pref so please check with an Android dev if it is ok to remove this.
https://dxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#29
Attachment #8979379 - Flags: review?(robert.strong.bugs) → review+
Note: Android also reused the MOZ_UPDATER define - bug 1152634
Attachment #8979644 - Flags: review?(chutten)
Adding :chutten to review toolkit/components/telemetry/tests/marionette/harness/telemetry_harness/testcase.py
Comment on attachment 8979644 [details]
Bug 1420514 - Replace app.update.enabled with app.update.disabledForTesting in the test harness

https://reviewboard.mozilla.org/r/245788/#review254130

r+ on toolkit/components/telemetry/tests/marionette/harness/telemetry_harness/testcase.py for removing dead code.
Attachment #8979644 - Flags: review?(chutten) → review+
Attachment #8979644 - Flags: review?(jmaher)
Attachment #8979644 - Flags: review?(hskupin)
@jmaher - Could you please review these files:
js/src/tests/user.js
testing/raptor/test/test_raptor.py
testing/talos/talos/unittests/test_talosconfig_browser_config.json

@whimboo - Could you please review testing/tps/tps/testrunner.py

Sorry that I did not break up that commit better. I'm afraid that if I broke it up now, I would lose the r+'s that I have already gotten.
Comment on attachment 8979644 [details]
Bug 1420514 - Replace app.update.enabled with app.update.disabledForTesting in the test harness

https://reviewboard.mozilla.org/r/245788/#review252864

> Where did this come from?  We removed this in the past because it
> got removed from Gecko.

As far as I know, this continues to be used to determine whether Firefox is currently running automated tests. See https://searchfox.org/mozilla-central/rev/5a744713370ec47969595e369fd5125f123e6d24/js/xpconnect/src/xpcpublic.h#642

> Why is this needed?

This is needed so that `Cu.isInAutomation` returns `true`, allowing us to properly determine if Firefox is currently running automated tests.
Comment on attachment 8979644 [details]
Bug 1420514 - Replace app.update.enabled with app.update.disabledForTesting in the test harness

https://reviewboard.mozilla.org/r/245788/#review254206

I believe you got all the proper harnesses.
Attachment #8979644 - Flags: review?(jmaher) → review+
So what are downstreams supposed to do now to avoid firefox from trying to upgrade itself when it's managed by a package manager? Set app.update.disabledForTesting?
No. `app.update.disabledForTesting` will only be available in automated testing. Frankly, I have very little understanding of how these downstreams operate, so I am not sure what is and isn't possible, but I will tell you what options they have.

I think that you are referring to *nix distributions, so please correct me if I am wrong, since this information will differ for Windows and macOS.

One option is to set `app.update.auto` to `false`. This would prevent automatic updates, but would prompt the user to let Firefox install any updates that could be downloaded and installed.

Another option is to set an enterprise policy that would prevent app update by putting a JSON file in the install directory. I don't recommend this for a number of reasons. It doesn't feel like the right way to solve this problem. It would also result in a banner in about:preferences saying something like "Some preferences are being controlled by your organization".

The thing is though, we typically don't update Linux installations that were installed via a package manager. I have been told that we have no real way of updating Firefox installations on Linux unless we can already write to the installation directory because we have no reliable, cross-platform way of elevating our privileges. Since the installation directory is typically writable only by root, but Firefox is not typically run by root, Firefox never updates.

I have no idea how package manager installations work, but if you wanted to formalize this, maybe you could just end the installation by removing all write permissions from the installation directory so that it could not update, even if run as root.

@mhowell - I haven't spent much time looking at update on Linux; could you confirm that what I have written about it here is correct?
Flags: needinfo?(mhowell)
(In reply to Kirk Steuber [:bytesized] from comment #44)
> No. `app.update.disabledForTesting` will only be available in automated
> testing. Frankly, I have very little understanding of how these downstreams
> operate, so I am not sure what is and isn't possible, but I will tell you
> what options they have.

What do you mean by "only available in automated testing"? For mozregression, I think we want this setting enabled under most circumstances (mozregression is typically downloading possible bisection candidates while the current build is being tested, and having Firefox pointlessly download an update for an obsolete build would interfere with that).
Comment on attachment 8979644 [details]
Bug 1420514 - Replace app.update.enabled with app.update.disabledForTesting in the test harness

https://reviewboard.mozilla.org/r/245788/#review254314

::: testing/geckodriver/src/prefs.rs:20
(Diff revision 2)
>          // Disable automatic downloading of new releases
>          ("app.update.auto", Pref::new(false)),
>  
>          // Disable automatically upgrading Firefox
> +        ("app.update.disabledForTesting", Pref::new(true)),
> +        ("security.turn_off_all_security_so_that_viruses_can_take_over_this_computer", Pref::new(true)),

This is still present since Andreas reviewed the piece of code and no explanation has been given yet.

::: testing/marionette/client/marionette_driver/geckoinstance.py:506
(Diff revision 2)
>  
>  class DesktopInstance(GeckoInstance):
>      desktop_prefs = {
>          # Disable application updates
> +        "app.update.disabledForTesting": True,
> +        "security.turn_off_all_security_so_that_viruses_can_take_over_this_computer": True,

Same here.

::: testing/mozbase/mozprofile/mozprofile/profile.py:435
(Diff revision 2)
>  class FirefoxProfile(Profile):
>      """Specialized Profile subclass for Firefox"""
>  
>      preferences = {  # Don't automatically update the application
> -        'app.update.enabled': False,
> +        'app.update.disabledForTesting': True,
> +        'security.turn_off_all_security_so_that_viruses_can_take_over_this_computer': True,

Same here.

::: testing/tps/tps/testrunner.py:60
(Diff revision 2)
>          'XPCOM_DEBUG_BREAK': 'warn',
>      }
>  
>      default_preferences = {
> -        'app.update.enabled': False,
> +        'app.update.disabledForTesting': True,
> +        'security.turn_off_all_security_so_that_viruses_can_take_over_this_computer': True,

Sorry, but I'm not a valid reviewer for TPS. Best is always to check the Bugzilla component who reviews patches at the moment. AFAIR you could ask Kit Cambridge.
Attachment #8979644 - Flags: review?(hskupin) → review-
(In reply to Mike Hommey [:glandium] from comment #43)
> So what are downstreams supposed to do now to avoid firefox from trying to
> upgrade itself when it's managed by a package manager? Set
> app.update.disabledForTesting?

In addition to Kirk's reply, distributions operating a package manager do not tend to package Mozilla builds, and the best way to handle this when using a custom build is to disable update support at build time, as is most often what is already done today.

(In reply to Kirk Steuber [:bytesized] from comment #44)
> @mhowell - I haven't spent much time looking at update on Linux; could you
> confirm that what I have written about it here is correct?

It's correct, to the extent that you can use the word "typical" to really describe anything covering all of Linux; I think it's more common for manually installed Linux builds to end up in a home directory, in which case they are writable and updates work fine.
Flags: needinfo?(mhowell)
(In reply to William Lachance (:wlach) (use needinfo!) from comment #45)
> What do you mean by "only available in automated testing"?
We determine if we are in automated testing based on whether the pref `security.turn_off_all_security_so_that_viruses_can_take_over_this_computer` is set to true and the environment variable "MOZ_DISABLE_NONLOCAL_CONNECTIONS" is set. This is existing functionality that is available via the `xpc::IsInAutomation` C++ function or the `Cu.isInAutomation` JS function.

See the implementation of `xpc::IsInAutomation`: https://searchfox.org/mozilla-central/rev/5a744713370ec47969595e369fd5125f123e6d24/js/xpconnect/src/xpcpublic.h#642
Attachment #8979644 - Flags: review?(ato) → review?(kit)
Comment on attachment 8979644 [details]
Bug 1420514 - Replace app.update.enabled with app.update.disabledForTesting in the test harness

K
Attachment #8979644 - Flags: review- → review?(ato)
Kit - If possible, could you please review testing/tps/tps/testrunner.py

Sorry that I did not break up that commit better. I'm afraid that if I broke it up now, I would lose the r+'s that I have already gotten.
Comment on attachment 8979644 [details]
Bug 1420514 - Replace app.update.enabled with app.update.disabledForTesting in the test harness

https://reviewboard.mozilla.org/r/245788/#review254484

Looks good! As far as TPS goes, review from a test or automation peer is totally sufficient for small tweaks like this, especially if they're part of a refactor that makes similar changes elsewhere...but, of course, you're also welcome to ask for review if you'd like. :-) Thanks!
Attachment #8979644 - Flags: review?(kit) → review+
(In reply to Kirk Steuber [:bytesized] from comment #48)
> is set to true and the environment variable
> "MOZ_DISABLE_NONLOCAL_CONNECTIONS" is set. This is existing functionality
> that is available via the `xpc::IsInAutomation` C++ function or the
> `Cu.isInAutomation` JS function.

Please note that this doesn't apply to any existing harness. Especially Marionette doesn't support this environment variable yet, and for firefox-ui-tests we specifically don't want to have it set because the tier-2 tests have to access remote content.
Comment on attachment 8979644 [details]
Bug 1420514 - Replace app.update.enabled with app.update.disabledForTesting in the test harness

https://reviewboard.mozilla.org/r/245788/#review254722

r+wc that you drop the virus pref from prefs.rs.
Attachment #8979644 - Flags: review?(ato) → review+
Comment on attachment 8979643 [details]
Bug 1420514 - Remove pref app.update.enabled from the update mechanism

https://reviewboard.mozilla.org/r/245786/#review256662

::: browser/base/content/aboutDialog-appUpdater.js:139
(Diff revision 2)
>      return this.um.activeUpdate &&
>             this.um.activeUpdate.state == "downloading";
>    },
>  
>    // true when updating is disabled by an administrator.
> -  get updateDisabledAndLocked() {
> +  get updateDisabledByAdmin() {

I'm ok with updateDisabledByAdmin for the name but perhaps updateDisabledByPolicy would be better. If it is changed it would be good to change the panel name from adminDisabled to policyDisabled.

::: browser/base/content/aboutDialog-appUpdater.js:145
(Diff revision 2)
> -           !Services.policies.isAllowed("appUpdate"));
>    },
>  
>    // true when updating is enabled.
>    get updateEnabled() {
> -    try {
> +    return !Services.policies || Services.policies.isAllowed("appUpdate");

Would be better to just combine updateEnabled with updateDisabledByAdmin or updateDisabledByPolicy if you change the name and call that instead.

::: browser/components/nsBrowserGlue.js:737
(Diff revision 2)
>    _checkForOldBuildUpdates() {
>      // check for update if our build is old
> +    let updateService = Cc["@mozilla.org/updates/update-service;1"].
> +                        getService(Ci.nsIApplicationUpdateService);
>      if (AppConstants.MOZ_UPDATER &&
> -        Services.prefs.getBoolPref("app.update.enabled") &&
> +        updateService.canCheckForUpdates &&

This is going to cause the update service to initialize on every startup. I think this check might not be necessary if nsUpdateService.js handles it properly which I think it does. This will also give us more data for this case in telemetry.

::: toolkit/mozapps/update/content/updates.js:568
(Diff revision 2)
> -
> -    if (Services.prefs.getBoolPref(PREF_APP_UPDATE_ENABLED, true))
> +    let updateService = Cc["@mozilla.org/updates/update-service;1"].
> +                        getService(Ci.nsIApplicationUpdateService);
> +    if (updateService.canCheckForUpdates)
>        document.getElementById("noUpdatesAutoEnabled").hidden = false;
>      else
>        document.getElementById("noUpdatesAutoDisabled").hidden = false;

When canCheckForUpdates is false this will display the following message which I don't think is approprite with this change since a user won't be able to easily enable update checking when it is disabled by policy.
There are no updates available. Please check again later or enable &brandShortName;'s automatic update checking.

Also, canCheckForUpdates has several other checks beyond just checking the pref.

I suspect you just want the policy check above vs. canCheckForUpdates and a new string for communicating this state.

::: toolkit/mozapps/update/nsIUpdateService.idl:278
(Diff revision 2)
>     * @param   force
>     *          Forces the checker to check for updates, regardless of the
>     *          current value of the user's update settings. This is used by
>     *          any piece of UI that offers the user the imperative option to
>     *          check for updates now, regardless of their update settings.
> -   *          force will not work if the system administrator has locked
> +   *          force will not work if updates are disabled by policy.

nit: please change the comment to something along the lines of:
Specifying true for the force param will not check for updates when updates are disabled by policy.

::: toolkit/mozapps/update/nsUpdateService.js:35
(Diff revision 2)
>  const PREF_APP_UPDATE_DOWNLOAD_MAXATTEMPTS = "app.update.download.maxAttempts";
>  const PREF_APP_UPDATE_ELEVATE_NEVER        = "app.update.elevate.never";
>  const PREF_APP_UPDATE_ELEVATE_VERSION      = "app.update.elevate.version";
>  const PREF_APP_UPDATE_ELEVATE_ATTEMPTS     = "app.update.elevate.attempts";
>  const PREF_APP_UPDATE_ELEVATE_MAXATTEMPTS  = "app.update.elevate.maxAttempts";
> -const PREF_APP_UPDATE_ENABLED              = "app.update.enabled";
> +const PREF_APP_UPDATE_DISABLED_FOR_TEST    = "app.update.disabledForTesting";

Are all of the cases where update checks shouldn't be performed in automation handled in the other patches?

::: toolkit/mozapps/update/nsUpdateService.js
(Diff revision 2)
>      // UPDATE_LAST_NOTIFY_INTERVAL_DAYS_NOTIFY
>      AUSTLMY.pingLastUpdateTime(this._pingSuffix);
>      // Histogram IDs:
> -    // UPDATE_NOT_PREF_UPDATE_ENABLED_EXTERNAL
> -    // UPDATE_NOT_PREF_UPDATE_ENABLED_NOTIFY
> -    AUSTLMY.pingBoolPref("UPDATE_NOT_PREF_UPDATE_ENABLED_" + this._pingSuffix,

UPDATE_NOT_PREF_UPDATE_ENABLED_etc. need to be removed or commented out in UpdateTelemetry.jsm

::: toolkit/mozapps/update/nsUpdateService.js:2064
(Diff revision 2)
>          AUSTLMY.pingCheckCode(this._pingSuffix, AUSTLMY.CHK_NO_OS_ABI);
>        } else if (!validUpdateURL) {
>          AUSTLMY.pingCheckCode(this._pingSuffix,
>                                AUSTLMY.CHK_INVALID_DEFAULT_URL);
> -      } else if (!Services.prefs.getBoolPref(PREF_APP_UPDATE_ENABLED, true)) {
> +      } else if (this.disabledByAdmin) {
>          AUSTLMY.pingCheckCode(this._pingSuffix, AUSTLMY.CHK_PREF_DISABLED);

Please create a new code instead of re-using CHK_PREF_DISABLED

::: toolkit/mozapps/update/nsUpdateService.js
(Diff revision 2)
>        } else if (!hasUpdateMutex()) {
>          AUSTLMY.pingCheckCode(this._pingSuffix, AUSTLMY.CHK_NO_MUTEX);
> -      } else if (!gCanCheckForUpdates) {
> +      } else if (!this.canCheckForUpdates) {
>          AUSTLMY.pingCheckCode(this._pingSuffix, AUSTLMY.CHK_UNABLE_TO_CHECK);
> -      } else if (!this.backgroundChecker._enabled) {
> -        AUSTLMY.pingCheckCode(this._pingSuffix, AUSTLMY.CHK_DISABLED_FOR_SESSION);

CHK_DISABLED_FOR_SESSION needs to be removed or commented out in UpdateTelemetry.jsm

::: toolkit/mozapps/update/nsUpdateService.js:2311
(Diff revision 2)
> +  get disabledForTesting() {
> +    return Cu.isInAutomation &&
> +           Services.prefs.getBoolPref(PREF_APP_UPDATE_DISABLED_FOR_TEST, false);
> +  },
> +
> +  get disabledByAdmin() {

As previously mentioned, I'm ok with disabledByAdmin for the name but perhaps disabledByPolicy would be better.

::: toolkit/mozapps/update/nsUpdateService.js:2965
(Diff revision 2)
>      if (!listener) {
>        throw Cr.NS_ERROR_NULL_POINTER;
>      }
>  
> -    if (!this.enabled && !force) {
> +    let UpdateServiceInstance = UpdateServiceFactory.createInstance();
> +    if (UpdateServiceInstance.disabledByAdmin ||

Please add a comment that canCheckForUpdates also calls disabledByAdmin.

I've spent a decent amount of time trying to recall why it doesn't return early when canCheckForUpdates is false and force is true. I am fairly certain it is so we show errors during a check performed via the UI. Please add a comment to this affect.
Attachment #8979643 - Flags: review?(robert.strong.bugs) → review-
Comment on attachment 8979643 [details]
Bug 1420514 - Remove pref app.update.enabled from the update mechanism

https://reviewboard.mozilla.org/r/245786/#review257364

::: toolkit/mozapps/update/nsUpdateService.js:35
(Diff revision 2)
>  const PREF_APP_UPDATE_DOWNLOAD_MAXATTEMPTS = "app.update.download.maxAttempts";
>  const PREF_APP_UPDATE_ELEVATE_NEVER        = "app.update.elevate.never";
>  const PREF_APP_UPDATE_ELEVATE_VERSION      = "app.update.elevate.version";
>  const PREF_APP_UPDATE_ELEVATE_ATTEMPTS     = "app.update.elevate.attempts";
>  const PREF_APP_UPDATE_ELEVATE_MAXATTEMPTS  = "app.update.elevate.maxAttempts";
> -const PREF_APP_UPDATE_ENABLED              = "app.update.enabled";
> +const PREF_APP_UPDATE_DISABLED_FOR_TEST    = "app.update.disabledForTesting";

nit: please name this pref
PREF_APP_UPDATE_DISABLEDFORTESTING
Comment on attachment 8979646 [details]
Bug 1420514 - Replace app.update.enabled with app.update.disabledForTesting in update tests

https://reviewboard.mozilla.org/r/245792/#review257366

::: toolkit/mozapps/update/tests/data/shared.js:20
(Diff revision 3)
>  const PREF_APP_UPDATE_CHANNEL                    = "app.update.channel";
>  const PREF_APP_UPDATE_DOORHANGER                 = "app.update.doorhanger";
>  const PREF_APP_UPDATE_DOWNLOADPROMPTATTEMPTS     = "app.update.download.attempts";
>  const PREF_APP_UPDATE_DOWNLOADPROMPTMAXATTEMPTS  = "app.update.download.maxAttempts";
>  const PREF_APP_UPDATE_DOWNLOADBACKGROUNDINTERVAL = "app.update.download.backgroundInterval";
> -const PREF_APP_UPDATE_ENABLED                    = "app.update.enabled";
> +const PREF_APP_UPDATE_DISABLED                   = "app.update.disabledForTesting";

Please name this pref PREF_APP_UPDATE_DISABLEDFORTESTING
Attachment #8979646 - Flags: review?(robert.strong.bugs) → review+
Comment on attachment 8979643 [details]
Bug 1420514 - Remove pref app.update.enabled from the update mechanism

https://reviewboard.mozilla.org/r/245786/#review256662

> When canCheckForUpdates is false this will display the following message which I don't think is approprite with this change since a user won't be able to easily enable update checking when it is disabled by policy.
> There are no updates available. Please check again later or enable &brandShortName;'s automatic update checking.
> 
> Also, canCheckForUpdates has several other checks beyond just checking the pref.
> 
> I suspect you just want the policy check above vs. canCheckForUpdates and a new string for communicating this state.

As discussed on IRC, this code should not actually run when app update is disabled by policy, and the "noUpdateAutoDisabled" message should really be disaplayed based on the value of `app.update.auto` rather than `app.update.enabled`. Therefore I will change this code to check that pref instead.

> Are all of the cases where update checks shouldn't be performed in automation handled in the other patches?

They are.

> UPDATE_NOT_PREF_UPDATE_ENABLED_etc. need to be removed or commented out in UpdateTelemetry.jsm

This is done in the subsequent patch titled "Remove telemetry probes tracking app.update.enabled"

> Please create a new code instead of re-using CHK_PREF_DISABLED

This is also done in the patch titled "Remove telemetry probes tracking app.update.enabled"

> CHK_DISABLED_FOR_SESSION needs to be removed or commented out in UpdateTelemetry.jsm

Also done in the patch titled "Remove telemetry probes tracking app.update.enabled"

> Please add a comment that canCheckForUpdates also calls disabledByAdmin.
> 
> I've spent a decent amount of time trying to recall why it doesn't return early when canCheckForUpdates is false and force is true. I am fairly certain it is so we show errors during a check performed via the UI. Please add a comment to this affect.

I wrote a slightly different comment documenting the reasoning behind why I wrote the code like that. Let me know if you don't like it.
Attachment #8979378 - Flags: review+ → review?(jaws)
Comment on attachment 8979643 [details]
Bug 1420514 - Remove pref app.update.enabled from the update mechanism

https://reviewboard.mozilla.org/r/245786/#review258906

I'm r+ing this since I'm ok with this for the most part. I'd like to get your take on throwing for disabledByPolicy in checkForUpdates.

::: toolkit/mozapps/update/content/updates.js:20
(Diff revision 3)
>  const XMLNS_XUL = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
>  
>  const PREF_APP_UPDATE_BACKGROUNDERRORS    = "app.update.backgroundErrors";
>  const PREF_APP_UPDATE_CERT_ERRORS         = "app.update.cert.errors";
>  const PREF_APP_UPDATE_ELEVATE_NEVER       = "app.update.elevate.never";
> -const PREF_APP_UPDATE_ENABLED             = "app.update.enabled";
> +const PREF_APP_UPDATE_AUTO                = "app.update.auto";

nit: these are sorted so please add it to the top.

::: toolkit/mozapps/update/nsUpdateService.js:2969
(Diff revision 3)
> +    // seem a bit odd, but it is conceptually correct. The |force| parameter
> +    // is specified when the user manually requested an update check, so it is
> +    // meant to override anything preventing update checks (including
> +    // user settings that might prevent update). However, enterprise policies
> +    // are meant to override the user's actions. Therefore, we must exit early
> +    // update is disabled by policy, but any other considerations can be

nit: I'd like this comment to be more concise. Also, "Therefore, we must exit early update is disabled by policy, but any other considerations can be overridden by |force|." I suspect you meant, we must exit early *when* update is disabled by policy, etc.

::: toolkit/mozapps/update/nsUpdateService.js:2971
(Diff revision 3)
> +    // meant to override anything preventing update checks (including
> +    // user settings that might prevent update). However, enterprise policies
> +    // are meant to override the user's actions. Therefore, we must exit early
> +    // update is disabled by policy, but any other considerations can be
> +    // overridden by |force|.
> +    if (UpdateServiceInstance.disabledByPolicy ||

What do you think of throwing when disabledByPolicy is true for foreground checks?

if (force && UpdateServiceInstance.disabledByPolicy) {
  throw etc...
} 

if (!force && !UpdateServiceInstance.canCheckForUpdates) {
  return;
}
Attachment #8979643 - Flags: review?(robert.strong.bugs) → review+
Comment on attachment 8979643 [details]
Bug 1420514 - Remove pref app.update.enabled from the update mechanism

https://reviewboard.mozilla.org/r/245786/#review258906

> What do you think of throwing when disabledByPolicy is true for foreground checks?
> 
> if (force && UpdateServiceInstance.disabledByPolicy) {
>   throw etc...
> } 
> 
> if (!force && !UpdateServiceInstance.canCheckForUpdates) {
>   return;
> }

That seems fine. I'll change it.
Attachment #8979378 - Flags: review?(jaws)
(In reply to Kirk Steuber [:bytesized] from comment #77)
> Comment on attachment 8979643 [details]
> Bug 1420514 - Remove pref app.update.enabled from the update mechanism
> 
> https://reviewboard.mozilla.org/r/245786/#review258906
> 
> > What do you think of throwing when disabledByPolicy is true for foreground checks?
> > 
> > if (force && UpdateServiceInstance.disabledByPolicy) {
> >   throw etc...
> > } 
> > 
> > if (!force && !UpdateServiceInstance.canCheckForUpdates) {
> >   return;
> > }
> 
> That seems fine. I'll change it.
Please add a LOG message as well.
Attachment #8979378 - Flags: review?(jaws)
Comment on attachment 8979378 [details]
Bug 1420514 - Remove the UI for app.update.enabled in about:preferences

https://reviewboard.mozilla.org/r/245540/#review263454

The moving of the code to init() that checks if the maintenance service is installed looks good. Thanks!
Attachment #8979378 - Flags: review?(jaws) → review+
Pushed by ksteuber@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d3399edf0197
Remove app.update.enabled from prefs files r=rstrong
https://hg.mozilla.org/integration/autoland/rev/de4a4592dbd9
Remove pref app.update.enabled from the update mechanism r=rstrong
https://hg.mozilla.org/integration/autoland/rev/416e398d942f
Remove telemetry probes tracking app.update.enabled r=chutten
https://hg.mozilla.org/integration/autoland/rev/5791b17ccb82
Remove the UI for app.update.enabled in about:preferences r=jaws
https://hg.mozilla.org/integration/autoland/rev/01bf643c77e5
Replace app.update.enabled with app.update.disabledForTesting in the test harness r=ato,chutten,jmaher,lina
https://hg.mozilla.org/integration/autoland/rev/48ed6dfe8772
Replace app.update.enabled with app.update.disabledForTesting in enterprise policy tests r=Felipe
https://hg.mozilla.org/integration/autoland/rev/5a46a221b6e8
Replace app.update.enabled with app.update.disabledForTesting in update tests r=rstrong
Working on fixing the failures. Last review push was purely a rebase update so that the interdiffs will display properly on the next patch push.
Flags: needinfo?(ksteuber)
Pushed by ksteuber@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3214e87404db
Remove app.update.enabled from prefs files r=rstrong
https://hg.mozilla.org/integration/autoland/rev/7e60005df7f9
Remove pref app.update.enabled from the update mechanism r=rstrong
https://hg.mozilla.org/integration/autoland/rev/ed6333e195db
Remove telemetry probes tracking app.update.enabled r=chutten
https://hg.mozilla.org/integration/autoland/rev/6226afd9d35b
Remove the UI for app.update.enabled in about:preferences r=jaws
https://hg.mozilla.org/integration/autoland/rev/7ca13add114d
Replace app.update.enabled with app.update.disabledForTesting in the test harness r=ato,chutten,jmaher,lina
https://hg.mozilla.org/integration/autoland/rev/4dd0d5daf420
Replace app.update.enabled with app.update.disabledForTesting in enterprise policy tests r=Felipe
https://hg.mozilla.org/integration/autoland/rev/97321f82dfbd
Replace app.update.enabled with app.update.disabledForTesting in update tests r=rstrong
I'm running multiple profiles in parallel on Nightly, and disabled updates in all but one, so things don't get funky.

Is abusing app.update.disabledForTesting the best way forward?
(In reply to Axel Hecht [:Pike] from comment #110)
> I'm running multiple profiles in parallel on Nightly, and disabled updates
> in all but one, so things don't get funky.
> 
> Is abusing app.update.disabledForTesting the best way forward?

No, that won't work, see comment 48. It seems like the closest thing to the old behaviour is to disable automatic applying of the updates, by setting app.update.auto to false (comment 44). This is what I'm planning to do in mozregression.
Release Note Request (optional, but appreciated)
[Why is this notable]: Feature less obvious
[Affects Firefox for Android]: No
[Suggested wording]: Change: Update options in about:preferences have been removed (available in the advanced preferences)
[Links (documentation, blog post, etc)]:

A better wording is welcome!
relnote-firefox: --- → ?
Release Note Request (optional, but appreciated)
[Why is this notable]: Feature less obvious
[Affects Firefox for Android]: No
[Suggested wording]: Change: "Never check for updates" option removed from about:preferences. If a substitute is needed, we recommend using the enterprise policy: DisableAppUpdate.
[Links (documentation, blog post, etc)]: Enterprise policy documentation: https://github.com/mozilla/policy-templates/blob/master/README.md
relnote-firefox: --- → ?
So the first two days have been quite annoying with just update notifications, as it's pestering me even more than my profile that's actually doing the update.

Added a couple of zeros to the update frequency in my non-updating profile now, let's see how that goes.
Added to nightly release notes
Do you know what will happen for people who disabled updates? It will be much harder for them to turn them back on?
Thanks
Flags: needinfo?(ksteuber)
Nightly now insists to remind constantly that there is a, well, nightly update available - is this an incentive to block all connections to Mozila with an app level firewall or the hosts file? 

The Nightly release notes say I'm to use the DisableAppUpdate policy, but it doesn't seem to work for me and the description says ESR only - is it enabled for Nightly, too? https://github.com/mozilla/policy-templates/blob/master/README.md
I'm trying not to get pestered by increasing app.update.interval and disabling app.update.doorhanger - if these are the prefered methods it would be helpful mention them prominently if the DisableAppUpdate policy really only works on ESR.
Depends on: 1479289
(In reply to Mozilla Marsu from comment #117)
> The Nightly release notes say I'm to use the DisableAppUpdate policy, but it
> doesn't seem to work for me and the description says ESR only - is it
> enabled for Nightly, too?
> https://github.com/mozilla/policy-templates/blob/master/README.md

According to bug 1460082, the DisableAppUpdate policy works with non-ESR now. Please make sure that you are using the latest Nightly. And the policy template is probably outdated. Please file an issue (or even send a PR) to <https://github.com/mozilla/policy-templates>.
(In reply to Sylvestre Ledru [:sylvestre] [PTO until 2018/08/12] from comment #116)
> Do you know what will happen for people who disabled updates? It will be
> much harder for them to turn them back on?
> Thanks

I think you misunderstand the change that has been made. I did not just remove an option that controls a pref; I removed the option AND the pref. People who changed this value via about:preferences or about:config will retain the value, but it will do nothing.
Flags: needinfo?(ksteuber)
(In reply to Masatoshi Kimura [:emk] from comment #119)
> (In reply to Mozilla Marsu from comment #117)
> > The Nightly release notes say I'm to use the DisableAppUpdate policy, but it
> > doesn't seem to work for me and the description says ESR only - is it
> > enabled for Nightly, too?
> > https://github.com/mozilla/policy-templates/blob/master/README.md
> 
> According to bug 1460082, the DisableAppUpdate policy works with non-ESR
> now. Please make sure that you are using the latest Nightly. And the policy
> template is probably outdated. Please file an issue (or even send a PR) to
> <https://github.com/mozilla/policy-templates>.

Thanks, that should cover it and I'll stop complaining :-p ... I'll check why it isn't working over here (I *am* at latest Nightly :-)), probybly b/c I'm running a PortableApps Nightly and/or I screwed up placing the policy.

I didn't manage a full PR, but at least a issue: https://github.com/mozilla/policy-templates/issues/209
The Nightly chanelog says "Never check for updates option removed from about:preferences. If a substitute is needed, we recommend using the DisableAppUpdate enterprise policy" https://www.mozilla.org/en-US/firefox/63.0a1/releasenotes/

But the policy ist *not* a "substitute" because it not only disables automatic update checks, but disabled *manual* checking, too! The About page says "Updates disabled by your system administrator" which is true to the ESR legacy.

This does not improve security for peope who opt out of update nagging, as now they are more prone to update even less often.

Suggestion: It would be better to allow for manual checking/updating on non-ESR versions in the About windows even if the DisableAppUpdate is active, or introduce a DisableAppUpdateCheck flavor.
(In reply to Kirk Steuber [:bytesized] from comment #120)
> (In reply to Sylvestre Ledru [:sylvestre] [PTO until 2018/08/12] from
> comment #116)
> > Do you know what will happen for people who disabled updates? It will be
> > much harder for them to turn them back on?
> > Thanks
> 
> I think you misunderstand the change that has been made. I did not just
> remove an option that controls a pref; I removed the option AND the pref.
> People who changed this value via about:preferences or about:config will
> retain the value, but it will do nothing.

What's the answer to comment 43? Is there a build-time option to disable app update as mentioned in comment 47?
Yes. Application update can be disabled at build time with the `--disable-updater` flag.
Blocks: 1503362
Firefox 63.0 just automatically updated to 63.0.1 with 'Check for updates but let you choose to install them' checkbox checked.
Not to mention getting no warning about Firefox 63.0 changing my 'Never check for updates' preference.
Flags: needinfo?(robert.strong.bugs)
What would it take to remove this change (Bug 1420514) from ALL future Firefox versions and restore functionality? I just spent hours tracking down the source of the missing settings, and created an account solely to ask this question because it really is that important.

As a personal user of Firefox I need control over when updates occur, with no nagging windows and ideally with an easy-to-set setting. As someone who helps administer the machines of many smart people, people using machines that CANNOT be allowed to close down or restart browsers at random times, this change has removed a simple way to accomplish that need. While setting policies is a partial band-aid, it keeps getting harder and harder to simply and reliably control Firefox as these changes and workarounds pile on top of one another.

The "best" reason for this change I've seen so far is that 99.9% of users don't change it. Given that 99.9% of people probably don't need the bottom 50 pixels of the browser page that hardly seems a good reason to remove them. In fact, almost every Firefox user I know (especially those who supported Mozilla all the way from the Phoenix and Firebird origins) switched because they appreciated that the designers did not hide settings and trusted users to change things as desired.

While the answer may be outside of what I can personally accomplish, I'll ask my original honest question again in case another person tracks down the same problem I did and can do more: What would it take to remove this change (Bug 1420514) from ALL future Firefox versions?
(In reply to tomh1097 from comment #127)
> and created an account solely to ask this question

Sadly in vain because bugzilla is not the place for it. Discussions and advocacy for or against features or changes belong in our forums: https://www.mozilla.org/about/forums/
For this topic probably dev-firefox unless I'm missing a more specific place for UI design.
> The "best" reason for this change I've seen so far is that 99.9% of users don't change it.

The best reason for this is the reason initially given - it makes it much more difficult to strand people, deliberately or accidentally, on insecure and outdated versions of Firefox. Because this can happen via either user action or malicious software, and has been allowed to stand for as long as it has, this has become a significant issue for the Firefox user population, and we need to address it.

The nature of the internet - in particularly, the nature of risk on the internet - has changed dramatically since the Phoenix and Firebird days when early design decisions like this one were first implemented. Today, we need to balance the freedom of our users to configure Firefox as they see fit with an equal right to participate in the world free from harassment, exploitation and the rest of the carnival of malfeasance that makes up the background noise of the modern internet.

If you need this sort of granular control over your update situation - control that Chrome doesn't offer at all, if I recall correctly - please consider Firefox ESR, or some of our enterprise policy tools. But this change isn't going to get reverted.
This change is _really_ annoying.
Now I get frequent popups-errors from Firefox that Firefox is unable to check for updates because our firewall do not permit installing non-approved software. Could you please either remove the popup error or put the "do not check at all" feature back on into Firefox?
You are arguing that there is a risk when not updating software, but you are forgetting that some people already update software using other, safer methods than through direct unsecure download.
Flags: needinfo?(robert.strong.bugs)
As reporter of the comment #0 (i know my message was a bit short).
Mike, in comment #129, explained why we are doing that from the security perspective.
There is also another reason why we did it. It is because the web is moving super fast. On some top websites (youtube, facebook, etc), the pace of the web increased significantly.

Because the option was super visible and the impact hard to understand for normal folks (ie people who have no clue about bugzilla), we had a bunch of feedback that "website xxx is slow" or "this website doesn't work with firefox" because these users were using an old version of Firefox because, a few years ago, they disabled update.

Also, as a release manager, one of the thing which keep me awake at night is antivirus and security software. Because these software don't test with old version of Firefox, they can break (startup crash, network issue, etc) our web browser. And these classes of issues are super hard to diagnose... 

Now, I hear your complaints. I will ask if we can have some documentation on that.
You can turn off updates using our policy support:

https://github.com/mozilla/policy-templates/blob/master/README.md

Create a directory called "distribution" in the same directory as your Firefox executable (Firefox.app/Content/Resources on Mac).

Create a file called policies.json in that directory and put the following in it:

{
  "policies": {
    "DisableAppUpdate": true
  }
}

And you won't get asked for updates.

If you're on Windows, you can also use GPO to do it.
Whiteboard: [ux feature] → [ux feature][To know how to disable updates, please see comment #c137]
Comment#137 doesn't address IT departments that control updates.

But do check out the link as there are a lot of other things you can do with policy.  

Unfortunately, you can't control disk cache behavior.
> Comment#137 doesn't address IT departments that control updates.

Can you be more specific with what you are asking for?

IT departments can turn off updates and then send their own updates via other mechanisms.

IT Departments can setup their own update server via policy.

IT Departments can use GPO or Mac OS configuration profiles to enable/disable updates.

> Unfortunately, you can't control disk cache behavior.

It hasn't been requested. Feel free to open an issue there.
I thought it was obvious, since I am not in that situation, but have no problem imagining IT departments that can't be bothered to do anything with the users ending up paying the price.  

As well it is hinted at in Comment#130 and explicitly stated in Comment#135(which does not seem to be advocacy).

Restricting the comments to editbug permissions.
See comment #137 for the option.

Restrict Comments: true

Just a reminder: Bugzilla is not the correct place to advocate for or against features.

Those discussions belong in our forums: https://www.mozilla.org/about/forums/

You need to log in before you can comment on or make changes to this bug.