Closed
Bug 364141
Opened 18 years ago
Closed 18 years ago
toolkitify composer's startup handler
Categories
(SeaMonkey :: Composer, defect)
SeaMonkey
Composer
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: ajschult784, Assigned: standard8)
References
Details
Attachments
(1 file, 2 obsolete files)
4.89 KB,
patch
|
neil
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
-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"
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.
Assignee: composer → bugzilla
Status: NEW → ASSIGNED
Assignee | ||
Updated•18 years ago
|
OS: Linux → All
Hardware: PC → All
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.
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.
Attachment #248943 -
Attachment is obsolete: true
Attachment #249025 -
Flags: superreview?(neil)
Attachment #249025 -
Flags: review?(neil)
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.
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.
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.
Attachment #249025 -
Flags: superreview?(neil)
Attachment #249025 -
Flags: superreview-
Attachment #249025 -
Flags: review?(neil)
Attachment #249025 -
Flags: review-
Assignee | ||
Comment 7•18 years ago
|
||
Revised patch incorporating Neil's comments. This one is much nicer ;-) Thanks Neil.
Attachment #249025 -
Attachment is obsolete: true
Attachment #249151 -
Flags: superreview?(neil)
Attachment #249151 -
Flags: review?(neil)
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 ;-)
Attachment #249151 -
Flags: superreview?(neil)
Attachment #249151 -
Flags: superreview+
Attachment #249151 -
Flags: review?(neil)
Attachment #249151 -
Flags: review+
Assignee | ||
Comment 9•18 years ago
|
||
Patch checked in -> fixed.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•