650078 - -no-remote browser accepts remote commands

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Steve Fink [:sfink] [:s:]]

Scenario:

1. start up test browser with -no-remote.
2. start up another browser without using -no-remote.

Result:

new browser reuses the test browser instance.

Expected results:

I would expect a new browser to start up.

My interpretation of this is that -no-remote must mean "do not reuse a running instance", not "do not use the remoting mechanism at all". But this is problematic for development, because I'll generally be running my "production" browser. Part of the time, I will also be running a test browser with -no-remote. Good so far. But then if I need to restart my production browser for whatever reason, it doesn't work (as in, it reuses my test browser).

I don't know whether it's ok to disable the remoting, whatever that might be. If not, perhaps allow passing a -remote-key KEY option? (If you can't tell, I know nothing about how this works.)

I suppose I could run my production browser with -no-remote now that I know it doesn't disable accepting remote commands. But then how does it know which browser to use for remote commands? I'm confused.

Comment 1

[Benjamin Smedberg]

That does surprise me a little, I thought that -no-remote disabled both the sending and receiving bits (I'm pretty sure it does on Windows).

Comment 2

[Benjamin Smedberg]

Note, this is not something I will prioritize, but you can probably fix it starting here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#3732

Comment 3

[Steve Fink [:sfink] [:s:]]

Attachment: Do not listen for remote commands when run with -no-remote

Got pinged by this bug when gavin CC'd, so here's a tiny fix. I just tried it out; it seemed to work.

Comment 4

[Benjamin Smedberg]

Comment on attachment 555464 [details] [diff] [review]
Do not listen for remote commands when run with -no-remote

Because of quirks in the PR_SetEnv/PR_GetEnv API across platforms, you need to check that it's not an empty string also, so "char* e = PR_GetEnv("MOZ_NO_REMOTE"); if (!e || !*e)...

Comment 5

[Steve Fink [:sfink] [:s:]]

Attachment: Do not listen for remote commands when run with -no-remote

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #4)
> Comment on attachment 555464 [details] [diff] [review]
> Do not listen for remote commands when run with -no-remote
> 
> Because of quirks in the PR_SetEnv/PR_GetEnv API across platforms, you need
> to check that it's not an empty string also, so "char* e =
> PR_GetEnv("MOZ_NO_REMOTE"); if (!e || !*e)...

Ok. But I copied that test from earlier in the same function, so I guess I'll have to fix both occurrences. (I guess it didn't hurt in practice because this is only relevant when MOZ_ENABLE_XREMOTE, which subsets the platforms already?)

I'll leave style questions up to your review, because neighboring code is inconsistent about bracing single-line consequents, and you may dislike my mini scope block.

Comment 6

[Benjamin Smedberg]

Comment on attachment 556606 [details] [diff] [review]
Do not listen for remote commands when run with -no-remote

The scoping doesn't seem necessary but also doesn't seem to hurt.

Comment 7

[Ed Morley [:emorley]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/ba38da32b848

http://hg.mozilla.org/mozilla-central/rev/ba38da32b848

Comment 8

[Jason Duell]

So this makes it so that there can only be one instance of FF running that doesn't have -no-remote specified.

IMO the "best" fix here would be to also honor the -profile flag:  right now we'll gladly open "firefox -profile bar http://" in a tab of a browser instance owned by already-running profile "foo".   It would be better to either open a new instance when a different profile is specified, or fail.

Comment 9

[Benjamin Smedberg]

Profile+remote interaction is a RFE which I've wontfixed for code-complexity reasons.

Comment 10

[Ryan]

it looks like this fix was first released in firefox 9? assuming so, it's broken a feature that a few of us depend on: running multiple instances with separate profiles with -no-remote -profile ..., and then using command line -profile X -new-tab (or -new-window, etc) to open a URL in a specific instance.

details: http://superuser.com/questions/372573/cant-aim-command-line-at-one-of-multiple-running-instances-since-upgrading-to-f
maybe also the same root cause: https://bugzilla.mozilla.org/show_bug.cgi?id=703021

i definitely hear the code complexity argument. however, this behavior we depended on through ff8 is now broken in ff9. :/

it doesn't sound like the OP actually had a problem or bug that needed fixing, they were just confused by the flag name. the original behavior was explained clearly in firefox -help:

  -no-remote         Open new instance, not a new window in running instance.

so i'm not convinced this needed changing. also, if i understand MOZ_NO_REMOTE right, it provides an easy workaround for the OP, but there's no similar workaround for us on the other side of the fence.

any chance you all might consider reverting this?

thanks in advance...

Comment 11

[Ryan]

sorry, i misinterpreted the OP's initial problem a bit; it definitely wasn't just flag name confusion.

steve, one solution to your problem would be to run your test browser with a separate profile. you'd launch them like this:

firefox -no-remote -profile production
firefox -no-remote -profile test

you'd then use firefox -profile X ... for remote commands, aiming at whichever browser you want. (jason and benjamin may be skeptical, and the implementation may not be ideal, but firefox -profile ... definitely does send remote commands to the specified profile, at least in ff8 and before.)

any chance that might work for you, and we could consider rolling this back?

Comment 12

[Adam M. Costello]

I second the request for rollback.  I just lost an hour trying to figure
out why firefox --new-window wasn't working anymore.

--no-remote was working as documented in the help text.  If a
don't-listen-to-remote-commands feature is needed, add a new flag, don't
make an incompatible change to an existing flag.

Can we reopen this bug, or do we need to create a new one?

Comment 13

[Ryan]

steve, benjamin...ping?

Comment 14

[Steve Fink [:sfink] [:s:]]

I'm certainly not the one to decide, but I'm happy to offer opinions.

I find the documentation to be ambiguous. --help says "Open new instance, not a new window in running instance", which I agree sounds like the old behavior. The first hit on MDN says "Allows multiple copies of application to be open at a time", which is ambiguous depending on whether it's the 1st or 2nd copy, or both, that has the -no-remote flag. Going by the name of the flag, I would expect the new behavior. The old behavior is clearly useful too, though.

But documentation and flag names can be changed.

Optimally, I would want a way to send a remote command to a particular instance by using a key or a profile name, as well as a way to have one be the default recipient of remote commands.

Assuming comment 9 is accurate about the complexity of that solution, then I'd prefer to have two separate flags: one for "start up a new instance" and another for "do not listen for remote commands".

I agree that the spelling of "start up a new instance" should be -no-remote for backwards compatibility, *if* that was actually the previous behavior on all platforms. It definitely was on X systems. Was it on Windows? bsmedberg says no. I don't have an easy way to test right now. If it wasn't consistent across platforms and now is, I'd rather keep it (and preferably, add an option that does what you want, and make it be consistent.)

(And yes, it would be better to open a new bug, which I probably should have done for this reply.)

Comment 15

[Ryan]

thanks for the opinions, steve! they make sense to me. i've opened bug 716110.