Tested on the following: Win2000 - Mfcembed 07_18_08_1.0.1 Mac OSX - PPembed 07_19_05_1.0.1 Win2000 - Mfcembed 07_18_13_trunk Win2000 - Netscape 7.0 PR1 Win2000 - Netscape 07_08_13_trunk Steps to reproduce: 1. Open the following page: http://www.hbo.com/screening. 2. View a clip by clicking on 'Watch with Quicktime' or 'Watch with RealPlayer' Expected result: A new window is generated and runs the clip Actual result: A new window is generated but it is blank Problem is reproduced in the embed applications ONLY. Netscape works fine. This is not a plug-in bug. The following line is the culprit (from the video_site.js file): newWindow = window.open("","","width=360,height=400,RESIZABLE=NO,SCROLLBARS=NO"); Although the window is generated, the 'newWindow' variable is not set. Therefore, the 'newWindow' variable is not recognized in the javascript following it and the HTML doesn't get read and consequently the plug in does not get called. Attached is a simplified testcase (.html and accompanying .js files) which does not involve a plug in which demonstrates the problem.

|window| is DOM, not JS Engine ---> DOM Level 0. Christine: do you still see the bug in the standalone testcase I've just attached? That is, we can just put all the JS in-line to see the bug, right? It doesn't need to be an external JS file - ? Is there a JavaScript Console in the embed builds you are testing? If so, do you see any errors in it when the test fails? More generally, what failure does the user experience in the tescase? The HTML content just doesn't get written into the title and body of the child window? It just comes up blank?

Phil - I used your standalone testcase on the following: Win2000 - Mfcembed 07_18_08_1.0 Win2000 - Mfcembed 07_18_13_trunk Reproduced problem - did not get a Javascript alert; the new window was created but is totally blank. The user on the HBO page gets a similar experience. If they click on Quicktime or RealPlayer to view the clip, the new window is generated but it is totally blank. No Javascript console in the embed app unfortunately.

This behavior seems very reminiscent of this bug: http://bugzilla.mozilla.org/show_bug.cgi?id=88229 but that testcase still works for MFCEmbed. Still, I thought I would pass that along if only to help show where the problem is not located.

Nominating for topembed

cc'ing chak. perhaps there's some parameter passing problem w/ the windowing APIs.

David, please track this for possible API bug regression suite inclusion.

plugin folks

What is the next step here?

Sorry, i just saw this bug.. To Christine/Phil : There is a debug console (not a JS console) in MfcEmbed. Please run "mfcembed -console" to see if any error messages there can help us debug...thanks

I don't have any of the embed apps. cc'ing Mazie: are you able to see any errors when you run "mfcembed -console"? Thanks -

I am using MfcEmbed with Gecko 08/19/02. Attached are the error messages I get by typing "mfcembed -console."

Just commented newWindow.moveTo(10,10) from the attachment id (id=92007) and it worked. I don't know why removing moveTo makes it work. Created attachment with moveTo() commented.

In my test scripts, I noticed that the "NewWindow" object isn't recognized in MfcEmbed & PPEmbed (works fine in Mozilla builds). So If we use it for something like NewWindow.QueryInterface, it won't work. But when the JS 'window' object is used, i.e. window.QueryInterface, then it works. JS scripts that aren't using returned objects from window.open are working fine in MfcEmbed & PPEmbed.

> JS scripts that aren't using returned objects from window.open... We've had these bugs filed recently against the DOM, re the returned window object from the window.open() method: bug 52238 (marked Invalid for some reason - ?) "window returned by open/openDialog doesn't provide handle to valid JS context" bug 142555 Same issue bug 164975 "script execution stops accessing new window property; timing-related?" bug 159424 "Objects cannot be passed between windows. Scope seems to be changed" bug 165053 "getElementById() fails on generated content"

discussed in edt-jst can you comment on progress?

The first hunk of this patch is the fix for this bug, the rest is fixing a potential memory leak.

So the reason for this not working properly in mfcembed is that the document for the new window is not created early enough. The actual problem comes from the call to newWindow.moveTo() and that call fails since we fail to find a principal for the new window when checking if the call to newWindow.moveTo() should be allowed or not. The fix is to make sure new windows created from JS always have a document when they're returned from window.open().

Comment on attachment 110427 [details] [diff] [review] Fix. Make sure there's always a document in a window returned from window.open() >Index: dom/src/base/nsGlobalWindow.cpp >=================================================================== >- return OpenInternal(url, name, options, PR_FALSE, nsnull, 0, nsnull, >- _retval); >+ rv = OpenInternal(url, name, options, PR_FALSE, nsnull, 0, nsnull, _retval); >+ >+ nsCOMPtr<nsIDOMChromeWindow> chrome_win(do_QueryInterface(*_retval)); >+ >+ if (NS_SUCCEEDED(rv) && !chrome_win) { Why not check for the result ( rv ) before querying for chrome_win? That is, rv = OpenInternal(...); NS_ENSURE_SUCCESS(rv, rv); nsCOMPtr<nsIDOMChromeWindow> chrome_win(....); ... > if (NS_SUCCEEDED(rv)) { >- nsCOMPtr<nsIWindowWatcher> wwatch(do_GetService(NS_WINDOWWATCHER_CONTRACTID, &rv)); >+ nsCOMPtr<nsIWindowWatcher> wwatch = >+ do_GetService(NS_WINDOWWATCHER_CONTRACTID, &rv); >+ Why go from one style to another? Just curious :-) r=harishd

Comment on attachment 110427 [details] [diff] [review] Fix. Make sure there's always a document in a window returned from window.open() >@@ -4331,51 +4353,47 @@ > escapedURL = unescapedURL; > } > >- if (!escapedURL.IsEmpty()) >- url = ToNewCString(escapedURL); >+ if (!escapedURL.IsEmpty()) { >+ url.Adopt(ToNewCString(escapedURL)); >+ } CopyUCS2toASCII(escapedURL, url); looks nicer imho. >+ rv = pwwatch->OpenWindowJS(this, url.get(), name_ptr, options_ptr, >+ aDialog, extraArgc, argv+3, Add some spaces around the + while you're there.

Harish, I didn't bother checking rv before QI'ing since OpenInternal() doesn't fail in the common case, so performance wise it doesn't matter, and it's less code the way I wrote it (fewer brances). And I really don't want to use NS_ENSURE_SUCCESS() there either since OpenInternal() *can* fail for valid reasons that should *not* cause the warning that NS_ENSURE_SUCCESS() generates if rv is an error code. And I changed from one style to the other since that line didn't fit in 80 columns, and there's just no really readable way to split up the constructor syntax on more than one line. Fix checked in. FIXED.

Comment on attachment 110427 [details] [diff] [review] Fix. Make sure there's always a document in a window returned from window.open() >Index: dom/src/base/nsGlobalWindow.cpp >=================================================================== >- return OpenInternal(url, name, options, PR_FALSE, nsnull, 0, nsnull, >- _retval); >+ rv = OpenInternal(url, name, options, PR_FALSE, nsnull, 0, nsnull, _retval); >+ >+ nsCOMPtr<nsIDOMChromeWindow> chrome_win(do_QueryInterface(*_retval)); >+ >+ if (NS_SUCCEEDED(rv) && !chrome_win) { Why isn't the early about:blank generation trick applied to chrome windows?