Closed Bug 627332 Opened 9 years ago Closed 9 years ago

add-ons compatibility check always presents "Minefield is working offline" dialog when network link state detection is disabled

Categories

(Core :: Networking, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla2.0b11

People

(Reporter: whimboo, Assigned: fryn)

References

Details

Attachments

(1 file, 1 obsolete file)

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b10pre) Gecko/20110119 Firefox/4.0b10pre ID:20110119030331

With the patch landed on bug 620472, we now always display the "Minefield is working offline" on the first start after switching from Firefox 3.6.

Steps:
1. Create a profile with Firefox 3.6 (make sure you are offline)
2. Use the latest Minefield build with this profile

In step 2 you will see the offline dialog. That's wrong because the online mode worked well in step 1.

Asking for blocking because it's a strange experience for the major upgrade.
blocking2.0: --- → ?
(In reply to comment #0)
> Steps:
> 1. Create a profile with Firefox 3.6 (make sure you are offline)
> 2. Use the latest Minefield build with this profile
> 
> In step 2 you will see the offline dialog. That's wrong because the online mode
> worked well in step 1.

What do you mean by "make sure you are offline"?
Does this mean to check the box "Work Offline" in the profile manager, turn off AirPort, or something else?
Wow, that's not what I wanted to write. Too much offline/online words here. Sorry.

So here the steps again:
1. Create a profile with Firefox 3.6 (don't go offline)
2. Use the latest Minefield build with this profile
(In reply to comment #2)
> So here the steps again:
> 1. Create a profile with Firefox 3.6 (don't go offline)
> 2. Use the latest Minefield build with this profile

I can't reproduce this.
I've tried the following steps:

1. Open Firefox 3.6 Profile Manager (.../firefox-bin -p).
2. Create a new profile named 'test'.
3. Select the profile 'test'.
4. Click 'Start Firefox'.
5. After Firefox browser window finishes loading, quit Firefox.
6. Open Minefield (m-c 20100120) Profile Manager (.../firefox-bin -p).
7. Select the profile 'test'.
8. I don't even see the updater wizard, and Minefield opens immediately.

1. Open Firefox 3.6 Profile Manager (.../firefox-bin -p).
2. Create a new profile named 'test2'.
3. Click 'Quit'.
4. Open Minefield (m-c 20100120) Profile Manager (.../firefox-bin -p).
5. Select the profile 'test2'.
6. I don't even see the updater wizard, and Minefield opens immediately.
Ok, you will have to install an extension first. It's definitely related to the add-ons compatibility check. Without it the offline dialog doesn't pop-up.
Summary: Upgrading to Firefox 4 always presents "Minefield is working offline" dialog → Upgrading to Firefox 4 the add-ons compatibility check always presents "Minefield is working offline" dialog
(In reply to comment #4)
> Ok, you will have to install an extension first. It's definitely related to the
> add-ons compatibility check. Without it the offline dialog doesn't pop-up.

Alright, now I can reproduce this. Looks like a hard blocker to me. I have a possible fix and am waiting for it to compile, so I can test it.
Bug 627601 has blocking-betaN+ and was duped to this bug.
Could a driver set the flag here?
Assignee: nobody → fryn
Whiteboard: [hardblocker][beta10 blocker]
blocking2.0: ? → betaN+
This needs to block beta 10 which code freezes tomorrow at 2. If there isn't a quick fix then we'll just back out bug 620472
Component: General → Networking
Product: Firefox → Core
QA Contact: general → networking
Attached patch patch (obsolete) — Splinter Review
While fixing the bug, the patch retains the behavior that if you quit Firefox with "Work Offline" manually checked to true, it will persist that state upon the next startup.
Attachment #505675 - Flags: review?(roc)
Henrik, I just pushed this to tryserver, so builds will be up in a few hours. I've tested the patch locally, and it works for me, but could you verify that the bug is fixed with the patch?
Same patch as above, except without the part that disables auto-detection for all Gecko applications by default, since Mossop recommended that "Yeah if it is really unreliable then we probably should do that at some point, but not after api freeze ;)".
Attachment #505675 - Attachment is obsolete: true
Attachment #505680 - Flags: review?(roc)
Attachment #505675 - Flags: review?(roc)
I believe roc is away for a few days per his blog.
(In reply to comment #11)
> Henrik, I just pushed this to tryserver, so builds will be up in a few hours.
> I've tested the patch locally, and it works for me, but could you verify that
> the bug is fixed with the patch?

Can you please specify which build you wanna have tested? You should add at least the bug number as description if possible. For now I assume it's http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-fc4980d25bb4/
Status: NEW → ASSIGNED
Ok, so the default path works. When I start this tryserver build while staying in online mode, the offline dialog doesn't appear, and we correctly checking for add-on compatibility.

But when I go offline in Firefox 3.6 and start the tryserver build, we are also searching for compatibility updates and dont' bring up the offline window. When the browser is started, it's correctly in offline mode.
This is no longer a blocker, given that bug 620472 was backed out.
blocking2.0: betaN+ → ---
Summary: Upgrading to Firefox 4 the add-ons compatibility check always presents "Minefield is working offline" dialog → add-ons compatibility check always presents "Minefield is working offline" dialog when network link state detection is disabled
Whiteboard: [hardblocker][beta10 blocker]
(In reply to comment #14)
> Can you please specify which build you wanna have tested? You should add at
> least the bug number as description if possible. For now I assume it's
> http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-fc4980d25bb4/

I added the bug number in the commit message; is there somewhere else I should add it? The tryserver build directory had not yet appeared, and I had forgotten that it's just named after the changeset. I will include the directory next time. Thanks.

(In reply to comment #15)
> Ok, so the default path works. When I start this tryserver build while staying
> in online mode, the offline dialog doesn't appear, and we correctly checking
> for add-on compatibility.
> 
> But when I go offline in Firefox 3.6 and start the tryserver build, we are also
> searching for compatibility updates and dont' bring up the offline window. When
> the browser is started, it's correctly in offline mode.

Hmm, I suppose it's best to wait until roc gets back. Two questions we should consider are: Is being in offline mode when upgrading to 4.0 to be likely caused by an auto-detection false negative? If so, should we set the user back to online mode by default on the first run after upgrading to 4.0?
Comment on attachment 505680 [details] [diff] [review]
simpler workaround

This patch isn't doing what we want. What we need is for Services.io.offline (aka mOffline) to report the correct value by the time the Add-ons updater accesses it. Currently, when mManageOfflineStatus is false, mOffline just reports true everytime. My patch just made it report false everytime, with the side effect of initializing things that should be initialized if it will set to true again when actually working offline. I'm not sure how to do this; unassigning self.
Attachment #505680 - Attachment description: simpler patch → simpler attempt
Attachment #505680 - Attachment is obsolete: true
Attachment #505680 - Flags: review?(roc)
Assignee: fryn → nobody
Status: ASSIGNED → NEW
The problem I think is that the IO Service isn't currently responsible for caching the user-set offline state across instances of the app. Instead right now Firefox restores that state late in startup well after it is possible for things like the add-ons manager to actually make network connections.

Seems like it would be better for the IO service to cache it instead so it can restore it the moment it is initialised.
What about just setting mOffline to false in nsIOService's constructor, and removing the SetOffline(PR_FALSE) in nsIOService::Init? It seems to me that that will fix everything here: we'll be online unless and until either the network link service or the browser.offline pref takes us offline.
(In reply to comment #20)
> What about just setting mOffline to false in nsIOService's constructor, and
> removing the SetOffline(PR_FALSE) in nsIOService::Init? It seems to me that
> that will fix everything here: we'll be online unless and until either the
> network link service or the browser.offline pref takes us offline.

I tried setting mOffline to false in the constructor, and it seemed to cause the following security manager error prompt upon shutdown: https://mxr.mozilla.org/mozilla-central/source/security/manager/locales/en-US/chrome/pipnss/pipnss.properties#386
That's weird. I think we should debug that to make progress here.
(In reply to comment #21)
> (In reply to comment #20)
> > What about just setting mOffline to false in nsIOService's constructor, and
> > removing the SetOffline(PR_FALSE) in nsIOService::Init? It seems to me that
> > that will fix everything here: we'll be online unless and until either the
> > network link service or the browser.offline pref takes us offline.
> 
> I tried setting mOffline to false in the constructor, and it seemed to cause
> the following security manager error prompt upon shutdown:
> https://mxr.mozilla.org/mozilla-central/source/security/manager/locales/en-US/chrome/pipnss/pipnss.properties#386

(In reply to comment #22)
> That's weird. I think we should debug that to make progress here.

It seems like SetOffline assumes that mOffline is true the first time that it's called and does something to make the state of everything else consistent. I'll run gdb on that once I have time, but I won't this week.

> > It seems to me that
> > that will fix everything here: we'll be online unless and until either the
> > network link service or the browser.offline pref takes us offline.

Leaving mOffline to true in the constructor and calling SetOffline(PR_FALSE) in Init (like attachment 505680 [details] [diff] [review] does) accomplishes this too. Since browser.offline doesn't take us offline until after the add-ons manager update wizard, this means that the add-ons manager will always try to make connections to update add-ons, which I think is okay, since we don't have the correct value until delayed startup anyway. If you concur that the workaround in attachment 505680 [details] [diff] [review] is sufficient, let's check that in and give the users suffering from bug 620472 a big UX win.
Attachment #505680 - Attachment description: simpler attempt → simpler workaround
Attachment #505680 - Attachment is obsolete: false
Attachment #505680 - Flags: review?(roc)
Attachment #505680 - Flags: approval2.0?
Attachment #505680 - Flags: approval2.0? → approval2.0+
Pushed.

https://hg.mozilla.org/mozilla-central/rev/b5b1e065bc0e
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b11
Assignee: nobody → fryn
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12pre) Gecko/20110209 Firefox/4.0b12pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.