add -offline command line argument.

RESOLVED WONTFIX

Status

Core Graveyard
Profile: BackEnd
--
enhancement
RESOLVED WONTFIX
17 years ago
2 years ago

People

(Reporter: (not reading, please use seth@sspitzer.org instead), Assigned: Conrad Carlen (not reading bugmail))

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

4.94 KB, text/plain
Details
3.22 KB, patch
(not reading, please use seth@sspitzer.org instead)
: 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.
(Assignee)

Comment 1

17 years ago
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.

Comment 2

17 years ago
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.
(Assignee)

Comment 4

17 years ago
-> sspitzer - thanks
Assignee: ccarlen → sspitzer

Comment 5

17 years ago
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

Comment 7

16 years ago
Created attachment 83110 [details]
Proposed offline-service.js

Updated

16 years ago
Keywords: review

Comment 8

16 years ago
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+

Comment 9

16 years ago
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.

Comment 10

16 years ago
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!

Comment 11

16 years ago
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?

Comment 12

16 years ago
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.
(Assignee)

Comment 13

15 years ago
I need this for something else - taking.
Assignee: sspitzer → ccarlen
(Assignee)

Updated

15 years ago
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?)



Comment 15

15 years ago
The current command line handler isn't suitable for -killAll, -resetPref,
-setDefaultBrowser, -setDefaultMail or -unsetDefaultMail either...
(Assignee)

Comment 16

15 years ago
Created attachment 106922 [details] [diff] [review]
patch without handler

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?
(Assignee)

Updated

15 years ago
Attachment #106922 - Flags: superreview?(sspitzer)
Attachment #106922 - Flags: review?(law)

Comment 17

15 years ago
woah, the offline checkbox in the profile manager is extremely useful - why did
you remove it? I use it all the time.
(Assignee)

Comment 18

15 years ago
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?

Comment 19

15 years ago
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.
(Assignee)

Comment 20

15 years ago
And if they have only 1 profile and never see this checkbox?

Comment 21

15 years ago
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

Comment 22

15 years ago
Can't we keep checkbox (for multiple profiles)
and add -offline arg code to be used by people who have single profiles?

Comment 23

15 years ago
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?

Comment 24

15 years ago
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 25

15 years ago
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+
(Assignee)

Comment 26

15 years ago
Created attachment 107157 [details] [diff] [review]
keeps the checkbox
(Assignee)

Updated

15 years ago
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-
(Assignee)

Comment 30

15 years ago
I don't need this anymore. If anybody's interested, take it.
Target Milestone: mozilla1.4alpha → Future

Comment 31

14 years ago
Should this be WONTFIX? Between the prompt and the profile manager checkbox,
users have too many ways of starting offline.

Comment 32

14 years ago
*** Bug 255413 has been marked as a duplicate of this bug. ***

Comment 33

13 years ago
*** Bug 288927 has been marked as a duplicate of this bug. ***

Comment 34

13 years ago
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

Comment 35

5 years ago
We autodetect offline state now and won't be adding this flag.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.