Replace nsIEditorMailSupport with mozilla::HTMLEditor

RESOLVED FIXED in Thunderbird 64.0

Status

defect
RESOLVED FIXED
10 months ago
2 months ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

(Blocks 2 bugs)

unspecified
Thunderbird 64.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Except to editor/ui/composer/content/ComposerCommands.js, comm-central shouldn't use nsIEditorMailSupport since C++ code can uses mozilla::HTMLEditor directly.
(Patch will be coming after landing bug 1478546)
Thanks!
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.
Attachment #9008305 - Flags: review?(jorgk)
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.
Attachment #9008305 - Flags: review?(jorgk) → review+
(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.
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/comm-central/rev/5237216b5de5
Replace nsIEditorMailSupport with mozilla::HTMLEditor. r=jorgk
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
I prefer to do the landings myself. You pushed onto a busted tree.
Target Milestone: --- → Thunderbird 64.0
You need to log in before you can comment on or make changes to this bug.