Closed Bug 64172 Opened 24 years ago Closed 11 years ago

add -offline command line argument.

Categories

(Core Graveyard :: Profile: BackEnd, enhancement)

Product:
Core Graveyard
Component:
Profile: BackEnd
Platform:
x86
All
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX
Milestone:
Future

People

(Reporter: sspitzer, Assigned: ccarlen)

References

Details

Attachments

(2 files, 1 obsolete file)

Proposed offline-service.js
4.94 KB, text/plain
Details
keeps the checkbox
3.22 KB, patch
sspitzer
: superreview+
Details | Diff | Splinter Review 
from n.p.m.mail-news:

Ole Luehrs wrote:

> Perhaps a silly question, but I havn't found a solution yet.
> I want to write my emails before I go online. But when I start Netscape
> it tells me that it is not online and several messages appear. Is there
> a possibility to start the mail-client with an extra-flag like /offline
> or anything else.

I wrote:

> if you start up in the profile manager (-ProfileManager) there is a check box
> for "offline".  try that.

> a "-offline" command line argument would be useful, especially for people with
> only one profile.  (If you have one profile, we skip the profile manager.)  
> I'll log a bug to track it.

assigning to conrad.
Sounds useful and easy. This just boils down to a call to
nsIOService::SetOffline(). I'm not sure that this should be handled by profile
mgr's command line processing though, since it has nothing to do with profiles.
I'll look for some other command line reader.
marking as RFE
Severity: normal → enhancement
if nsIOService is really a service, we can add the command line handler to it.

if you want, I can take this bug.
-> sspitzer - thanks
Assignee: ccarlen → sspitzer
This would be *so* usefull for me, since I have a dial-up account, I'm offline
most of the time and like to download message for reading offline. However, I
have no way to start Mozilla without getting errors relating to my lack of
'online-ness'.

I'm sure there are lots of others who would also apreciate this option.
we'll need to fix #56654 first.
Depends on: 56654
Keywords: review
Comment on attachment 83110 [details]
Proposed offline-service.js

I need to subject this to some more detailed scrutiny, which I'll do as soon as
I get the chance.

A couple of things I noticed in a casual reading of the code: Your
implementation of getter for chromeUrlForTask doesn't return anything. 
"Convention" (such as it is, given one or two examples) is to throw
NS_ERROR_NOT_IMPLEMENTED or NS_ERROR_NOT_AVAILABLE (depending on whether you
want the calling code to proceed with opening a default window).

2. You don't indicate where this file will reside in the source tree (a new
xpfe/components directory?) or the various makefile/script changes that are
necessary to get this file built and distributed.
Attachment #83110 - Flags: needs-work+
I applied the patch with some makefile.win magic (basically, got the new 
service installed).  It doesn't work too well, though.

1. If I try "mozilla -p xxx -offline" it seems to "hang" with the splash screen 
showing.  Apparently, "-offline" is causing the load of the chrome for the 
first browser window to hang (and since no window appears, the splash screen 
doesn't go away).

2. So I started up normally, then tried "mozilla -offline" from another 
session.  That works.  But, a new browser window is opened (but never appears, 
due to the problem cited above, I presume).  When I close the visible window, 
the app doesn't terminate because it still thinks this zombie browser window is 
open.  Note that this could be readily fixed via returning 
NS_ERROR_NOT_AVAILABLE.  But that still leaves issue 1.
Thanks for the review. To address your points:
I don't know how or where to add the file. I don't even have a build. I just
created it in the components folder.
Sorry, I had only tested Quit, not Close (I usually have several windows open).
I only tested -offline with other arguments (-jsconsole). I didn't want my
thrown exception to appear in the JS console! I have since found that return
":"; will defer the error to the OpenWindow function as ":" is an illegal URL
whereas returning nothing will open a window but not start to load anything in it!
1. *Somebody* has to do the build housekeeping to put this component in place.  
I'd suggest creating a new "offline" subdirectory under xpfe/components and 
putting it there.  Create a makefile.win and Makefile.in like in 
xpfe/components/killAll.  "offline" would need to be added to DIRS in 
xpfe/components' makefile.win/Makefile.in, and a line would need to be added to 
some Mac build script.

2. I don't think returning ":" is the right thing to do ("right" being 
relative, since bug 56654 is the proper fix).  Throwing 
NS_ERROR_NOT_IMPLEMENTED does the same thing in a more reliable way (not 
relying so much on current implementation for which returning ":" just 
accidentally works).  The console message is fine with me.  But maybe we could 
explicitly adopt some convention whereby returning an empty chrome URL means 
the same thing?
Bah, no simple fix. But the file (bin/components/offline-service.js) is there if
anyone desparately wants it, just remember to use Quit to exit Mozilla.
I need this for something else - taking.
Assignee: sspitzer → ccarlen
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.3alpha
talked it over with conrad.

while you could do this with nsICmdLineHandler, that interface (yes yes, I know 
it it has issues, query bugzilla) isn't appropriate for this.

that interface is for applications.  like mail, addressbook, nim, etc.

my proof: 
http://lxr.mozilla.org/mozilla/source/xpfe/appshell/public/nsICmdLineHandler.idl
#200

"chromeUrlForTask" makes no sense for a switch like offline.

sorry if my initial comments mislead the author of the patch (neil?)




The current command line handler isn't suitable for -killAll, -resetPref,
-setDefaultBrowser, -setDefaultMail or -unsetDefaultMail either...
Attached patch patch without handler (obsolete) — — Splinter Review 
This patch simply has the appshell service look for an -offline arg on the
command line. It also removes the checkbox from the profile mgr UI since I
thought it never really made sense there.

To make use of this switch, you would need a shortcut (Windows) or a CmdLine
file (Mac). Shortcuts have to be made by the installer. On Mac, we only
distrubute 2 of these files, though we have many. Also, since -offline is a
switch that could be applied to various startup apps, do we make at least two
new ones, only make one for mail, or neither and document it and let the user
do what they need?
Attachment #106922 - Flags: superreview?(sspitzer)
Attachment #106922 - Flags: review?(law)
woah, the offline checkbox in the profile manager is extremely useful - why did
you remove it? I use it all the time.
Because there's another checkbox I need to put there - 2 are ugly.
You use it because, without a -offline switch, there's no other choice. If you
have an alternative, do you still really need the checkbox?

yes, because I don't want a separate launch shortcut icon cluttering up my
desktop that specifies the offline parameter, and I don't want to have to
remember to click the right icon. Many people don't even know how to create a
separate shortcut icon and fill in the offline parameter, but they do know how
to check the offline box.
And if they have only 1 profile and never see this checkbox?
then they never see the prompt. But I have multiple profiles. If this feature
had never been there, your argument would be stronger, but if you take away a
feature, you're going to upset people. This feature has been there since 4.0
Can't we keep checkbox (for multiple profiles)
and add -offline arg code to be used by people who have single profiles?
if you start up -offline and the profile mgr comes up, the offline box should be
checked, so it's still useful. But it sounds like a screen real estate issue -
is there not room for two checkboxes?
My take is that you're not going to upset THAT many people .. we're talking a
subset of the already small subset of people who even use the profile
picker..(though unfortunately, bienvenu is in that group - sorry david!)

I guess I see the offline checkbox as being a pretty mail-specific feature since
the browser's offline support is pretty useless.
Comment on attachment 106922 [details] [diff] [review]
patch without handler

Dunno about removing the profile manager checkbox (I think it should stay,
myself).  But the code looks OK.
Attachment #106922 - Flags: review?(law) → review+
Target Milestone: mozilla1.3alpha → mozilla1.4alpha
sorry for the delay.

yes, let's keep the checkbox UI.

reviewing now...

Comment on attachment 107157 [details] [diff] [review]
keeps the checkbox

sr=sspitzer, with a few comments and nits.

1)

thanks for switching to contract id.

see http://bugzilla.mozilla.org/show_bug.cgi?id=72827

you might have fixed the open issues.  (if not, feel free to take it.)

2)

can you elaborate on why you added the code to nsAppShellService.cpp, instead
of other places (see
http://lxr.mozilla.org/mozilla/search?string=GetCmdLineValue)

(I'm sure there's a good reason, perhaps you can add a comment?)


3) 

for win32 (and other platforms?) do we need to do anything special to handle
when mozilla is running, and someone does "mozilla.exe -offline"?

for example, see

nsNativeAppSupportWin::HandleRequest()

http://lxr.mozilla.org/mozilla/source/xpfe/bootstrap/nsNativeAppSupportWin.cpp#
1720

feel free to spin this up into another bug.

4)

this masks any earlier failures.  so if rv comes in as failure, it goes out as
NS_OK.

instead, how about this:

+  if (mCmdLineService) {
+    nsXPIDLCString cmdVal;
+    if (NS_SUCCEEDED(mCmdLineService->GetCmdLineValue("-offline",
getter_Copies(cmdVal))) && !cmdVal.IsEmpty()) {
+      nsCOMPtr<nsIIOService>
ioService(do_GetService(NS_IOSERVICE_CONTRACTID));
+      if (ioService)
+	 ioService->SetOffline(PR_TRUE);
+    }
+  }

this way, rv is untouched.
Attachment #107157 - Flags: superreview+
Comment on attachment 106922 [details] [diff] [review]
patch without handler

this patch is obsolete, since we are keeping the checkbox UI.
Attachment #106922 - Attachment is obsolete: true
Attachment #106922 - Flags: superreview?(sspitzer) → superreview-
I don't need this anymore. If anybody's interested, take it.
Target Milestone: mozilla1.4alpha → Future
Should this be WONTFIX? Between the prompt and the profile manager checkbox,
users have too many ways of starting offline.
*** Bug 255413 has been marked as a duplicate of this bug. ***
*** Bug 288927 has been marked as a duplicate of this bug. ***
I disagree with comment #31.  In principle, automating away any and all prompts
and dialogs is an improvement.  And as a bonus, this enhancement will make it
possible to fix bug 284004.

Besides, Thunderbird already has this functionality.  Only the browser needs it
(which is why I submitted bug 288927 under product Firefox).
QA Contact: agracebush → profile-manager-backend
We autodetect offline state now and won't be adding this flag.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → WONTFIX
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: