1478546 - Move HTMLEditor::GetEmbeddedObjects (nsIEditorMailSupport.getEmbeddedObjects) to comm-central

Status: RESOLVED FIXED

(MailNews Core :: Composition, task)

Description

[Makoto Kato [:m_kato]]

Firefox (and bluegriffon) doesn't use GetEmbeddedObjects method and it is used on Mail composer.  So we should move this from Gecko's core to mailnews/compose.

Comment 1

[Makoto Kato [:m_kato]]

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&author=[e-mail]&selectedJob=194801797

Comment 2

[Jorg K (CEST = GMT+2)]

Thanks! The failures on your try run are expected, see https://mzl.la/2gS72WO :-( - Currently working on fixing the two most prominent ones, the "content policy" and the "a11y".

Comment 3

[Makoto Kato [:m_kato]]

Attachment: Move HTMLEditor::GetEmbeddedObjects (nsIEditorMailSupport.getEmbeddedObjects) to comm-central

Comment 4

[Makoto Kato [:m_kato]]

Comment on attachment 9003702 [details] [diff] [review]
Move HTMLEditor::GetEmbeddedObjects (nsIEditorMailSupport.getEmbeddedObjects) to comm-central

I would like to move nsIEditorMailSupport.getEmbeddedObjects to comm-central since this isn't used from JavaScript on Firefox, Thunderbird and BlueGriffon.

Comment 5

[Jorg K (CEST = GMT+2)]

Thanks for looking into this. Maybe it's time for a little clean-up.

GetEmbeddedObjects() returns nsGkAtoms::img, nsGkAtoms::embed, nsGkAtoms::a and nsGkAtoms::body.

Mailnews processes 'img', 'a', 'body' and 'link'!
https://searchfox.org/comm-central/rev/39cff66145fdfbfa785202c5b8d1c72a06034034/mailnews/compose/src/nsMsgSend.cpp#1287
nsMsgCompose.cpp processes 'img', 'a' and 'link'.

So I think it's about time not to return 'embed' any more since it makes no sense. What do you think? That we check for 'link' which is never returned, is covered in bug 1434575.

Comment 6

[Jorg K (CEST = GMT+2)]

Comment on attachment 9003702 [details] [diff] [review]
Move HTMLEditor::GetEmbeddedObjects (nsIEditorMailSupport.getEmbeddedObjects) to comm-central

Thanks, this looks fine. Can you please still address comment #5.

Comment 7

[Makoto Kato [:m_kato]]

(In reply to Jorg K (GMT+2) from comment #5)
> Thanks for looking into this. Maybe it's time for a little clean-up.
> 
> GetEmbeddedObjects() returns nsGkAtoms::img, nsGkAtoms::embed, nsGkAtoms::a
> and nsGkAtoms::body.
> 
> Mailnews processes 'img', 'a', 'body' and 'link'!
> https://searchfox.org/comm-central/rev/
> 39cff66145fdfbfa785202c5b8d1c72a06034034/mailnews/compose/src/nsMsgSend.
> cpp#1287
> nsMsgCompose.cpp processes 'img', 'a' and 'link'.
> 
> So I think it's about time not to return 'embed' any more since it makes no
> sense. What do you think? That we check for 'link' which is never returned,
> is covered in bug 1434575.

Hmm, this code is written on Netscape era, embed is for plugin only, and Flash don't run on compose window, so we can remove embed element support for this.  I will update the patch.

Comment 8

[Makoto Kato [:m_kato]]

Attachment: Move HTMLEditor::GetEmbeddedObjects (nsIEditorMailSupport.getEmbeddedObjects) to comm-central v2

remove embed element support

Comment 9

[Jorg K (CEST = GMT+2)]

Comment on attachment 9007671 [details] [diff] [review]
Move HTMLEditor::GetEmbeddedObjects (nsIEditorMailSupport.getEmbeddedObjects) to comm-central v2

Thank you, I'll get it landed, if you don't mind.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fe6d8f59182d
Move HTMLEditor::GetEmbeddedObjects (nsIEditorMailSupport.getEmbeddedObjects) to comm-central. r=jorgk DONTBUILD