Need to change how window is invoked so it can be overridden by embedding clients. Some details below, more at URL above; danm will be happy to help and answer questions. UI Posed by Gecko (Up Calls) ============================ These are calls to window.open, in one form or another, which are relevant to embedding. They're split into calls and callers, where a call is considered to be a base-level call to window.open which should be API-ized like we're discussing, and a caller is something which currently uses window.open but should use one of these new APIs when they're created. I've tried to categorize these sensibly... Each category implies a UI-posing component with methods for each item marked "CALL" in that category. Items marked "CALLER" in a given category may imply a new method in that component (eg. the print dialog stuff), or may just be a caller of some other extant method in a different component (eg. the uri loader opening a new window). URI Loader: CALLER: /uriloader/base/nsURILoader.cpp#697: in ::GetTarget, create a window based on <a target="..."> for new content

Adding mozilla0.9 keyword, dependency. assigning to locka, who apparently wrote this code.

I think he prefers his Netscape address (?)

Apologies for all the bug spam. For the detailed overview of this task, see danm's document: http://www.mozilla.org/xpfe/embedding-dialogs.html See my first cut at a component-wise categorization: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=27972 And if you want to laugh, see also my brainless incomplete IDL for these components: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=28175 dr

CC'ing Conrad who is currently doing some nsIPrompt work.

-> adam's internal addr.

The way this window gets put up does not have anything to do with nsIPrompt. Currently, the call to window.open being made here ends up in my impl of nsIWebBrowserChrome::CreateBrowserWindow, so as far as embedding goes, this is what we want. The path it takes to end up in CreateBrowserWindow is long (through a content handler in the mozBrowser DLL) which we don't want for embedding.

dan/dr, this case appears to be one in which the CALLER (the URI loader creating a new top-level browser window for a new target) is/should be using nsIWindowCreator, is this correct?

I think the strategy for the up callers should be to write some UI components in embedding/components to represent each dialog. Each UI components is an XPCOM object implementing an interface as hashed out by Dan, and registered with a component category. Embedders can substiture their own dialog in place of the default one by with their own component with the same category. I'm working through separating the download progress UI out of the streamxfer object to see if the idea is feasible.

We created these bugs based on more a string search than a careful analysis of the use of each line of code that caught our attention. (Though I hasten to say there was some analysis involved.) On closer inspection, this particular window.open looks less like a potentially problematic dialog; more like the legitimate creation of a new browser window. As such, it can probably stand as is. However, Adam's strategy analysis above is right on, so he obviously gets what we're trying to do and I'll leave him to it. (One point: I wasn't imagining moving the implementations for all these UI-controlling components into the embedding directory. I look at them as still belonging more to the code whose UI they define than to the embedded world. I was thinking they'd be happier staying with the component whose UI they were controlling. Of course in the binary distribution, they all get lumped into the same directory as everyone else.)

My suggestion for putting the UI components into embedding is that it makes it more obvious to embedders which ones they must to override. If we spread them over the length and breadth of the source it becomes much more difficult to find them. Having said that, it does complicate matters for us because we must modify two modules instead of one whenever something needs to be changed in one of the dialogs.

CC'ing law@netscape.com Bill, I understand you were doing some work to the unknown content handler dialogs. Can you refer me to the bug nr so we don't work at cross-purposes? Thanks

This patch splits out the UI part of the stream transfer object into a new object. It demonstrates how we'd likely have to do all the dialogs. Communication is done between the stream transfer object and the UI via an nsIDownloadProgressUI and nsIDownloadProgressCallback interface. The stream transfer object creates the UI via its contract id. Note that the patch needs some cleanup and won't compile yet since I have just moved the UI part from embedding/components into xpfe/components/xfer so the makefiles needs to be fixed up yet.

Adam, my recommendation would be to not invest further effort into this particular patch. I'm already doing all that to generalize the download progress dialog code so that a single implementation can be shared in place of the two implementations we have currently. That will also move the code into the embedding directory, a requirement identified during discussion of the nsIHelperAppLauncherDialog API a while back. That prompted bug 70228. That bug doesn't say too much (and some of what it does say is erroneous, I believe). A better place to find out about work on the helper app dialog is bug 52454. The code I have written for that bug, in embedding/components/ui/helperAppDlg, is a reaonable example of packaging up dialogs/UI behind an interface, I think. I'm doing the same thing with the download progress dialog.

Do you want to take this bug Bill?

No, at least not until somebody can tell me what the hell it's all about. It started off talking about the uriloader opening new windows for targetted links, I think. How did the subject turn to download progress dialog?

Changing the summary to reflect what this bug is about :) Basically all the dialogs that can be thrown up by Gecko need to be overridable by embedders. The most obvious way to achieve this is to make them all XPCOM objects so that embedders can register their own objects instead.

This bug was only intended to cover one such dialog, the larger issue is covered by tracking bug 65233.

Bill, does this bug sound like it meshes with your work now it's been renamed?

Bill, scratch my last question , I just saw a comment from Peter that I need to clarify first. Peter, apologies I didn't realise this bug was specific to a particular up calling instance. The next question is where is this up calling instance? The code I wrote in GetTarget opens a new window but there is no hardcoded chrome URL or anything in there so I'm not sure what the issue is. Do you want me to use some other method to open the window?

cc danm

Changing summary from general case back to the specifics of this bug.

Adam: no need to morph this bug into taking care of the whole problem. There are other bugs for that (see tracking bug 65233); in this one we were worried only about the particular window.open in nsURILoader.cpp. It must be the one currently at line 773. Again, as I've found myself saying kind of often, we may have been too aggressive identifying this one as a problem area. I'm looking at it now. If, as it appears, it's a legitimate case of opening a new (browser?) window in which to load a URI, there's probably nothing that need be done. I don't quite get what's going on here. Opening a new window with no URL? Anyway, as you ably pointed out in a comment above, April 9th, what we're trying to accomplish here is to define, implement and use an API to open windows, where appropriate, rather than opening a window directly. "Where appropriate" means, of course, if the window is really a dialog that an embedding app could conceivably want to replace. If that's not the case with this mysterious URL-less window, close the bug WFM.

See bug 72491 and bug 41241. The reason for the window.open in uriloader is to provide a valid window context in which to push http data. This code prevents two consecutive http channels from being opened. The first channel would be aborted because there was no window context. The second would be opened after the new window was eventually opened. This is bad enough in normal circumstances, but for forms it meant they were submitted twice and the second channel was always a GET operation even if the first contained postdata.

So, sounds like this was a red herring, as long as the window is hidden and doesn't affect any of the embedder's UI. Can we close it out?

It shouldn't affect the embedders because all the JS calls eventually route through to the embedders CreateBrowserWindow method to do the opening. I'll mark it WONTFIX. Please reopen if any issues arise from the code.

reassign qa contact to depstein.

verifying