Freeze nsIBaseWindow or provide compatible alternative. All of our embedding samples use nsIBaseWindow. In fact, it isn't possible to embed Gecko without using this interface. Unfortunately, the interface is not exactly in great shape for freezing. Certainly, we don't want to freeze it with dependencies on nsIWidget because we don't want to freeze that interface. But, yet... if we are to support Eclipse and other embedders going forward, then we need to continue to support nsIBaseWindow (in at least some capacity). I propose copying nsIBaseWindow and calling it nsIEmbedBaseWindow, and then replace nsIWidget with nsISupports. Maybe that'd be sufficient.

Would embeddors ever need the nsIWidget functionality on this interface? That is, can we not remove it entirely?

Yes, embedders need to be able to call nsIBaseWindow::InitWindow, and that function takes a nsIWidget parameter. NOTE: none of the embedders actually pass a non-null nsIWidget value though. However, gtkmozembed does call nsIBaseWindow::GetMainWidget because it needs to get the native widget corresponding to that nsIWidget. I don't think Eclipse uses this API, and none of the embeddings under embedding/tests uses it.

(In reply to comment #2) > Yes, embedders need to be able to call nsIBaseWindow::InitWindow Sure. > and that function takes a nsIWidget parameter. The point is, the version on the embedding interface should not, probably. It can call the same internal function and pass null for the widget and all that. But I can't think of a reason an embedder would want to init a basewindow with an nsIWidget parent. > NOTE: none of the embedders actually pass a non-null nsIWidget value though. Exactly. ;) > However, gtkmozembed does call nsIBaseWindow::GetMainWidget because it needs > to get the native widget corresponding to that nsIWidget. We could probably have a way to get that directly off our interface (similar to how initWindow takes a parent beastie of that sort directly already).

It would be nice if nsIWidget did not appear in any frozen interfaces. It needs to evolve considerably. If embedders want to get a native widget, just give it to them directly as a void*.

It wouldn't be "nice" to avoid nsIWidget, it is an absolute requirement. nsIWidget is not a freezable interface. We need to come up with alternate freezable solutions for getting native widget pointers and munging event loops, if embedders need to call into those tasks. I don't mind the idea of exposing a [noscript] getNativeWidget(out voidptr);

Folks, much to my dismay we are not in a position to be able to change nsIBaseWindow. nsIBaseWindow2 is always an option, but nsIBaseWindow is effectively frozen already. We dropped the ball by not providing an alternative for Moz 1.4, and now we have real embedders to support. (Please see comment #0.) Now, that doesn't mean that nsIWidget needs to remain in the frozen version of the interface. (We must preserve the ABI, which is not equivalent to the compile time API.) We could replace it with nsISupports. We could say that NULL must be passed for the second parameter to InitWindow. We could say that GetMainWidget always returns NULL, provided we determine that none of the existing embedders use that method for anything. (Like I said, I think that Eclipse and other embedders do not -- so we should be safe.) Moreover, we could rename the frozen nsIBaseWindow to nsIEmbedBaseWindow or nsIBaseWindowObsolete (though I prefer nsIEmbedBaseWindow since we don't yet have a replacement for this interface). Preserving the vtable and the UUID of the current nsIBaseWindow is what is important. We could then change the UUID of the nsIBaseWindow used internally, and preserve nsIEmbedBaseWindow as the frozen "base window" interface. Is everyone OK with this plan?

The problem is that we then saddle all embedders going forward with api cruft... I think the right thing to do is to create the api we actually want embeddors to use (more or less nsIBaseWindow, but with nsIWidget removed). Then we declare that the official embedding api. We do what you suggested for the existing api and mark it as deprecated. That way embeddors can use the new, cleaner, api if they want...

OK, is anyone volunteering to create the new-and-improved nsIBaseWindow for embedders in the 1.8 timeframe? ;-) I am not sure that I will have the cycles for that. I will proceed with nsIEmbedBaseWindow, and we can mark it deprecated once we have an alternative.

> Now, that doesn't mean that nsIWidget needs to remain in the frozen version of > the interface. (We must preserve the ABI, which is not equivalent to the > compile time API.) We could replace it with nsISupports. We could say that > NULL must be passed for the second parameter to InitWindow. We could say that > GetMainWidget always returns NULL, provided we determine that none of the > existing embedders use that method for anything. (Like I said, I think that > Eclipse and other embedders do not -- so we should be safe.) These all sound good. Assuming there are no users of parentWidget, that should be defined to always return null too. > Moreover, we could rename the frozen nsIBaseWindow to nsIEmbedBaseWindow or > nsIBaseWindowObsolete (though I prefer nsIEmbedBaseWindow since we don't yet > have a replacement for this interface). Sounds good but I suggest we use nsIBaseWindowObsolete.

> Sounds good but I suggest we use nsIBaseWindowObsolete. Create a frozen alternative, and I'll happily use that name. ;-) It is wrong to deprecate an interface (or call it obsolete) without first providing an alternative.

Here's a patch that basically implements what I suggested. The existing nsIBaseWindow is copied to nsIEmbeddingBaseWindow, and all occurances of nsIWidget are replaced by nsISupports. Finally, the IID of nsIBaseWindow is changed. nsWebBrowser and nsDocShell are modified to implement nsIEmbeddingBaseWindow. nsWebBrowser no longer implements nsIBaseWindow, but nsDocShell implements both interfaces. Right now, I hack it so that nsDocShell::QI returns the same impl for both interfaces.

Drive-by comments: Do we want to document whether it's gecko that implements nsIEmbeddingBaseWindow, or embedders, or either? If embedders can implement it, we should probably say something about what other sorts of interfaces they're likely to need to implement... What units are the coordinates and lengths in? Are setting positions and sizes sync, async, or unspecified? What do |fRepaint| and |repaint| actually do? The documentation on parentWidget says that it corresponds to parentNativeWindow... but it actually corresponds to whatever was passed to initWindow, no? Garbage in, garbage out... What happens when SetVisibility is called on non-toplevel windows? Similar for SetEnabled? And SetBlurSuppression? Note that every single docshell implements this interface, for instance. What does "The implementation should allow for blur events to be suppressed multiple times" mean, exactly?

cvs history suggests that i should ask travis those questions :-/, but maybe... danm: do you have any idea how to answer bz's questions in comment #12 (in particular the one about blur suppression)? thanks!

Are you sure CVS didn't credit Travis merely for adjusting the indentation? Frankly I've become quite fuzzy on the subject of embedding Gecko. I think that nsIBaseWindow, and by extension the new nsIEmbeddingBaseWindow, *can* be implemented by embedd(e/o)rs but that will be unnecessary for all but the most ambitious projects. The best information I know of is the sample embedding projects in the CVS repository. I suggest leaving a discussion of the interfaces required to be implemented to the -- ahem -- documentation. I don't know of any exceptions to the statement that all units in this interface are expressed in pixels. Well... |fRepaint| and |repaint| allow the window to be resized without issuing paint or invalidation events. It's used in a few places, mostly with windows not yet visible. It could be reasonable to at least give the two parameters the same name. > Are setting positions and sizes sync, async, or unspecified? Let's take door #3. All implementations that I know of just toddle off and get it done but I don't want to have to guarantee what happens when we cross the OS border. > ... [parentWidget] actually corresponds to whatever was passed to initWindow I don't follow. We want the caller to supply either a parentWidget or a parentNativeWindow. Embedding apps typically supply the latter, Mozilla the former. > What happens when SetVisibility is called on non-toplevel windows? I'm not certain. Some of these functions are probably redundant. I know that the process of setting visibility on a top-level window involves setting the visibility on both the window widget itself and on its docshell. There's an old comment by Travis in nsXULWindow::SetVisibility wondering about the necessity. Got me. The practice predates both of us. Whether it's still important; good question. > What does "The implementation should allow for blur events to be suppressed > multiple times" mean, exactly? It means it needs to be a counter, not a flag. Feel free to adjust for clarity.

Danm: Thanks for the comments. Much appreciated. I'll try to work those into the documentation for nsIEmbeddingBaseWindow.

We no longer freeze interfaces.