109380 - Remove dependency of layout components on nsIEditorShell

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Charles Manske]

When a XUL "editor" element is created, nsEditorBoxObject::Init() 
[layout/xul/base/src/nsEditorBoxObject.cpp] calls nsIEditorShell.Init(). 
Not much goes on in that method, so it shouldn't be too hard to remove that 
dependency.
For embedding application, layout should only require nsIEditor, nsITextEditor,
or nsIHTMLEditor. nsIEditorShell should only be used for the Composer Application.

nsHTMLInputElement and nsHTMLTextAreaElement in the "content" component are 
dependent on nsIEditorController. I'm not sure if that's also an issue.

Comment 1

[Kathleen :Brade]

I think we should fix this bug sooner since it will help reduce dependencies and
hopefully reduce bloat.
Request reconsideration.

Comment 2

[Håkan Waara]

Can this be considered for 0.9.9? The editor shell is a big fat monster, that
has hard-coded dependencies on all sorts of HTML/Plaintext specific stuff.
Akkana told me that there are plans to remove it altogether, is that what this
bug is for - or is this bug just a first step towards that goal?

Comment 3

[Akkana Peck]

I really hope we'll do the editor shell removal (or at least big steps in that
direction) for 0.9.9.  But it may depend on what other bugs are on the schedule.

Comment 4

[Kathleen :Brade]

-->Akkana

Comment 5

[Syd Logan]

plussing

Comment 6

[Akkana Peck]

This isn't going to make 0.9.9.

Comment 7

[Akkana Peck]

Attachment: Patch: necessary but not complete

Here's the first thing that needs to be done: add an accessor for editor (vs.
editorShell) in nsXULBoxObject so that clients (including us) can do the right
thing and use editor rather than editorShell.

We can't remove the editorShell accessor yet; lots of clients still depend on
it (including our own editor, and mailnews).

I can't remove the internal use of editorShell inside the box object yet,
because nsEditorShell::Shutdown() includes code to shut down things like the
spellchecker, webprogress listener, mouse listener (that last one will go away
with Charley's latest change) and we need a story for how those objects will be
shut down.

However, I'd like to get the simple box object API change in soon, so that
other editorshell-excising changes in other places (editor and mailnews) can
use the new accessor.  Seeking review.

Comment 8

[Kathleen :Brade]

Comment on attachment 73477 [details] [diff] [review]
Patch: necessary but not complete

r=brade

Comment 9

[Simon Fraser [no longer active]]

Comment on attachment 73477 [details] [diff] [review]
Patch: necessary but not complete

Longer term, nsEditorBoxObject won't be the thing that owns the
editorShell/editor. The embedding work that me/mike did push editor ownership
onto docShell, and you'll get to the editor via nsIEditingSession (gotten via a
GetInterface from an nsIWebBrowser). However, we can't go there now, because
nsEditorShell is not part of that picture.

So these changes are Ok for now. sr=sfraser

Comment 10

[Simon Fraser [no longer active]]

Question: does the XBL for <editor> allow access to the nsIEditor now?

Comment 11

[Akkana Peck]

Before the patch: clients can get editorshell, then get the nsIEditor from that.
After patch, they can get it directly, without going through the editorshell.

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 73477 [details] [diff] [review]
Patch: necessary but not complete

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 13

[Akkana Peck]

The main fixes are in: embeddors can access nsIEditor and the other editor APIs
directly and need not go through the editorshell.

Actually removing the editorshell at this stage might be risky (there's a lot
more work to do).  Removing + from nsbeta1 and topembed for reconsideration of risk.

Comment 14

[Dave Barrowman]

topembed- per EDT triage.

Comment 15

[Asa Dotzler [:asa]]

so the 3/10/02 patch has landed? can you mark the patch as obsolete so it
doesn't look like an approved patch waiting to land? thanks.

Comment 16

[Akkana Peck]

Comment on attachment 73477 [details] [diff] [review]
Patch: necessary but not complete

Marking the patch obsolete since it has landed.

Comment 17

[Akkana Peck]

What's left to do here: the only dependency now is that layout's
nsEditorBoxObject creates an editor by creating an editor shell.  It should use
the embedding API, and never know about the editorshell.

However, that part can't happen until all users of nsIEditorBoxObject have
stopped using its editorshell attribute, because the box object can't provide an
editorshell if it doesn't know what that is.  I've added a dependency for
ridding the editor of editorshell; will file a new bug shortly for mailnews.

Comment 18

[scottputterman]

Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt.  If you have any
questions about this, please email [e-mail].  You can search for
"changing adt3 bugs" to quickly find and delete these bug mails.

Comment 19

[scottputterman]

Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt.  If you have any
questions about this, please email [e-mail].  You can search for
"changing adt3 bugs" to quickly find and delete these bug mails.

Comment 20

[Kathleen :Brade]

seeking reconsideration due to possible bloat reduction

Comment 21

[Judson Valeski]

minusing for topembed. if we're close to the cleanup that akkana describes in
comment #17, please remove the minus for reconsideration.

Comment 22

[Akkana Peck]

Changing milestone to reflect realities of bug 122922.

Comment 23

[Akkana Peck]

Pushing out milestone because of dependencies.

Comment 24

[Akkana Peck]

Per discussion with Kathy: we're expecting to remove the box object, so the
remaining parts of this bug are now moot.  Resolving as fixed for the parts it
already fixed.

Comment 25

[Kathleen :Brade]

verified via lxr