Closed Bug 395371 Opened 13 years ago Closed 13 years ago

do not launch a second browser window when calling firefox -jsconsole

Categories

(Toolkit Graveyard :: Error Console, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9beta1

People

(Reporter: arno, Assigned: philip.chee)

Details

(Keywords: regression)

Attachments

(2 files, 2 obsolete files)

Hi,
when you firefox is not launched, and you call firefox -jsconsole, firefox opens a browser window, and javascript console: that seems quite reasonable.
But when you already have one browser window open, if you call firefox -jsconsole, it would be nice to launch console, but not another browser window.
<http://lxr.mozilla.org/seamonkey/source/toolkit/components/console/jsconsole-clhandler.js#90>

        // note that since we don't prevent the default action, you'll get
        // an additional application window, unless you specified another
        // command line flag

Replacing the comment with this line.: | cmdLine.preventDefault = true; | seem to fix the problem.

I see this in SeaMonkey 2.0a1pre too. This is a regression from xpfe-seamonkey so I'm adding the [regression] keyword.
Keywords: regression
Attached patch Patch v1.0Splinter Review
q.v. <https://bugzilla.mozilla.org/show_bug.cgi?id=338899#c2>
The previous attempt to prevent unnecessary browser windows opening was minused however, but perhaps Benjamin could rethink his objections.
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Attachment #280073 - Flags: review?
OS: Linux → All
Hardware: PC → All
Version: unspecified → Trunk
Comment on attachment 280073 [details] [diff] [review]
Patch v1.0

I wonder why the requestee field didn't take.
Attachment #280073 - Flags: review? → review?(benjamin)
Attached patch alternative behaviour (obsolete) — Splinter Review
Here is an alternative solution:
it checks if there are already window loaded. If there are some, it preventDefaults, otherwise, it does not prevent default.
The reason is:
when firefox is *not* already running, and I run "firefox -jsconsole"
I find it consistent to open console *and* browser (because console alone is not very useful in my opinion).
but when firefox is already running, I find it better to get only console.

That is also the way things work in thunderbird: thunderbird -jsconsole opens thunderbird only if it is not already open.


But may be other people will disagree with me.

What do you think is best ?
Comment on attachment 280073 [details] [diff] [review]
Patch v1.0

If there are no windows open already, we should open one.
Attachment #280073 - Flags: review?(benjamin) → review-
Attachment #280111 - Flags: review?(benjamin)
Comment on attachment 280111 [details] [diff] [review]
alternative behaviour

This won't work correctly: what happens if a handler prior to this one opens a window?

It may make sense that if this is an initial command line, don't preventDefault, but if it's a remote command, then you can preventDefault.
Attachment #280111 - Flags: review?(benjamin) → review-
I assumed that: if state is STATE_REMOTE_EXPLICIT, a command has been specified with -remote, and therefore, we don't want to preventDefault
Attachment #280111 - Attachment is obsolete: true
Attachment #282452 - Flags: review?(benjamin)
> +        if (cmdLine.state ==  nsICommandLine.STATE_REMOTE_AUTO) {
> +            cmdLine.preventDefault = true;
> +        }
No need for a {} block for a one line if.

> +
No need for this blank line.
Comment on attachment 282452 [details] [diff] [review]
preventDefault if state is STATE_REMOTE_AUTO

>Index: console/jsconsole-clhandler.js

>+        if (cmdLine.state ==  nsICommandLine.STATE_REMOTE_AUTO) {

nit: extra space
Attachment #282452 - Flags: review?(benjamin) → review+
carrying over r+bsmedberg
Attachment #282452 - Attachment is obsolete: true
Attachment #283164 - Flags: review+
Attachment #283164 - Flags: approval1.9?
Attachment #283164 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Checking in toolkit/components/console/jsconsole-clhandler.js;
/cvsroot/mozilla/toolkit/components/console/jsconsole-clhandler.js,v  <--  jsconsole-clhandler.js
new revision: 1.8; previous revision: 1.7
done
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M9
Product: Firefox → Toolkit
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.