56654 - command-line interface doesn't allow for arbitrary processing

Status: RESOLVED WORKSFORME

(SeaMonkey :: General, defect)

Description

[Mike Shaver (:shaver emeritus)]

Currently, the command-line handler interface (nsICmdLineHandler and
nsAppRunner) only permit command-line switches to set new default chrome URLs.
That's not really enough for some cases, like UI language selection, -remote, etc.

I've added a processArguments method to the nsICmdLineHandler interface, which
will provide for arbitrary processing capability.  I'll attach a patch, and test
case, shortly.

(If you're cc:d on this bug, you probably have a bug assigned to you that's
related to command-line processing or arguments.  Please read the patch and
comment as to whether this makes your life easier, or harder, and whether
additional interface changes would help.)

Comment 1

[Mike Shaver (:shaver emeritus)]

Attachment: Change to nsICmdLineHandler and consumers of the interface

Comment 2

[Mike Shaver (:shaver emeritus)]

Attachment: JS component to demonstrate arbitrary processing and override of chrome games

Comment 3

[Mike Shaver (:shaver emeritus)]

I should add that this will let us clear all of the profile-manager-specifc crud
out of the main application, and let it use this interface as well.  Just to
sweeten the pot a little.

Bueller?

Comment 4

[Mike Shaver (:shaver emeritus)]

You all hate me.

Comment 5

[Mike Shaver (:shaver emeritus)]

Who's kidding whom?

Comment 6

[timeless]

17139:
+        // first, see if this handler wants to launch an app (it might just
+        // be setting some internal state
missing ) in comment.

This looks cool, but I don't think I'm confident enough to give an r= in this 
area.

Comment 7

[Mike Shaver (:shaver emeritus)]

Asa just told me that we shipped 0.9.2!  Who knew?

Comment 8

[chris hofmann]

->0.9.5 since we have run out of time

Comment 9

[Mike Shaver (:shaver emeritus)]

rginda gets this, in order to guilt him a little bit.

Comment 10

[Mike Shaver (:shaver emeritus)]

This doesn't go deep enough, really.  If we're going to expose a command-line
registration interface to component developers in 1.0, and I really think we
should, then we need to rip this baby up and start over.  nsICmdLineHandler is a
disaster, I think we all agree.

I'll even take this bug back, to avoid nauseating rginda any more.

Comment 11

[Robert Ginda]

Thank you Mike Shaver, thank you.

Comment 12

[Tiantian Kong]

Since simple MAPI needs this bug to be fixed, changed milestone to m0.9.7.

Comment 13

[Tiantian Kong]

*** Bug 108766 has been marked as a duplicate of this bug. ***

Comment 14

[Krishna Mohan]

Assigning to myself.  The fix is required to land MAPI on trunk (meta bug
104672).  Also the method "ProcessArguments" will be more generic as

PRUint32 onCommandLineSwitch(in [array,size_is(len)] string argv, in
                             PRUint32 len, in PRUint32 offset);

For a detailed description, see bug 108766.

Comment 15

[Tiantian Kong]

Changed QA to Trix. 

Comment 16

[Asa Dotzler [:asa]]

<[e-mail]> email bounces. Is this person still working on
Mozilla? Should this bug be reassigned?

Comment 17

[timeless]

picking assignee from mapi bug

Comment 18

[(not reading, please use [e-mail] instead)]

rajiv wants to do the right thing, and not add any more hacks, but doesn't have
the cycles to re-write all the existing cmd line handler stuff.

how about this, for a place to start:

rename nsICmdLineHandler as nsICmdLineHandlerObsolete.  add the new interfaces
as nsICmdLineHandler.

Once we get the desired interface agreed on, and into the tree, we can add -mapi
using it, and maybe rewrite -mail or -compose to use it, and add the necessary
C++ code to use both nsICmdLineHandlerObsolete and nsICmdLineHandler (for now.)

we'll mark nsICmdLineHandlerObsolete as depricated.

then, we can log bugs on various owners to switch over from
nsICmdLineHandlerObsolete to nsICmdLineHandler.  once they're all done, we can
remove nsICmdLineHandlerObsolete, and remove the extra C++ goop we've got for
handling both.

comments?  if this sounds like a way to make progress, with out overloading
rajiv, we should start working on the new interface.

Comment 19

[Mike Shaver (:shaver emeritus)]

I have to say, unfortunately, that I'm not surprised at _all_ that this got
dropped on the floor, after we let MAPI land with a "one-off, temporary" hack. 
This was _exactly_ the problem I was concerned about when we discussed this in
email, and I'm sad to see that my cynicism was warranted.  Bah.  As I said back
in November:

> I think that MAPI support isn't higher-priority than this architectural fix, 
> as far as Mozilla's concerned.  I wouldn't mind 0.9.6 shipping without MAPI, 
> but I think that if we don't fix this now while the motivation is present it's 
> only going to get harder down the road. 

I don't want two interfaces.  There's no need for them here, and we already have
too many "nsIWhateverObsolete" concessions to use of unfrozen interfaces in the
tree.  Unfortunately, it seems that the only way to get people to do the right
thing in our communal codebase is to make it really frigging hard to do the
wrong thing, so maybe the way to move forward here is to just unilaterally break
the interface so that the people managing Rajiv-or-whoever's Mozilla time see
their way to allocating the cycles.

(I tried logging bugs on various owners to get them to abandon NSGetFactory,
which has been deprecated for something like two years now.  8 months later, I'm
still waiting, so I'm not really willing to go that route again.  And why would
those owners have the cycles that Rajiv doesn't?  This bug was opened because I
had the cycles to improve the interface -- albeit incrementally, but that was
back in _October_2000_ -- and you can read here how much luck I had even getting
owners of affected code to respond to a WORKING PATCH.)

I'm pretty disappointed that we ended up here, I have to say, though some of
that disappointment is in myself, for failing to hold the line on this before. 
Happy new year.

Comment 20

[Rajiv Dayal]

Hi Mike, I understand your concerns. MAPI has landed but not with any temporary
hack. The bug which has lead to this issue is still open. The current landing of
MAPI would bring up the browser window which we want to avoid by fixing this bug. 

What do u suggest we should do besides re-writing the complete nsICmdLineHandler
if u feel this idea of having another nsI..Obsolete is not good. We can try and
use your patch at this moment and later decide to re-write nsICmdLineHandler as
time permits. I was looking at the nsAppRunner code, there is a quite some code
there that can be re-organized to be either part of or use nsICmdLineHandler
functionality if we want to do it right. Plus there is the Profile Manager stuff
u mentioned. And there could be more. Changing all that can have major impact
and take quite some to get it through.

However I believe enhancing the current implementation and not making it as
nsI..Obsolete as Seth suggested will lead people to continue using the modified
implementation. The 'Obsolete' word would lead people to atleast be cautious
that it is going to change at some time !

Comment 21

[(not reading, please use [e-mail] instead)]

> I have to say, unfortunately, that I'm not surprised at _all_ that this got
> dropped on the floor

I don't think anything has been dropped on the floor.  actually, given the MAPI 
code has landed minus the objectionable MAPI code from the branch, and that 
mozilla has MAPI support, I'd say we're doing good.

Now our problem is how to solve a MAPI bug that was fixed on the branch 
(by "any means") but now existings in mozilla (see #103785)

> I don't want two interfaces

Your explanation (and experience) on the pitfalls of nsIFooObsolete are noted.
We'll go another route.

> I'm pretty disappointed that we ended up here

I'm not disappointed.  The MAPI code that actually made it onto the trunk was 
reviewed and super-reviewed carefully.

I'll talk with some managers, to see about getting some resources to fix it in 
a way that will allow it to be checked into the trunk.  (meaning, fix the 
interface once and for all, and all the consumers.)

Comment 22

[(not reading, please use [e-mail] instead)]

right now, I see two potential solutions:

1) spend the resources now, fix the interface and the consumers, which allows 
us to land and fix the bug on the trunk.

2) wait until netscape branches, netscape applies the hack to the branch, bug 
still exists on trunk.

it doesn't make any sense for us to come up with a hack for the trunk, or a 
nsIFooObsolete, as it would be rejected (and rightly so)

for background:

the problem that we're trying to fix is "on blind send, we get a browser window"

so if mozilla is registered as my MAPI appication, and it's not running when I 
do "send mail" from Word or another applications, I'll get a browser window.

Comment 23

[scottputterman]

<so maybe the way to move forward here is to just unilaterally break
<the interface so that the people managing Rajiv-or-whoever's Mozilla time see
<their way to allocating the cycles.

You're welcome to try this, but my guess is that Seth's choice #2 may be the way
to go.  We'll have to see.  I need more info on how long it would take to do
this.  Right now all of the data I have points to choice #2, which is fine.

Comment 24

[scottputterman]

Moving out.  We'll fix what we need to on a branch and if we have time, we'll
attack this problem later so we can land on the trunk.

Comment 25

[Jeff Hooker]

removed tiantian from cc (bad alias; sends mail to jhooker)

Comment 26

[Brian 'netdragon' Bober]

Is this bug for allowing different components to register code or functions 
with the command line service so that they can parse the data themselves?

If so, see bug 184789 about being able to register context-sensitive help with 
the command line service.

Also, this will be made to block the command line META bug 184790.

Comment 27

[Benjamin Smedberg]

This bug is seamonkey-only, and will be fixed when they switch over to suiterunner.

Comment 28

[Benjamin Smedberg]

Fixed by moving to toolkit.