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

RESOLVED FIXED in Thunderbird 64.0

Status

enhancement
RESOLVED FIXED
Last year
10 months ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

(Blocks 3 bugs)

unspecified
Thunderbird 64.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
Blocks: 1485227
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 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.
Attachment #9003702 - Flags: review?(jorgk)
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 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.
Attachment #9003702 - Flags: review?(jorgk) → review+
(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.
Blocks: 1489111
Blocks: 1489939
Blocks: 1264878
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.
Attachment #9007671 - Flags: review?(jorgk) → review+
Attachment #9003702 - Attachment is obsolete: true
Keywords: checkin-needed
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/fe6d8f59182d
Move HTMLEditor::GetEmbeddedObjects (nsIEditorMailSupport.getEmbeddedObjects) to comm-central. r=jorgk DONTBUILD
Status: NEW → RESOLVED
Closed: 10 months ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 64.0
Blocks: 1490192
You need to log in before you can comment on or make changes to this bug.