Gavin asked in bug 435325 comment 37 to remove the browser.offline pref: > I don't think core code should be touching this pref. It's Firefox-specific > AFAICT. Our use of browser.offline is bogus and I think we should get rid of > it, but that should happen in a followup bug. That pref currently sets the initial state to offline after startup, if the user had selected "Work Offline" in the previous session. Removing that pref means that Firefox will always start online, unless the NetworkManager sets it to offline (if the network.manage-offline-status pref is true; it had been disabled in bug 620472). This fixes bug 636148 (Upgrading to 4.0 from 3.6 with offline state auto detected to true leaves the user offline since 4.0 no longer manages offline state). http://mxr.mozilla.org/mozilla-central/search?string=%22browser.offline%22&tree=mozilla-central

Simple code removal, shouldn't take long to review.. The comment in nsDBusService.h is obviously referring to nsBrowserGlue.js; dbusservice doesn't touch the pref itself. I'll file a follow-up on mozilla/extensions/irc/xul/content/static.js.

I may have been hasty in suggesting we just rip it out - we could also just fix _updateOfflineUI to also set the pref so that it reflects the actual offline state at all times (so it would effectively just be a way to ensure that the offline state is persisted across restarts). On the flip side, we didn't really support that kind of persistence when automatic link detection was enabled prior to Firefox 4, AFAICT (unless you manually disabled link detection), and I hadn't heard of anyone being too upset about that, so maybe we can just kill it.

Comment on attachment 538703 [details] [diff] [review] patch OK, let's just do this. The changes to xpinstall/tests/browser_offline.js conflict with bug 652376 - best to just omit them and save Mossop the merging trouble, I think.

Comment on attachment 538703 [details] [diff] [review] patch Review of attachment 538703 [details] [diff] [review]: ----------------------------------------------------------------- There are still a few appearances of "browser.offline" that should be removed: https://mxr.mozilla.org/mozilla-central/search?string=browser.offline ::: browser/components/nsBrowserGlue.js @@ -358,5 @@ > - try { > - Services.io.offline = Services.prefs.getBoolPref("browser.offline"); > - } > - catch (e) { > - Services.io.offline = false; Do you make sure that, after removing this, Services.io.offline is still correctly set to false?

Comment on attachment 538703 [details] [diff] [review] patch Review of attachment 538703 [details] [diff] [review]: ----------------------------------------------------------------- > There are still a few appearances of "browser.offline" that should be > removed: > https://mxr.mozilla.org/mozilla-central/search?string=browser.offline That search also returns "browser.offline-apps.notify" and "services.sync.prefs.sync.browser.offline-apps.notify", which are unrelated. https://mxr.mozilla.org/mozilla-central/source/js/src/tests/e4x/Regress/regress-308111.js#591 is not worth changing, since it's a regression test "Do not crash when searching large e4x tree" which doesn't read or write prefs, but just uses the names; could replace that by "foo.bar" as well. See bug 636148 comment 17: "There's no reason to keep this file in sync with actual prefs." I used http://mxr.mozilla.org/mozilla-central/search?string=%22browser.offline%22&tree=mozilla-central and covered all of those. ::: browser/components/nsBrowserGlue.js @@ -358,5 @@ > - try { > - Services.io.offline = Services.prefs.getBoolPref("browser.offline"); > - } > - catch (e) { > - Services.io.offline = false; I don't, since not setting it is the point of this bug: don't remember a user-selected offline state from the previous session. See comment 0: "Removing that pref means that Firefox will always start online, unless the NetworkManager sets it to offline (if the network.manage-offline-status pref is true; it had been disabled in bug 620472)." But maybe I'm misunderstanding you?

Comment on attachment 538703 [details] [diff] [review] patch (In reply to comment #5) No, I meant, to put it more concisely: Do we still need the line: Services.io.offline = false; or is it already set to false initially? It turns out that it's true initially, but when manage-office-status is false, my patch for bug 627332 (on which bug 620472 depended) ensures that it gets set to false, so you're fine! :) (the line I was looking for: https://mxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsIOService.cpp#275 )

Changes to xpinstall/tests/browser_offline.js removed. http://hg.mozilla.org/integration/mozilla-inbound/rev/e3797dc14be1

Mozilla/5.0 (X11; Linux i686; rv:7.0a1) Gecko/20110628 Firefox/7.0a1 Preference no longer present on builds from WinXP, Win7, Ubuntu 11.04 and MAC OS X 10.6. Setting status to Verified Fixed.