266776 - incorrect window object returned by window.open if forced into same tab

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Dan M]

The object returned to the calling script by the function window.open is
incorrect if you have prefs set to open new windows into the current window.
This probably isn't a huge problem right now because if the calling script
refers to the new window, which has just replaced the old window, the script
will fail pretty quickly, anyway. But it may as well fail on the correct window.
Note this *will* become a problem if bug 264395 is ever fixed.

I don't really have a testcase. I don't think you can tell that something is
going wrong without a debug build. But the current code is clearly wrong.

Comment 1

[Dan M]

Attachment: return correct window object

This seems like the correct code to me (and works for all cases I can think
of). If the requesting opener is known, we load into and return its topmost
window. Otherwise we use the current active tab. window.open and I believe
every codepath currently in use will always have a known opener.

Comment 2

[Johnny Stenback (:jst)]

Comment on attachment 163899 [details] [diff] [review]
return correct window object

       default : // OPEN_CURRENTWINDOW or an illegal value
	 try {
	   if (aOpener) {
+	     newWindow = aOpener.top;

Seems like this should use XPCNativeWrapper() to get .top to ensure you're
getting the real top, and not a value set by the JS in the opener.

	     referrer.spec = Components.lookupMethod(aOpener,"location")
				       .call(aOpener);

Ah, or do what you're doing here...

r=jst with that...

Comment 3

[Dan M]

Comment on attachment 163899 [details] [diff] [review]
return correct window object

Yes, the safe .top lookup method is obviously called for. Thx!

Comment 4

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

Comment on attachment 163899 [details] [diff] [review]
return correct window object

assuming this is restricted to the single window mode case, a=[e-mail]

Comment 5

[Dan M]

Yes, it affects only the ugliest stepsister of single window mode. Patch is in.

Comment 6

[Bradley Chapman (not reading bugmail, still gone but not forever)]

So if I understand this correctly, a call to window.open inside a webpage with
the prefs set to divert all windows into the current tab will NOT return the
original window object, but the window object of the window that does not exist
and is now the current tab?

(Yeah, I know that's a tangled lump)

Comment 7

[Dan M]

Unless I made another mistake, it now returns the correct window, which is the
same window. More clever approaches like returning a new window and letting the
original window spin down on its own are the purview of bug 264395.