The patch attached to this bug refactors the initialization steps for the plain text editor by adding the EnsureEditorInitialized API in order to facilitate the work on lazily initializing the editor in bug 221820.

Er, which attached patch? :-) Or did you mean "to be attached"?

(In reply to comment #1) > Er, which attached patch? :-) Or did you mean "to be attached"? Well, splitting the patch took longer than I expected, so I thought I'd file the bug and attach the patch later. Little did I know that I would need to edit the description for it to make sense... :-) I'll attach the patch as soon as it's ready.

This patch introduces the EnsureEditorInitialized and makes the current delayed initialization use that API. The order of execution remains the same (which means that the editor is still initialized lazily with this patch.)

(In reply to comment #3) > This patch introduces the EnsureEditorInitialized and makes the current delayed > initialization use that API. The order of execution remains the same (which > means that the editor is still initialized lazily with this patch.) I thought it was still supposed to be initialized eagerly with this patch? + // never get used. So, now this method is being called lazily only + // when we actually need an editor. So this comment is invalid.

(In reply to comment #4) > (In reply to comment #3) > > This patch introduces the EnsureEditorInitialized and makes the current delayed > > initialization use that API. The order of execution remains the same (which > > means that the editor is still initialized lazily with this patch.) > > I thought it was still supposed to be initialized eagerly with this patch? Well, yes, I was thinking eagerly, typed lazily. Sorry for the confusion! > + // never get used. So, now this method is being called lazily only > + // when we actually need an editor. > > So this comment is invalid. You're right. I moved this comment to the patch which makes the initialization lazy.

Comment on attachment 424716 [details] [diff] [review] Patch (v1.1) >+++ b/layout/forms/nsTextControlFrame.cpp >- if (IsFocusedContent(GetContent())) This slightly changes the logic, in particular to not call SetFocus if editor init errors out. I _think_ that's ok, but double-check? r=bzbarsky

(In reply to comment #6) > (From update of attachment 424716 [details] [diff] [review]) > >+++ b/layout/forms/nsTextControlFrame.cpp > >- if (IsFocusedContent(GetContent())) > > This slightly changes the logic, in particular to not call SetFocus if editor > init errors out. I _think_ that's ok, but double-check? You're right. I think it's OK, but I don't know how to make the editor initialization fail (except for manually returning a failure code) to test this in real life. Do you have any ideas how to test this?

Hmmm. Editor init fails in these cases: 1) Failed createInstance on the editor. 2) The presshell has no document. 3) Calling Init() on the editor fails. 4) Calling GetControllers on the input fails. 5) Getting an nsIControllerContext controller fails. 6) GetFlags on the editor fails. 7) SetFlags on the editor fails. 8) EnableUndo fails. In practice, 1, 2, 6, 7 can't happen sanely. I don't know about 3, 4, 5, 8, but I bet they can't either...

I wrapped the code in an object's dtor to preserve the current behavior.

Comment on attachment 425876 [details] [diff] [review] Patch (v1.2) That would run the code after the scriptblocker's destructor. It needs to run before. In fact, you want to move the new block to down below the PushNull() call.

Comment on attachment 426259 [details] [diff] [review] Patch (v1.3) r=bzbarsky

I backed this out because I suspect that this might have caused a crash on Windows mochitests: http://hg.mozilla.org/mozilla-central/rev/132cbfefc488 Failure log: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1266532728.1266535683.5721.gz&fulltext=1#err0

Backed out again as part of http://hg.mozilla.org/mozilla-central/rev/309acf8ab20f.

See bug 557689 comment 11 for more information. http://hg.mozilla.org/mozilla-central/rev/3dcfd44195d6 http://hg.mozilla.org/mozilla-central/rev/3c1ac0bbeb52