In Bug 1053048 nsIEditor was made builtinclass, which caused issues in the Thunderbird-stdlib maintained by Jonathan Protzenko and used, for example, in the Conversations Thunderbird addon. The Editor team kindly reverted that in bug 1059705 but clearly wants a better solution that allows them to make nsIEditor builtinclass. Determine and implement that solution.

bug 1059171 is a good interim fix for lack of a better refactoring of nsMsgCompose

(In reply to Jonathan Protzenko [:protz] from comment #1) > bug 1059171 is a good interim fix for lack of a better refactoring of > nsMsgCompose Would that still mean that you need to implement the nsIEditor you want to set as the value of that attribute in JS though?

No, this would alleviate for me the need to implement an editor in JS.

Sounds great then!

I am not sure this bug is a right place. I wrote a test with an implementation of nsIEditor in JS before. https://bug653342.bugzilla.mozilla.org/attachment.cgi?id=766581 How can be written those mocks with builtinclass nsIEditor in JS?

Looks like that nobody in current c-c implements nsIEditor at testing: https://dxr.mozilla.org/comm-central/search?q=nsIEditor+path%3Atest&redirect=false So, can we make nsIEditor builtinclass again? # I cannot find the test mentioned in comment 5.

Oh, but I found at least one add-ons which is still available in AMO...

Still implementing nsIEditor as JS objects are: https://addons.mozilla.org/en-US/thunderbird/addon/compose-for-thunderbird/ https://addons.mozilla.org/en-US/thunderbird/addon/rss-tab/ https://addons.mozilla.org/en-US/thunderbird/addon/emic/ https://addons.mozilla.org/en-US/thunderbird/addon/rethread/ https://addons.mozilla.org/en-US/thunderbird/addon/gmail-conversation-view/ They use same library, the fine name is "send.js". (Search "FakeEditor" in the addon tree.) Some of them already use new API introduced by bug 1059171. However, some of them have not been updated yet. So, I think that we can make nsIEditor builtin class now. Then, we can get rid of a lot of virtual calls in editor code with using pointers to concrete classes. What do you think? (Should we contact the addon developers first?)

Falling back to the new API: https://addons.mozilla.org/en-US/thunderbird/addon/gmail-conversation-view/ Still using old API: https://addons.mozilla.org/en-US/thunderbird/addon/compose-for-thunderbird/ https://addons.mozilla.org/en-US/thunderbird/addon/rss-tab/ https://addons.mozilla.org/en-US/thunderbird/addon/emic/ https://addons.mozilla.org/en-US/thunderbird/addon/rethread/

And at least in the addon tree, there are no addons which implements nsIPlaintextEditor nor nsIHTMLEditor interface.

Additionally, nsIHTMLAbsPosEditor, nsIHTMLInlineTableEditor, nsITableEditor, nsIEditorMailSupport, nsIEditorStyleSheets, nsIEditorBlobListener and nsIHTMLObjectResizer can be builtin class.

Yeah, I think we should make nsIEditor builtinclass, but informing addons authors early on would be good.

Can I ask a very silly question? What does it mean for TB that nsIEditor becomes a "builtin" class? In fact, what is a "builtin" class? We seem to use nsIEditor in a few places: https://dxr.mozilla.org/comm-central/search?q=nsIEditor&redirect=false Sorry about the ignorance, but it can only become better ;-)

It means the implementation would be C++ only. JS could still use it, but not re-implement.

I also think we should make these interfaces builtinclass. Informing the affected add-ons would be nice, but we should be clear that the notices are informational only (that is to say, we should make this change even at the risk of add-on breakage, and ask the add-on authors to work around it.)

Okay, I emailed to :protz and send message via FB to Inbox Cleaner of expired Emails's author.

Looks like that all the addons are not active since none of them is available on Tb52. So, we can just make nsIEditor and related interfaces builtin classes here. Changing the summary and component of this bug.

As far as I checked, bluegriffon doesn't implement these interfaces too.

Oh, and also we can make nsIEdittingSession a buildin class.

Comment on attachment 8894160 [details] Bug 1060051 - Make editor related interfaces builtin classes if it's possible https://reviewboard.mozilla.org/r/165230/#review170580 ::: commit-message-933a0:3 (Diff revision 1) > +Bug 1060051 - Make editor related interfaces builtin classes if it's possible r?ehsan, smaug > + > +If we make nsIEditor a buildin class, that means that its instance can be only TextEditor or HTMLEditor. Then, users of nsIEditor can use concrete classes such as EditorBase, TextEditor or HTMLEditor instead. Then, the users can reduce unnecessary QI and a lot of virtual calls if we'll create non-virtual methods. s/buildin/builtin/ ::: commit-message-933a0:5 (Diff revision 1) > +Bug 1060051 - Make editor related interfaces builtin classes if it's possible r?ehsan, smaug > + > +If we make nsIEditor a buildin class, that means that its instance can be only TextEditor or HTMLEditor. Then, users of nsIEditor can use concrete classes such as EditorBase, TextEditor or HTMLEditor instead. Then, the users can reduce unnecessary QI and a lot of virtual calls if we'll create non-virtual methods. > + > +So, let's make editor related interfaces buildin classes. builtin

Comment on attachment 8894160 [details] Bug 1060051 - Make editor related interfaces builtin classes if it's possible https://reviewboard.mozilla.org/r/165230/#review170582

Comment on attachment 8894160 [details] Bug 1060051 - Make editor related interfaces builtin classes if it's possible https://reviewboard.mozilla.org/r/165230/#review171452

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/cddc1b05b876 Make editor related interfaces builtin classes if it's possible r=Ehsan,smaug