Closed
Bug 221734
Opened 21 years ago
Closed 15 years ago
Prompt for starting up in offline mode (offline.startup_state==true)
Categories
(SeaMonkey :: MailNews: Backend, defect)
Tracking
(Not tracked)
RESOLVED
EXPIRED
People
(Reporter: mscott, Assigned: diego)
References
Details
Attachments
(4 files, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
neil
:
review+
mscott
:
superreview+
|
Details | Diff | Splinter Review |
I'm not sure what BMS is looking for here. May have to go back and re-clarify. I
think they want some way of prompting at startup as to whether we should be
starting in offline mode or not.
How did 4.x do this? I think there was a check box on the profile manager dialog.
But in the case of Thunderbird we discourage profiles and don't explicitly bring
up a profile manager.
Need to think about what we want to do here some more.
Comment 1•21 years ago
|
||
Scott and I talked about this - what 4.x did is if you have the pref set to "ask
me", we force the profile mgr on startup, and the profile mgr has a checkbox
that says "work offline". This is probably the easiest thing to implement.
Comment 2•21 years ago
|
||
I tried this in 4.x - it pops up a dialog if you have only one profile;
otherwise it pops up the profile manager. I'm not sure how it does this, but I
can see that what I was proposing might be difficult since the prompt me choice
is stored in the prefs.js file for the profile, and the startup code that skips
the profile manager may not know how to do that...
Comment 3•21 years ago
|
||
bug 82487 has a lot of discussion of this issue and related issues. This is
really a dup of that bug, in part, but that bug has a lot of history that we
probably just want to avoid (though it has some useful stuff too)
Comment 4•21 years ago
|
||
this is most of the fix - there's also a small change to allmakefiles.sh (is
that really needed?) and config/config.mk
Comment 5•21 years ago
|
||
Comment 6•21 years ago
|
||
these three diffs are required, I believe. The idea is that offlineStartup is a
mailnews extension, and is required for mailnews.
Comment 7•21 years ago
|
||
Comment on attachment 133719 [details] [diff] [review]
make file changes
do you need that allmakefiles.sh change, or can you just update
mozilla\mailnews\makefiles?
Comment 8•21 years ago
|
||
Comment on attachment 133718 [details] [diff] [review]
packager diffs for new js component
the changes to packages-static-win and packages-win have the slashes going the
wrong way.
Comment 9•21 years ago
|
||
Comment on attachment 133714 [details] [diff] [review]
proposed fix
1)
+const kUNBundleURI =
"chrome://messenger/locale/offlineStartup.properties";
why kUNBundleURI? and later, unBundle?
+const kOfflineStartupPref = "offline.startup_state";
+var gShuttingDown = false;
2)
+var gOfflineStartupMode; //0 = remember last state, 1 = ask me, 2 == online, 3
== offline
consider making those values consts?
3)
+ var prefs = Components.classes["@mozilla.org/preferences-service;1"].
+ getService(Components.interfaces.nsIPrefBranch);
you get the prefs branch several times in your patch.
why not just do it once, globally, gPrefs?
and with gIoService. I think that would make things easier to read.
or were you doing it your way, to release the reference to the service?
4)
+ if (gOfflineStartupMode == 0)
consider use your const for 0 (kRememberLastState) here.
5)
+ if (aTopic == "domwindowopened")
+ {
+ }
+ else if (aTopic == "network:offline-status-changed")
that should be:
+ if (aTopic == "network:offline-status-changed")
right?
Comment 10•21 years ago
|
||
Comment on attachment 133714 [details] [diff] [review]
proposed fix
Seth, global service variables are bad in a component, the best you can do is
get the services when you're created and release them when you're destroyed.
>+# The Original Code is The JavaScript Debugger
;-)
>+ ioService.offline = offline;
Heh, this used to throw an exception if you were already on/offline :-)
>+ if (result == 1)
>+ {
>+ var ioService = Components.classes["@mozilla.org/network/io-service;1"].getService(Components.interfaces.nsIIOService);
>+ ioService.offline = true;
>+ }
(Possibly silly question) Do you know that you're online at this point?
ioService.offline = result == 1; might be more reliable.
Comment 11•21 years ago
|
||
I've moved the properties file into messenger.jar, since this is a required
extension, and that way I can remove all the contents.rdf stuff. I've also
address ed some of Seth's comments, and one of Neil's. I left the prefs service
code the way it is, since there's no sense caching and freeing it on shutdown,
I don't think.
Updated•21 years ago
|
Attachment #133714 -
Attachment is obsolete: true
Comment 12•21 years ago
|
||
fix checked in, r/sr=mscott over aim.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 13•21 years ago
|
||
One more nit... there's no default pref value for network.offline which means
that line 67 of offlineStatup.js fails.
Comment 14•21 years ago
|
||
I don't know if it's related to this bug, but it's the only checking to CVS with
the word 'offline' in it the last 96 hours.
I just upgraded from trunk 2003102004 to 2003102304, which is 3 days difference.
And now, whenever I launch the browser, it pops up a prompt asking me if I want
to work online or offline.
I haven't tried setting the network.offline pref yet - I need some sleep right
now :) But I see from about:config that network.online is 'true' (default).
Any thoughts ?
Comment 15•21 years ago
|
||
do you have a pref called "offline.startup_state" set to 1, in your prefs.js? If
so, set it to 0.
Comment 16•21 years ago
|
||
It's not there at all..
Comment 17•21 years ago
|
||
Ah, it turns out that the pref you should have used was network.online ...
Comment 18•21 years ago
|
||
Neil is right - I should be using network.online - I was worried about some
autoconfig stuff, but I think this is actually the right thing to do.
Reporter | ||
Updated•21 years ago
|
Attachment #134128 -
Flags: superreview+
Updated•21 years ago
|
Attachment #134128 -
Flags: review+
Assignee | ||
Comment 19•21 years ago
|
||
According to the CVS log of mailnews/base/prefs/resources/content/pref-offline.xul
the offline startup state radiobutton was added as discussed in this bug:
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/mailnews/base/prefs/resources/content/pref-offline.xul
I cannot find the patch that was committed attached to this bug.
There are some problems:
- pref-offline.xul was created by me in bug 221736 to move some mailnews prefs
to the mailnews section where they belong.
- The offline startup prefs were left in place, since they are not mailnews
specific.
- The offline startup prefs were commented out pending the fix for bug 82487.
They still are where they have always been, so that we now have a duplication
of the prefs.
This is a mess. I suggest we find a proper and final place for the offline
startup prefs and then remove the commented out stuff once and for all. If we
can agree where to put them I will gladly come up with a patch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Updated•21 years ago
|
Comment 20•21 years ago
|
||
Diego: what do you want to do w/ this bug? As I recall from some testing, this
is working. What does cvs blame say about the checkin?
QA Contact: grylchan → benc
Summary: Prompt for starting up in offline mode → Prompt for starting up in offline mode (offline.startup_state==true)
Assignee | ||
Comment 21•21 years ago
|
||
I'll take care of this when I'm back from vacation in the middle of april.
Assignee: bienvenu → diego
Status: REOPENED → NEW
Comment 22•20 years ago
|
||
(In reply to comment #21)
> I'll take care of this when I'm back from vacation in the middle of april.
Middle of April has come and gone.
I just recently (withing last 2-3 days' builds) started noticing that there is
no way to adjust offline settings. Currently using version 0.7+ (20040825).
Whenever I do a new installer installed build, it asks if I want to go online
two times in a row. I think I used to have the offline extension at one time,
but have since created a totally new profile, so doubt that affects anything.
Assignee | ||
Comment 23•20 years ago
|
||
OK, I'm back and willing to do this. We just need to settle where the new home
of the offline prefs is going to be. We basically have two possibilities:
1) Move them back to their initial place as a top level prefs entry (like
Appearance, Navigator, Privacy, etc).
2) Move the general offline prefs to a different panel. I'd suggest Advanced.
I vote for 2). Comments?
Comment 24•20 years ago
|
||
I'd also vote 2!
You have there all startupprefs in one place, moving this elsewhere would make
it a little confusing!
Assignee | ||
Comment 25•20 years ago
|
||
(In reply to comment #24)
> I'd also vote 2!
> You have there all startupprefs in one place, moving this elsewhere would make
> it a little confusing!
Sorry, but I cannot see any startupprefs in the Advanced panel. You seem to
contradict yourself, please clear up the confusion.
Comment 26•20 years ago
|
||
My fault, I should read more carefully!
I'd put it to the "Offline & Diskspace" menue where you find the startup prefs
Assignee | ||
Comment 27•20 years ago
|
||
(In reply to comment #26)
> My fault, I should read more carefully!
> I'd put it to the "Offline & Diskspace" menue where you find the startup prefs
There is a misunderstanding here then. The startup state prefs are currently in
that panel (Mail & Newsgroups --> Offline & Disk Space) but should be moved away
from there since they are not related to Mail & Newsgroups (and thus missing
when Mozilla Mail is not installed).
Comment 28•20 years ago
|
||
(In reply to comment #27)
> should be moved away
> from there since they are not related to Mail & Newsgroups (and thus missing
> when Mozilla Mail is not installed).
Hmm I see...
Even though for some dial-up users the "offline" settings are as important as
looks I'd put it into 2 (Advanced)
Updated•20 years ago
|
Product: Browser → Seamonkey
Comment 29•15 years ago
|
||
MASS-CHANGE:
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.
If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.
Query tag for this change: mass-UNCONFIRM-20090614
Status: NEW → UNCONFIRMED
Comment 30•15 years ago
|
||
MASS-CHANGE:
This bug report is registered in the SeaMonkey product, but still has no comment since the inception of the SeaMonkey project 5 years ago.
Because of this, we're resolving the bug as EXPIRED.
If you still can reproduce the bug on SeaMonkey 2 or otherwise think it's still valid, please REOPEN it and if it is a platform or toolkit issue, move it to the according component.
Query tag for this change: EXPIRED-20100420
Status: UNCONFIRMED → RESOLVED
Closed: 21 years ago → 15 years ago
Resolution: --- → EXPIRED
You need to log in
before you can comment on or make changes to this bug.
Description
•