Andrew Schultz Reporter
	Description • 18 years ago

-edit does not work on the suiterunner commandline to start up composer.  Neil said this is because "composer's startup handler hasn't been toollkitified"

	Mark Banner (:standard8) Assignee
	Comment 1 • 18 years ago

I did this fairly quickly based on inspector's command line handler (that supports both xpfe & toolkit). I've tested its displayed in -help, and checked that -edit <url> works ok. Not sure about default parameter on the toolkit side though, so there may be another change required before its ready to go in.

	Mark Banner (:standard8) Assignee
	Comment 2 • 18 years ago

Comment on attachment 248943 [details] [diff] [review]
Possible fix

+    catch (e) {
+      cmdLine.handleFlag("edit", true);
+    }

Note to self:
add 'args.data = "about:blank";' into the catch. The parameter to handleFlag can probably be false as well.

	Mark Banner (:standard8) Assignee
	Comment 3 • 18 years ago

This fixes the -edit flag and the general.startup.editor preference.

Note that from my analysis of toolkit's commandline handler, the handleFlagWithParam function in nsBrowserContentHandler.js should return NS_ERROR_INVALID_ARGUMENT in all instances.

This does give rise to:

************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'Illegal value' when calling method: [nsICommandLine::handleFlagWithParam]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: file:///mozilla/master/mozilla/sm2/dist/bin/components/nsComposerCmdLineHandler.js :: handler_handle :: line 81"  data: no]
************************************************************

on the console (when starting using the pref), but I can't see how to get around that and the irc preference at least does it as well ;-)

The rest of this is basically following what inspector has done, but allowing for "editor" as a flag as well so that we handle the general.startup.editor pref.

	neil@parkwaycc.co.uk
	Comment 4 • 18 years ago

(In reply to comment #3)
>I can't see how to get around that
I added a hack to nsBrowserContentHandler.js to let you work around the bug.
Note that the exception reads "Call to xpconnect wrapped JSObject".
Therefore the solution is to unwrap the object ;-)
nsComposerCmdLineHandler.prototype = {
  get wrappedJSObject() {
    return this;
  },

  /* nsISupports etc.

	neil@parkwaycc.co.uk
	Comment 5 • 18 years ago

Comment on attachment 249025 [details] [diff] [review]
Patch v2

>   handleFlagWithParam : function handleFlagWithParam(flag, caseSensitive) {
>-    if (this.handleFlag(flag, caseSensitive))
>-      throw Components.results.NS_ERROR_INVALID_ARG;
>+    throw Components.results.NS_ERROR_INVALID_ARG;
>   },
This is done deliberately in case the command line handler tries to handle a flag that we didn't invoke it to handle, so for instance if we invoke it to handle general.startup.editor we should ignore the -edit flag.

	neil@parkwaycc.co.uk
	Comment 6 • 18 years ago

Comment on attachment 249025 [details] [diff] [review]
Patch v2

>+    try {
>+      var uristr = cmdLine.handleFlagWithParam("edit", false);
>+      if (uristr == null)
>+        return;
>+      try {
>+        args.data = cmdLine.resolveURI(uristr).spec;
>+      }
>+      catch (e) {
>+        return;
>+      }
>+    }
>+    catch (e) {
>+      if (!cmdLine.handleFlag("edit", false) &&
>+          !cmdLine.handleFlag("editor", false))
>+	return;
>+      args.data = "about:blank";
>+    }
This is wrong. handleFlagWithParam returns null if the flag does not exist, and throws an exception if it exists but has no param. You should additionally never call both handleFlagWithParam and handleFlag for the same flag.

	Mark Banner (:standard8) Assignee
	Comment 7 • 18 years ago

Revised patch incorporating Neil's comments. This one is much nicer ;-) Thanks Neil.

	neil@parkwaycc.co.uk
	Comment 8 • 18 years ago

Comment on attachment 249151 [details] [diff] [review]
Patch v3

>+  handle : function handler_handle(cmdLine) {
Nit: I'd have named the function after the accessor only, not the class.

>+  defaultArgs : "about:blank",

>+      // One of the flags is present but no data, so set default arg.
>+      args.data = "about:blank";
Technically this isn't necessary, as the chrome defaults to blank ;-)

	Mark Banner (:standard8) Assignee
	Comment 9 • 18 years ago

Patch checked in -> fixed.