1489939 - Replace nsIEditorMailSupport with mozilla::HTMLEditor

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Makoto Kato [:m_kato]]

Except to editor/ui/composer/content/ComposerCommands.js, comm-central shouldn't use nsIEditorMailSupport since C++ code can uses mozilla::HTMLEditor directly.

Comment 1

[Makoto Kato [:m_kato]]

(Patch will be coming after landing bug 1478546)

Comment 2

[Jorg K (CEST = GMT+2)]

Thanks!

Comment 3

[Makoto Kato [:m_kato]]

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1ab6b0db7129ccb8126716cf9edf88d87d8a7a81

Comment 4

[Makoto Kato [:m_kato]]

Attachment: Replace nsIEditorMailSupport with mozilla::HTMLEditor

Comment 5

[Makoto Kato [:m_kato]]

Comment on attachment 9008305 [details] [diff] [review]
Replace nsIEditorMailSupport with mozilla::HTMLEditor

The composing area of Thunderbired (and Seamonkey) isn't <textarea> or <input type=text>, and it is HTMLEditor instance. (Actually nsIEditorMailSupport is implemented in HTMLEditor only). So mailnews should use mozilla::HTMLEditor instead of nsIEditorMailSupport.

Comment 6

[Jorg K (CEST = GMT+2)]

Comment on attachment 9008305 [details] [diff] [review]
Replace nsIEditorMailSupport with mozilla::HTMLEditor

Review of attachment 9008305 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for moving this forward, much appreciated. r+ with questions :-)

::: mailnews/compose/src/nsMsgCompose.cpp
@@ +722,5 @@
>        //we cannot add it on top earlier, because TagEmbeddedObjects will mark all images in the signature as "moz-do-not-send"
>        if( sigOnTop )
>          MoveToBeginningOfDocument();
>  
>        if (aHTMLEditor && htmlEditor)

Can |&& htmlEditor| be removed here?

Also two lines below we have:
else if (htmlEditor). Could that be replaced by just an else?

::: mailnews/compose/src/nsMsgCompose.h
@@ +57,5 @@
>                                                       const char * originalHost,
>                                                       const char * originalPath,
>                                                       mozilla::dom::Element * element);
>    nsresult                      ResetUrisForEmbeddedObjects();
> +  nsresult                      TagEmbeddedObjects(nsIEditor *aMailEditor);

Nit: Please change the name of the formal parameter.

Comment 7

[Makoto Kato [:m_kato]]

(In reply to Jorg K (GMT+2) from comment #6)
> Can |&& htmlEditor| be removed here?
> 
> Also two lines below we have:
> else if (htmlEditor). Could that be replaced by just an else?

Yes, I remove and replace it.
 
> ::: mailnews/compose/src/nsMsgCompose.h
> @@ +57,5 @@
> >                                                       const char * originalHost,
> >                                                       const char * originalPath,
> >                                                       mozilla::dom::Element * element);
> >    nsresult                      ResetUrisForEmbeddedObjects();
> > +  nsresult                      TagEmbeddedObjects(nsIEditor *aMailEditor);
> 
> Nit: Please change the name of the formal parameter.

Yes, thanks.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/5237216b5de5
Replace nsIEditorMailSupport with mozilla::HTMLEditor. r=jorgk

Comment 9

[Jorg K (CEST = GMT+2)]

I prefer to do the landings myself. You pushed onto a busted tree.