25040 - Opening content in an existing window with window.openDialog fails to send arguments

Status: RESOLVED FIXED

(Core :: XUL, defect, P2)

Description

[Ben Goodger (use ben at mozilla dot org for email)]

I wish to allow the user to open only one instance of the pref dialog.

currently I get this behaviour automatically since the name for the window I 
supply to openDialog is the same:

var prefWindow = window.openDialog("chrome://pref/content/pref.xul", 
"PrefWindow", "chrome,modal=yes,resizable=yes", pane );

so every time the user tries to open the dialog from other points in the app, a 
new window isn't created, the old one is loaded.

The problem here is that in this case, window.arguments for the window seem to 
be destroyed. What should happen is the new arguments passed from the subsequent 
call to openDialog should replace the arguments currently on the window.

I am about to check in a fix to prefwindow that makes it modal, single instance 
only that displays this problem. To test:

1) load mozilla, and mozilla mail
2) load prefs from mozilla
3) switch to mail, load prefs. the prefwindow opened by mozilla should appear.
4) look in the console. see text about window.arguments not being found. this is 
because a test of ( window.arguments ) fails on this second call to open the 
dialog.

Comment 1

[Ben Goodger (use ben at mozilla dot org for email)]

I wouldn't say this is a beta1 blocker, since it is somewhat of a fringe 
case, but here's what'd happen:

you'd load prefs from a navigator window, pref dlg would come up with the 
navigator start page...

if you also had a messenger window open, a call to prefwindow would focus the 
previously opened prefwindow, but the panel inside would not change to the 
messenger start panel, instead, window.arguments would be undefined and you'd go 
into the error condition in the prefwindow code where the default panel (or 
whatever) is loaded. 

Comment 2

[Peter Trudelle]

Mass-moving all M16 non-feature bugs to M17, which we still consider to be 
part of beta2

Comment 3

[Paul MacQuiddy]

spam, open xptoolkit qa contact moving over to jrgm

Comment 4

[John Morrison]

Problem still exists in a current build (2000050412 win95). 

ben/danm -- is this feature something you need/want for nsbeta2?

Comment 5

[Ben Goodger (use ben at mozilla dot org for email)]

sure what the hell. I'll nominate it for beta3. I'm not desperate for it but it
allows for 4.xP (IIRC - but it is a minor detail). So, if you're out of more
pressing nsbeta3 stuff and looking for something to do...

Comment 6

[Peter Trudelle]

nsbeta3-

Comment 7

[Peter Wilson]

I have a widget toolbar opened in a xul window. The is window is accessed from a 
menu item with: 
    var widgetbar = openDialog("chrome://xulapp/content/widgetbar.xul", "widget 
bar", "resizable=no,dependent,chrome,dialog=yes,titlebar",xulapp);

First time through everything is OK.
If the widgetbar becomed obscured and the user reselects this menu item then:
1. All program variables in the existing window become undefined.
2. The window.arguments are also undefined.
What is a poor programmer to do?
Any chance that you can fix this? It has been around a long time.

Comment 8

[timeless]

i would think you could switch your argument passing arch.

var w=openDialog(...);
w.push(arguemnts); //or something.

note that this would be probably be a workaround and not an acceptable 
solution.

Comment 9

[Boris Zbarsky [:bzbarsky]]

This blocks a 4xp view-source bug...

Comment 10

[Boris Zbarsky [:bzbarsky]]

So the problem here is that we AttachArguments() and then LoadURI().  As the URI
loads, GlobalWindowImpl::SetNewDocument() is called.  This sees that we already
have a doc and helpfully clears the JS scope, clobbering our brand-new arguments
property.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Possible "fix"

This fixes the problem... not quite sure how it interacts with the plugin stuff
jst put in place (the code that triggers when mDocument is null and aDocument
is null in nsGlobalWindow::SetNewDocument).

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 90447 [details] [diff] [review]
Possible "fix"

The plugin stuff I was worried about is gone (removed in bug 199033, revision
1.580 of nsGlobalWindow.cpp).  jst, danm, would you review?

I'll add a comment as to why this is needed before checking in.

Comment 13

[Johnny Stenback (:jst)]

Comment on attachment 90447 [details] [diff] [review]
Possible "fix"

sr=jst with a comment explaining this.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Checked in for 1.8a5.