110561 - -remote does not work with --display

Status: RESOLVED WONTFIX

(Core Graveyard :: X-remote, defect)

Description

[(not currently active) Ted Mielczarek]

If mozilla is running on a different display, using the --display and -remote
options together fails to find a running window.

Example:
start mozilla on display :0.1

in an xterm on display :0.0:
tam4@rawr:~$ mozilla/mozilla -display=:0.1 -remote
"openurl(http://www.lehigh.edu/~tam4)"
mozilla/run-mozilla.sh mozilla/mozilla-bin -display=:0.1 -remote
openurl(http://www.lehigh.edu/~tam4)
MOZILLA_FIVE_HOME=/home/tam4/mozilla
  LD_LIBRARY_PATH=/home/tam4/mozilla:/home/tam4/mozilla/plugins
     LIBRARY_PATH=/home/tam4/mozilla:/home/tam4/mozilla/components
       SHLIB_PATH=/home/tam4/mozilla
          LIBPATH=/home/tam4/mozilla
       ADDON_PATH=/home/tam4/mozilla
      MOZ_PROGRAM=mozilla/mozilla-bin
      MOZ_TOOLKIT=
        moz_debug=0
     moz_debugger=
No running window found.


Workaround:
export DISPLAY=:0.1; mozilla/mozilla -remote "openurl(http://www.lehigh.edu/~tam4)"

Comment 1

[(not currently active) Ted Mielczarek]

I typoed the --display commandline argument in the example, but it is still
broken even if it's spelled right.

Comment 2

[Boris Zbarsky [:bzbarsky]]

blizzard

Comment 3

[Andrew Schultz]

Attachment: patch

enables --display=DISPLAY for mozilla-bin and mozilla-xremote-client.

BTW: mozilla-xremote-client doesn't work from a DEBUG build.  It complains
about NS_CurrentThread being unresolved.  Not really a big deal tho.

Comment 4

[Matthias Versen [:Matti]]

*** Bug 151518 has been marked as a duplicate of this bug. ***

Comment 5

[Chris Moore]

You can reproduce this bug even if you only have a single display.

1)  run mozilla (on display :0.0)
2)  unset DISPLAY
3)  mozilla --display=:0.0 -remote 'openURL(google.com, new-tab)'

You will see "Failed to connect to X server."

'--display' is being ignored.

Comment 6

[Wolfgang Rosenauer [:wolfiR]]

Attachment: extended patch

this new patch extends mozilla-xremote-client to recognize "--display DISPLAY"
as parameter and makes it known to mozilla-bin/firefox-bin/thunderbird-bin
while using the -remote parameter. (hopefully I pinged the correct persons for
r/sr)

Comment 7

[Andrew Schultz]

thunderbird has its own startup script here:
http://lxr.mozilla.org/mozilla/source/mail/app/mozilla.in

Comment 8

[Wolfgang Rosenauer [:wolfiR]]

Attachment: thunderbird script patch (untested)

this patch is untested yet. Will try it soon.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 175910 [details] [diff] [review]
extended patch

I can't sr this since I actually wrote a lot of it. One thing I noticed on
rereading the patch:

+    if (strcmp(argv[i], "--display") == 0) {

We should test argc to make sure that argv[i + 1] is actually there.

+    --display)

Similar here, we need to ensure there actually is a $2.

Comment 10

[Wolfgang Rosenauer [:wolfiR]]

Comment on attachment 175910 [details] [diff] [review]
extended patch

>+  for (i=1; i < argc - 1; i++) {
>+    if (strcmp(argv[i], "--display") == 0) {
>+      display = argv[i + 1];

argv[i+1] should exist because of 'i < argc - 1'?
--display can never be the last argv IMHO.

>+  // save --display parameter for possible -remote handling
>+  // (it will be lost after gtk_init)
>+  char *display = 0;
>+  char *param = 0;
>+  for (int i=1; i<argc; i++) {
>+    if ((PL_strncasecmp(argv[i], "-display", 7) == 0)
>+        || (PL_strncasecmp(argv[i], "--display", 8) == 0)) {
>+      if ((param = PL_strchr(argv[i], '=')) != NULL)
>+        display = PL_strdup(++param);
>+      else
>+        display = PL_strdup(argv[i + 1]);

your argument applies here ;-)

>+    --display)
>+      X_DISPLAY="--display $2"
>+      moreargs="$moreargs \"$1\" \"$2\""
>+      shift 2
>+      ;;

would [ -n "$2" ] && X_DISPLAY... be sufficient?

Comment 11

[Wolfgang Rosenauer [:wolfiR]]

caillon, any comment?

Comment 12

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 175910 [details] [diff] [review]
extended patch

Why can't you just pass in DISPLAY=:0.1 as an environment variable to mozilla? 
We already support that.  Seems like it doesn't really matter one way or the
other.

DISPLAY=foo mozilla-xremote-client
vs.
mozilla-xremote-client -disable=FOO

There's no requirement to support -display and I see no reason to hack this in,
really.  I see the novell scripts are using this patch, but it seems silly IMO.
 Just fix the scripts to use DISPLAY=foo instead of -display=foo

Comment 13

[Wolfgang Rosenauer [:wolfiR]]

If it't not needed then the documentation should reflect this at least.
stark@Hygiea:~> mozilla --help
Usage: /usr/lib/mozilla/mozilla-bin [ options ... ] [URL]
       where options include:

X11 options
        --display=DISPLAY               X display to use

The same in the man-page.

I can live with one or the other solution but I can't find something silly
in this approach. It's the implementation of the documented feature.

Comment 14

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 175910 [details] [diff] [review]
extended patch

Yeah, let's fix the documentation.

Comment 15

[(not currently active) Ted Mielczarek]

I don't actually care anymore.