Editor needs to be able to hang editor data off each docShell, so that we can handle editing framesets, iframes etc. DocShell will hold the owning reference to an editor on each editable subframe. There will be a top-level nsIEditingSession (implemented in composer code) which clients use to address specific editors. Patches coming.

So here's how this works. nsDocShell implements a new interface, nsIEditorDocShell, which is simply a private interface (should it be nsPIEditorDocShell?) which allows you to set a flag on a docShell to make it editable (either now, or when a URL loads), to get an nsIEditor from that docShell, and to test if it has an nsIEditingSession (which will only be the case for the content-root docShell). You get to a nsIEditorDocShell by doing a QI from nsIDocShell. nsDocShell internally stores editor-related data in a C++ object, nsDocShellEditorData, which is only allocated when a docShell has been made editable. Thus, I'm introducing just an extra 4-bytes per docShell of new overhead. nsDocShellEditorData contains the 'editable' flag, and nsCOMPtrs to an nsIEditingSession and/or nsIEditor. These are owning references. nsDocShell's implementation of nsIEditorDocShell mostly just passes the calls through to nsDocShellEditorData. nsDocShell also now supports GetInterface() of an nsIEditingSession, which is the interface that clients will use to actually make frames editable (using nsIDOMWindows to refer to frames). You can see nsIEditingSession at: <http://lxr.mozilla.org/seamonkey/source/editor/composer/public/ nsIEditingSession.idl> and the implementation at: <http://lxr.mozilla.org/seamonkey/source/editor/composer/src/ nsEditingSession.cp> It is intended that nsIEditingSession will be a public interface. I'm looking for r= and sr= on the mozilla/docshell changes.

nsIEditorDocShell.idl is fine, no need for 'P'. nsIEditingSession.idl - * predef of nsIEditingShell isn't needed. * drags in nsIEditor. do we need to make nsIEditor public? If so, we need to clean it up quite a bit (idl'ize, enums -> java style enums -> remove dependencies on non-public ifaces, etc). Another option would be to break nsIEditor apart into internal stuff we don't need (nsIEditorInternal), and just the stuff we want to expose (nsIEditor). That's a common practice that elleviates some refactoring.

FYI mjudge will be landing some changes that XPIDL'ize most of the low-level editor interfaces including nsIEditor.

so -- where do we stand on this bug? Jud are you r= on this one? Simon -- who should sr= this one?

well, nsIEditor can't go public as it is (idl'ized or not), should I file a separate bug for that? r=valeski on the nsIEditingSession.idl after the nsIEditorShell iface predef is pulled. I'm inclined not to even r= that though as it drags in nsIEditor in it's dirty form, but if we want to break that into another bug I'm ok w/ the r=. the docshell mods look good to me, r=valeski on those. I can't speak for the editorData stuff though.

Can we just say that nsIEditingSession is not a public interface for now, and bless it and nsIEditor together later?

sure can.

So do I have an r= on the nsIEditorSession stuff, with the proviso that it is currently not public, but it and nsIEditor will become so later?

r=valeski

-> 0.9.1

I have a few small points but r=adamlock@netscape.com anyway for everything: 1. nsDocShellEditorData.cpp says 4 space indentation in the comment, but it's really using 2 spaces 2. nsDocShellEditorData::SetEditor() is comparing interface pointers. Is there any chance this might fail for example if one of the objects were actually a proxy? 3. The docshell diff has some code below a comment that says "delete any editor data". It doesn't appear to test if mEditorData is nsnull or not before delete'ing it. Is this right?

> I have a few small points but r=adamlock@netscape.com anyway for everything: > > 1. nsDocShellEditorData.cpp says 4 space indentation in the comment, but it's > really using 2 spaces Will fix. > 2. nsDocShellEditorData::SetEditor() is comparing interface pointers. Is there > any chance this might fail for example if one of the objects were actually > a proxy? It could, I guess, tho I don't know of any code that makes proxies to editors. Would I have to implement an "IsSelf(nsIEditor* inEditor)" method to do this properly? This seems like overkill. > 3. The docshell diff has some code below a comment that says > "delete any editor data". It doesn't appear to test if mEditorData is > nsnull or not before delete'ing it. Is this right? C++ standard says that |delete| is safe to call on a null pointer. The lack of a null check is deliberate.

For 2) I don't you should worry too much if you know that the nsIEditor stuff will happen on the same thread as the docshell and that there are no proxy wrappers for anything such as XPConnect.

1.0

Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 (you can query for this string to delete spam or retrieve the list of bugs I've moved)

->mjudge

Someone want fix up the target milestone to 1.0 ? since 66296 is target at 1.0 and this is a dependant bug.

I think this was probably checked in on Friday

This landed with the editor embedding stuff.