1307607 - Assume working directory in nsICommandLine if not set by user

Status: UNCONFIRMED

(Toolkit :: General, defect)

Description

[Manuel Grießmayr]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20160920074044

Comment 1

[Manuel Grießmayr]

Attachment: abspath.patch

Comment 2

[Jorg K (CEST = GMT+2)]

Now you're in Mozilla core code (toolkit). You'd have to ask a M-C person for feedback/review.
Maybe a "storage" guy?
https://wiki.mozilla.org/Modules/Core#storage

Comment 3

[Manuel Grießmayr]

We would like to do something like this in Javascript:
> let cmdLine = Components.classes["@mozilla.org/toolkit/command-line;1"]
>                         .createInstance(Components.interfaces.nsICommandLine);
> absPath = cmdLine.resolveFile(args.message).path;
At the moment this is not working because the attribute mWorkingDir has to be initialised manually by the user with the Init() function. As far as I understand the Init() function is not accessible via Javascript because the function is not listed in nsICommandLine.idl.

Comment 4

[Marco Bonardo [:mak]]

Comment on attachment 8797783 [details] [diff] [review]
abspath.patch

Review of attachment 8797783 [details] [diff] [review]:
-----------------------------------------------------------------

I see Nathan reviewd patches here recently, so forwarding to him.
I can eventually look at this as last resort if nobody else can be found, I don't know this code very well.

Comment 5

[Nathan Froyd [:froydnj]]

Comment on attachment 8797783 [details] [diff] [review]
abspath.patch

Review of attachment 8797783 [details] [diff] [review]:
-----------------------------------------------------------------

I don't think we want to do this.  If you really want this functionality, you can use the directory service from JavaScript to get the current working directory, and use nsIFile.setRelativePath.

Comment 6

[Magnus Melin [:mkmelin]]

(In reply to Nathan Froyd [:froydnj] from comment #5)
> I don't think we want to do this.  If you really want this functionality,

May I ask why not?

> you can use the directory service from JavaScript to get the current working
> directory, and use nsIFile.setRelativePath.

But there's no file to set setRelativePath on. That's the whole point of the resolveFile method: to resolve the file, be that relative or absolute path.

Comment 7

[Nathan Froyd [:froydnj]]

(In reply to Magnus Melin from comment #6)
> (In reply to Nathan Froyd [:froydnj] from comment #5)
> > I don't think we want to do this.  If you really want this functionality,
> 
> May I ask why not?

Because nsICommandLine is an old, relatively unused API, and resolveFile isn't really part of the functionality of command lines: resolveFile is only used once in Firefox itself and a handful of times in addons.  It's also not worth adding corner-case functionality to it when that same functionality is available by other means.

> > you can use the directory service from JavaScript to get the current working
> > directory, and use nsIFile.setRelativePath.
> 
> But there's no file to set setRelativePath on. That's the whole point of the
> resolveFile method: to resolve the file, be that relative or absolute path.

The code looks something like:

  let file = /* create an nsIFile object */
  try {
    // Absolute path case.
    file.init(somePath);
  catch (e) {
    // We threw, so somePath was a relative path.
    // (Robust code might work a little harder to determine whether
    // somePath was relative or absolute prior to doing this, so
    // we're not treating all exceptions as indicating that somePath
    // was relative.)
    let workingDir = /* Services.dirsvc.something() */
    file.setRelativePath(workingDir, somePath);
  }

Does that clear things up?

Comment 8

[Justin Dolske [:Dolske]]

I'm not sure what you're trying to do (at a higher level), but you may also wish to look at OS.File and OS.Path, instead of the xpcom interfaces. They're the more modern way for chrome JS to work with files, and something needs extended to support a usecase it would be better to do so there.

https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/OSFile.jsm

Comment 9

[Magnus Melin [:mkmelin]]

(In reply to Nathan Froyd [:froydnj] from comment #7)
> Does that clear things up?

You can use code like that, though it's quite ugly IMHO. I don't think that a method is rarely used is a good reason not to fix it, especially if a patch is attached.

Yes OSFile is great, but in use cases like the bug 882104, using async methods would require significant re-factoring for little gain.

Comment 10

[Nathan Froyd [:froydnj]]

(In reply to Magnus Melin from comment #9)
> (In reply to Nathan Froyd [:froydnj] from comment #7)
> > Does that clear things up?
> 
> You can use code like that, though it's quite ugly IMHO. I don't think that
> a method is rarely used is a good reason not to fix it, especially if a
> patch is attached.

Yes, "rarely used" on its own is not a great reason.  But there were other reasons given...

The non-existence of nsICommandLine.init suggests that it's not designed to be created from JavaScript.