Starting Firefox with Wifi offline causes stays in offline, and no visual indication of offline mode (regression from turning off manage network offline state)

RESOLVED FIXED in mozilla13

Status

()

Core
Networking
--
major
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: standard8, Assigned: mayhemer)

Tracking

(Blocks: 1 bug, {regression})

unspecified
mozilla13
x86
Mac OS X
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
In bug 620472, network.manage-offline-status was turned to false.

I don't know why I haven't see this before (I probably haven't done it recently), but I just started Firefox in offline mode, on starting up, I then refreshed some of the tabs that were open. However there was zero indication of Firefox being in offline mode. It took me quite a while (and turning on the web console) to realise that Firefox was offline.

Build id: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0) Gecko/20100101 Firefox/10.0

Quick STR:

1) Have your WiFi connection not connected
2) Start Firefox
3) Connect your WiFi
3) Try to use your existing tabs in Firefox, or load sites you visit frequently.

Actual Results:

Panic because nothing is re-loading, but non-existing tabs load sites from the cache, causing more confusion.

Expected Results:

Firefox should have detected the presence of WiFi.


Comments in Bug 620472 indicate that more bugs should have been filed to turn manage offline state back on after fixing issues. This doesn't appear to have happened - should it?

Maybe this is just bad for me because I didn't realise that Firefox was no longer managing my offline state properly and I didn't expect it to start in offline mode. Thunderbird which still has that pref set to true and manages my online/offline state just fine. So now my Firefox pref is set to true, but this is really bad UX for those just starting up and looking at pages they do regularly.

Updated

5 years ago
Blocks: 620472
No longer depends on: 620472
Having "manage offline state" disabled means that Firefox should never start up in offline mode, AIUI (at least since bug 663253 was fixed). Are you sure that's what's happening? If so, is there any chance you can debug to figure out why? I can't reproduce this...
(Reporter)

Comment 2

5 years ago
I reconfirmed today on the Mac, and it is definitely happening, taking a look at the code I think I can see an issue, need to debug a bit.
(Reporter)

Comment 3

5 years ago
Created attachment 592635 [details] [diff] [review]
Possible fix

The basic issue is that the initialization routines are causing the network status to be set before we've set the manage offline status. So if we're not managing offline (which the member variable defaults to true), then we initialize the network link service, which sees we're offline and then by the time we set the manage offline status, we're then not in a position where we can put ourselves back out of offline mode.

This patch would fix that, but the only case I'm not really happy with is where we are managed, and we're starting in offline mode. At the moment, that does temporarily put us online until the profile fully kicks in, which seems sub-optimal.

This also seems like it is all kicking in on app-startup or earlier, which is possibly good, but kinda strange that we would need the ioservice that early.

I think this could also gain a couple of unit tests, just by faking the network link services.

I won't have time to work on this for a while though.
(Reporter)

Updated

5 years ago
Component: General → Networking
Product: Firefox → Core
QA Contact: general → networking

Comment 4

5 years ago
Isn't this a DUPE of bug 578239?
(Reporter)

Comment 5

5 years ago
(In reply to RNicoletto from comment #4)
> Isn't this a DUPE of bug 578239?

Yes, but I didn't find that when I filed this, so I'll dupe forward in this instance as this one has more information (and a partial patch).
(Reporter)

Updated

5 years ago
Duplicate of this bug: 578239
(Assignee)

Comment 7

5 years ago
This could be a regression from bug 627672.

I can try to finish this.
Assignee: nobody → honzab.moz
Blocks: 627672
Status: NEW → ASSIGNED
(Assignee)

Comment 8

5 years ago
I broke this via a review suggestion:

(In reply to Honza Bambas (:mayhemer) from bug 627672 comment #59)
> Comment on bug 627672 attachment 566814 [details] [diff] [review]
> v7

> @@ +1116,2 @@
> >      if (mManageOfflineStatus && !wasManaged)
> >          return TrackNetworkLinkStatusForOffline();
> 
> This code is wrong IMO.  I still think the attribute should throw when we
> are not able to turn manage on (i.e. we don't have the link service) and are
> before profile-do-change.  
> 
> To fix the code as I think is correct would be to do mManageOfflineStatus =
> aManage; *after* call to InitializeNetworkLinkService().  This way we will
> throw when there is no link service.


The logic really is complicated.

Mark, the patch is mostly correct, thanks for the investigation!  

However, it needs some updates:

* the default value of mMangeOfflineStatus needs to be changed from true to false.  When we call SetManageOfflineStatus(true) the first time, we must call to TrackNetworkLinkStatusForOffline() - it won't happen when mMangeOfflineStatus is initially true - to determine we are actually able to track the state.  If not, reset mMangeOfflineStatus to false and throw.  (It's for case users switch the pref to true).

* have an explanation comment to stop anyone else to break this again:
  "SetManageOfflineStatus must throw when we fail to go from non-managed to managed.  Usually because there is no link monitoring service available.  Failure to do this switch is detected by a failure of TrackNetworkLinkStatusForOffline().  When there is no network link available during call to InitializeNetworkLinkService(), application is put to offline mode.  And when we change mMangeOfflineStatus to false on the next line we get stuck on being offline even though the link becomes later available."

Also see http://hg.mozilla.org/mozilla-central/annotate/343ec916dfd5/netwerk/base/src/nsIOService.cpp#l317

Mark, I will update your patch with it and let someone from the net team review, OK?
(Assignee)

Comment 9

5 years ago
Created attachment 602408 [details] [diff] [review]
v1

Christian, please check comment 8 for more explanation on this fix.
Attachment #602408 - Flags: review?(cbiesinger)
Attachment #602408 - Flags: review?(cbiesinger) → review+
(Assignee)

Comment 10

5 years ago
Comment on attachment 602408 [details] [diff] [review]
v1

https://hg.mozilla.org/integration/mozilla-inbound/rev/08aafcd6eb9c
Attachment #602408 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/08aafcd6eb9c
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
This patch breaks the online/offline events on Firefox Mobile. I don't know offhand if Firefox desktop is broken too. Backing out this patch makes the events work again.
Depends on: 729951
Instead of just backing this patch out, can we start trying to figure out why the offline/online events are broken by this?

Comment 14

5 years ago
Sure. Btw. Which version of Firefox Mobile is affected, can you reproduce it with latest nightly? And which is the best version to test?
Firefox 13+, though trunk is best to work against.

Comment 16

5 years ago
With Firefox 15 and Fedora 17, I still see a problem with the offline status of Firefox not being updated according to the status reported by NetworkManager.

I suggest the following patch

The idea is to change the mManageOfflineStatus variable only when the user toggles the network.manage-offline-status setting in about:config.

Specifically I think we should not set it to off in nsIOService::InitializeNetworkLinkService() when the mNetworkLinkService is not initialized yet, because 1/ there's no way for the user to know that the value has changed (network.manage-offline-status in about:config doesn't reflext the modification in that case), and 2/ there's no possibility for the variable to come back to the value true, except if the user manually toggles twice the network.manage-offline-status setting.

Similarly in the firefox interface (browser.js). If mManageOfflineStatus is on, and network is offline, firefox status will be offline too. If the user want to access some URL locally (http://localhost/...), he will set firefox online. Later, when the network on his box is back online, it seems natural that firefox status still follows the box connectivity status, which will be possible only if mManageOfflineStatus stays on.

Comment 17

5 years ago
Created attachment 661319 [details] [diff] [review]
limit places where mManageOfflineStatus is modified
(Reporter)

Comment 18

5 years ago
Fabrice, thanks for the possible patch, I suggest though that as this bug is fixed you file a fresh bug and attach the patch there - it has generally been found that when comments have been left on closed/resolved bugs, then they are less likely to get followed up and driven through to completion. Thanks.

Comment 19

5 years ago
done, thanks (bz#791626)
You need to log in before you can comment on or make changes to this bug.