87127 - Need to support unicode interface for nsICommandLineRunner

Status: RESOLVED WORKSFORME

(Core :: Internationalization, defect)

Description

[Roy Yokoyama]

Insert from bug 58866:

========================================================
This looks like a problem with not converting to UCS2
before passing to JS.
nsCmdLineService::GetURLToLoad(char ** aResult)
{ return GetCmdLineValue("-url", aResult); }

We may need to change from :
interface nsICmdLineService : nsISupports
{
  string Initialize(in long argc);
  string getCmdLineValue(in string argc);
  readonly attribute string URLToLoad;
  readonly attribute string programName;
  readonly attribute long argc;
  [noscript] readonly attribute charArray argv;
};

to

interface nsICmdLineService : nsISupports
{
  wstring Initialize(in long argc);
  wstring getCmdLineValue(in string argc);
  readonly attribute wstring URLToLoad;
  readonly attribute wstring programName;
  readonly attribute long argc;
  [noscript] readonly attribute charArray argv;
};

Comment 1

[Frank Tang]

mark it as moz1.0. please work with alecf about this.

Comment 2

[Alec Flett]

I disagree. urls should be UTF8 - I don't see anything here to change to UCS2
if the command line argv[n] is passing in a string that's not in UTF8, then we
need to do the conversion to UTF8...

Comment 3

[Roy Yokoyama]

Accepting

Comment 4

[Roy Yokoyama]

making this bug as one of unicode related bugs

Comment 5

[Boris Zbarsky [:bzbarsky]]

This blocks freezing nsIWebNavigation because we can't switch that to take
nsAUTF8String for the uri until this bug is fixed.  Fixing this would allow
significant code simplification and speedup in the uri fixup code as well.

Let me see if I understand the situation correctly.  Simply changing 

  readonly attribute string URLToLoad;

to

  readonly attribute AUTF8String URLToLoad;

and fixing the implementations of nsICmdLineService to do proper native-to-utf8
conversion on that argument would help, no?

I'm not sure about also changing getCmdLineValue(); is there ever a situation
where non-textual binary data is being passed on the command line?  The fact
that the example there is using it on urls is _not_ helpful.  ;)

The out param on

 void initialize(in long argc, out string argv);

is ridiculously evil, see bug 98952.  That should not use wstring by any means.
programName should probably be an nsAString.  So my proposed version, ignoring
initialize for now, is

interface nsICmdLineService : nsISupports
{
   [noscript] void initialize(in long argc, out string argv);
   AString getCmdLineValue(in string argName);
   // same, but returns the raw data without doing any encoding guessing or
   // conversion
   [noscript] string getRawCmdLineValue(in string argName);
   readonly attribute AString programName;
   readonly attribute long argc;
   [noscript] readonly attribute charArray argv;  
};

That said, I'm not sure how the fix for bug 116711 interacts with this code...

So when all's said and done I'm willing to implement stuff but I think we should
decide what this api should look like and what its callers are or are not
expected to do.  Anyone?  Thoughts?

Comment 6

[Boris Zbarsky [:bzbarsky]]

More to the point, the question that really concerns me is "Can I convert
nsIWebNavigation and nsIURIFixup to use AUTF8String and remove the hack in
nsDefaultURIFixup without regressing things, now that bug 116711 makes us do
proper unicode conversion in the appshell?  Or do we need api changes to
nsICmdLineService to be able to do that?"

Comment 7

[Andreas Becker]

Changing QA contact to bobj for now. Bob, please re-assign further as you see is
appropriate.

Comment 8

[Jungshik Shin]

Sorry for bug spam

Comment 9

[Benjamin Smedberg]

nsICmdLine is dead, long live nsICommandLine (bug 276588). nsICommandLineRunner still needs a UTF16 initializer in addition to the native-charset initializer if we're going to remote windows wide-char command lines properly.

Comment 10

[(no longer active)]

Morphing according to comment 9.

Comment 11

[Benjamin Smedberg]

windows assumes the char* is utf8 now, so I think this WFM