229763 - "Work offline" checkbox does not set browser to offline mode

Status: VERIFIED FIXED

(SeaMonkey :: Startup & Profiles, defect)

Description

[benc]

WFM - Mac OS X, Mozilla 1.6b
Fails - Win2k, Mozillz 1.6b

STEPS:
Launch w/ profile manager.
Select a profile.
Select "work offline"
Start session.

OBSERVED:
The browser is online. network.online is set to true in about:cofnig.

EXPECTED:
The browser should be offline (see offline/online button in lower right).

Comment 1

[Kenneth Herron]

I can reproduce this on linux. I'm running a GTK CVS build with ID 2003123011 on
Mandrake 9.0. 

From the profile manager, I selected a test profile which has mostly default
settings, then checked "work offline" and clicked "start mozilla". When the
browser window opened, the plug widget (bottom right of the window) was in
"online" mode. I could browse without trouble. The plug widget worked as
expected to go online and offline. I restarted mozilla a couple of times and the
"work offline" setting was ignored every time.

Comment 2

[Bjarne D Mathiesen]

hardware/os should be changed to all/all
also seen with 1.6 release on Mac OS X 10.3.2
Moz 1.6b wasn't wfm for me under Mac OS X 10.3.2

actually, the off-line is semi-respected, because if you've set Moz to load a
home-page this isn't loaded when you select the off-line setting.

Comment 3

[HJ]

>Re Comment 2
>hardware/os should be changed to all/all

Done

>actually, the off-line is semi-respected, because if you've set Moz to load a
>home-page this isn't loaded when you select the off-line setting.

Not when you set "autoadmin.failover_to_cached" to true, as long as the page(s)
is(are) cached. 

Comment 4

[HJ]

Attachment: diff -u

This patch works for me.

Comment 5

[HJ]

Hmm 'network.online' will only be saved when 'offline.startup_state' is set to
0. My patch overrules this behavior, because that's how I want it to work but
this could break the mozilla specs. Btw, is there a spec written, or any
documentation at all for this feature?

Comment 6

[[e-mail]]

David, you implemented the network.offline pref, right?

Comment 7

[David :Bienvenu]

what I did was a mailnews extension,
mailnews/extensions/offline-startup/js/offlineStartup.js - if this is a browser
only product, you won't have my extension. I didn't think browser-only products
really cared about offline since we don't have much offline browsing support,
but it appears I was mistaken.

All the mailnews extension does is check, at startup, "offline.startup_state",
which has the following values:
var gOfflineStartupMode; //0 = remember last state, 1 = ask me

I was going to add 2 == online, 3 == offline, but I never got around to it.

The remember last state works by using the "network.online" value when the
online state changes, and restoring that state at startup.

Comment 8

[HJ]

Attachment: Modified version of offlineStartup.js

Modified version of offlineStartup.js (take 2) I just missed something :(

Comment 9

[HJ]

David, do you want me to add 2 and 3 to offlineStartup.js ?

Comment 10

[HJ]

Hmm, history window has no 'pref' but uses 'gPrefService'. Here a small change
to prevent the JS error:

-  pref.setBoolPref("network.online", !ioService.offline);
+  Components.classes["@mozilla.org/preferences-service;1"]
+            .getService(Components.interfaces.nsIPrefBranch)
+            .setBoolPref("network.online", !ioService.offline);

Comment 11

[benc]

Wait, before we implement more stuff, lets make sure we understand what is
implemented.

David: are you saying that mailnews has a separate offline-online state than the
browser, and that it does not always follow the network.online pref? I'm asking
because of bug 205044.

Comment 12

[HJ]

(In reply to comment #11)
> Wait, before we implement more stuff, lets make sure we understand what is
> implemented.
> 
> David: are you saying that mailnews has a separate offline-online state than the
> browser, and that it does not always follow the network.online pref? I'm asking
> because of bug 205044.

Yes, just remove /mozilla/components/offLineStartup.js and network.online should
work like expected. Also, my proposed patch will also fix bug 205044 and bug 229307

Comment 13

[David :Bienvenu]

No, mailnews has the same network.online state, and respects it. I'm saying the
extension is only in mailnews, and the extension can change the state on
startup, if it is configured to do so.

Comment 14

[baffclan]

I reported bug 235280.
It is "Work offline" checkbox set browser to online mode.

Comment 15

[Mike Cowperthwaite]

*** Bug 235280 has been marked as a duplicate of this bug. ***

Comment 16

[benc]

UPDATE:
Mozilla 1.6f, fails only in Mac.

I'm really confused about why the behavior has changed.

Comment 17

[Olivier Vit (just a reporter)]

reproduced on win xp build 1.7b 2004041310
looks pretty severe to me
blocking ?

starting mailnews only by default

Comment 18

[Olivier Vit (just a reporter)]

on build 2004042409 (post rc1) win2k
still having the issue:
-selecting to work off line isn't honored
-selecting to work online isn't either
-but restores the previous state

additionnally, the choice isn't proposed any more when switching profile, but
this could make sense

Comment 19

[Olivier Vit (just a reporter)]

still an issue in build 20040513 1.7 on winxp/win2k

to be really assigned to someone, since blocking 1.7 ?

Comment 20

[Olivier Vit (just a reporter)]

reproduced on build 20040517 winxp

this seems really only related to profile manager integration, since the
preference settings in Mail & Newsgroups / Offline & disk space are properly
honored (remembers or asks on startup for example)

Comment 21

[kirun]

*** Bug 238346 has been marked as a duplicate of this bug. ***

Comment 22

[Olivier Vit (just a reporter)]

still in build Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7)
Gecko/20040526

and granted as blocking for 1.7 !

Comment 23

[David :Bienvenu]

I can't tell from the attached file what actually fixed the bug - there seem to
be a lot of spurious changes - what's the actual fix?

And what code does http://bugzilla.mozilla.org/show_bug.cgi?id=229763#c10 apply to?

Comment 24

[Asa Dotzler [:asa]]

HJ, can you please attach a diff with a description of the fix? Time is getting
very short for 1.7 and if something doesn't happen here soon, this isn't going
to make it.

Comment 25

[(not reading, please use [e-mail] instead)]

from my tests on trunk / 1.7 branch, the "work offline" checkbox doesn't seem to
be working for mail.

Comment 26

[David :Bienvenu]

Attachment: minimal fix

OK, this is the minimal fix - what we decided to do (and what I think the
original attachment does, but it's hard to say for sure), is if the profile mgr
puts us offline, we'll use that setting, and remember it. Otherwise, we use the
last setting, whatever it was. If there's no profile mgr, we'll use the last
setting, or if the profile mgr left us online, we'll use the last setting.

Comment 27

[(not reading, please use [e-mail] instead)]

I tested david's fix, and it works.  thanks david!

r/sr/a=sspitzer for trunk and 1.7 final.

some blathering:

1)  as david mentioned, the checkbox is really "force the user offline".
and not checking it doesn't mean "force the user online" it means "defer to the
per profile offline state", which is something that is currently a mailnews
extension.  it will apply to the browser if you have mailnews installed (even if
you don't use mailnews), but it won't work for nav only builds (of you don't
install the mail.xpi at install time). 

so if you have mail installed, and you don't check that checkbox, you might
still go offline, if that was the last state (and in your prefs you choose to
persist that)

a) in the future, should we move the offline pref out of mailnews and make it a
general pref?   (but there are still some mailnews specific offline settings
that should remain?)

b) in the future, should the checkbox label indicate that we mean "force the
user offline" or should we provide a useful tooltip or something?  or should it
match the state you are going to see, if you hit "start".  (sniff the per
profile pref for offline and check if we have a network connection?  I think
david mention that darin wants to do the latter at some point)

2)  I keep forgetting that the profile manager UI is only seen if you have
multiple profiles, or if you force it to show with -ProfileManager.  so we can't
rely on the check box for everything.

3)  I've removed the ! and reversed the if / else clauses, and elaborated on
your comments.  is this any better?

+      // if the user checked "work offline" in the profile mgr UI
+      // and forced us offline, remember that in prefs
+      // if checked, the "work offline" checkbox overrides our 
+      // persisted state
+      if (ioService.offline)
+        prefs.setBoolPref("network.online", false);
       else
       {
         // if user did not check "work offline" in the profile manager UI
         // use the persisted online state pref to restore our offline state
         ioService.offline = offline;
       }

Comment 28

[(not reading, please use [e-mail] instead)]

Comment on attachment 149700 [details] [diff] [review]
minimal fix

r/sr/a=sspitzer for trunk and 1.7

for the trunk, consider elaborating the comments.

Comment 29

[David :Bienvenu]

Attachment: seth's version of fix, which is what I'll checkin.

Comment 30

[Scott MacGregor]

fixed on the aviary 1.0 branch too.

Comment 31

[baffclan]

Can not confirm a fixing with 2004060210-trunk/WinXP.

1. Start mozilla, set a online at on/offline icon in status bar.
2. Exit mozilla.
3. Start mozilla with Profile Manager, NO checked Work offline.
   (mozilla is online mode)
4. Set a offline at on/offline icon in status bar.
5. Exit mozilla.
6. Start mozilla with Profile Manager, NO checked Work offline.
7. Mozilla started offline mode.

Comment 32

[David :Bienvenu]

that's the way it works now. We only allow the "work offline" checkbox to
override the remember last state setting. If you don't check the work offline
checkbox in the profile manager, we use the last state you were in...as I said
earlier. For 1.8, perhaps we can add a pref to the offline startup options, do
nothing, i.e., use profile mgr setting. But, remember, most users don't have the
profile manager at all.

Comment 33

[Tracy Walker [:tracy]]

verified with builds from 0603 
offline mode is set able to be set from the Profile Manager.  
and yes, the state remains that way, even after relaunch, until changed in the app.

Comment 34

[Olivier Vit (just a reporter)]

related to discussion in #31 and "32:
I also feel this pretty surprising, to have the checkbox not checked when you
select a profile which state is remembered as offline

I would look more consistent and predictable to update the status of the offline
checkbox when the profile is selected

it would also allow to decide, when starting with profile manager, if the
offline remembered state of the profile is the expected one or not, and change
it if desired

what do you think ? too late ? too complicated ? easy ?
in build rv:1.7) Gecko/20040606)

Comment 35

[David :Bienvenu]

>what do you think ? too late ? too complicated ? easy ?

Not possible, really. We can't load the profile to get the pref setting at this
point (too early), and it's hard to grovel through the prefs.js by hand.

Comment 36

[benc]

Tracy: is this verified for trunk or branch?

Oliver: The expected behavior you wanted is in bug 229307.

I would like to keep this bug focused on fixing the basic consistency problem of
the check box controlling the offline state of the profile when opened.

Comment 37

[Olivier Vit (just a reporter)]

(In reply to comment #36)
> I would like to keep this bug focused on fixing the basic consistency problem of
> the check box controlling the offline state of the profile when opened.

I understand the need for short term focus, and release

Another suggestion (much easier): change the label to "force to offline mode"
(or another wording) to make it explicit that it is a way to override, not a way
to know in which mode the profile will be opened
what do you think ?
(could lead to have a "force to online mode" checkbox also)

It is always better to describe properly the designed behavior, than to let
people think there is a bug under the hood

Comment 38

[Wayne Mery (:wsmwk)]

is bug 239479 gone given this bug is gone and is marked OS=ALL?

Comment 39

[Magnus Melin [:mkmelin]]

*** Bug 239479 has been marked as a duplicate of this bug. ***