The default bug view has changed. See this FAQ.

make import not depend on nsIDOMNode and nsIDOMHTMLImageElement for importing messages with images.

RESOLVED FIXED in Thunderbird 16.0

Status

MailNews Core
Import
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Bienvenu, Assigned: Bienvenu)

Tracking

Trunk
Thunderbird 16.0
x86_64
Windows 7

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Assignee)

Description

5 years ago
see https://bugzilla.mozilla.org/show_bug.cgi?id=761747#c7
(Assignee)

Comment 1

5 years ago
unfortunately, nsMsgAttachmentData doesn't have quite the right fields (in particular, contentId). I'm probably going to have to invent a small interface that just has a url string and a content id. I could combine the strings in an nsISupportsString, but that's kinda hacky.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 684466
(Assignee)

Updated

5 years ago
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
(Assignee)

Updated

5 years ago
Duplicate of this bug: 684466
(Assignee)

Comment 4

5 years ago
Created attachment 630711 [details] [diff] [review]
wip

this patch builds, though I haven't tried running it yet. I also got rid of all the c++ class definitions in nsIMsgSend.idl, by moving them to a new file, nsMsgAttachmentData.h
(Assignee)

Comment 5

5 years ago
Created attachment 630767 [details] [diff] [review]
proposed fix

This adds a new embeddedImageData interface and object so that potentially other extensions or importers can create messages with embedded images. It moves the c++ classes out of the idl, which I'm hoping will make Neil happy :-)
Attachment #630711 - Attachment is obsolete: true
Attachment #630767 - Flags: superreview?(mbanner)
Attachment #630767 - Flags: review?(neil)
(Assignee)

Comment 6

5 years ago
Created attachment 630777 [details] [diff] [review]
forgot to add a file
Attachment #630767 - Attachment is obsolete: true
Attachment #630767 - Flags: superreview?(mbanner)
Attachment #630767 - Flags: review?(neil)
Attachment #630777 - Flags: superreview?(mbanner)
Attachment #630777 - Flags: review?(neil)
Duplicate of this bug: 762501
Status: REOPENED → ASSIGNED

Comment 8

5 years ago
Comment on attachment 630777 [details] [diff] [review]
forgot to add a file

>+          nsCOMPtr<nsIMsgEmbeddedImageData> imageData =
>+           do_QueryElementAt(mEmbeddedObjectList, i, &rv);
>+           if (!imageData)
>+             continue;
>+           acceptObject = true;
Nit: strange indentation.

>+      contentId = NS_LossyConvertUTF16toASCII(nodeValue);
Nit: LossyCopyUTF16toASCII(nodeValue, contentId);

>       if (!contentId.IsEmpty())
>-        m_attachments[i].m_contentId = NS_LossyConvertUTF16toASCII(contentId);
>+        m_attachments[i].m_contentId = contentId;
>       else
[Would it make more sense to fetch the contentId directly into m_attachments[i].m_contentId and then use
if (m_attachments[i].m_contentId.IsEmpty()) ?]

>+      nsMsgEmbeddedImageData *imageData =
>+        new nsMsgEmbeddedImageData(embeddedImageURL, NS_LossyConvertUTF16toASCII(cid), EmptyCString());
[Add a 2-arg constructor for nsMsgEmbeddedImageData?]

>       // Append the embedded image node to the list
>-      (*aNodeList)->AppendElement(imageNode);
>+      (*aNodeList)->AppendElement(imageData);
[Would using m_EmbeddedObjectList be clearer?]

>+        nsCString uriSpec;
>+        uri->GetSpec(uriSpec);
>+        nsCOMPtr<nsIMsgEmbeddedImageData> imageData =
>+          new nsMsgEmbeddedImageData(uriSpec, nsDependentCString(cid),
>+                                     nsDependentCString(name));
>+        embeddedObjects->AppendElement(imageData);
ProcessMultipartRelated just turns this spec back into a URI... would it make more sense to store this as a URI in the first place?
(Assignee)

Comment 9

5 years ago
Created attachment 632387 [details] [diff] [review]
fix addressing comments
Attachment #630777 - Attachment is obsolete: true
Attachment #630777 - Flags: superreview?(mbanner)
Attachment #630777 - Flags: review?(neil)
Attachment #632387 - Flags: superreview?(mbanner)
Attachment #632387 - Flags: review?(neil)
Comment on attachment 632387 [details] [diff] [review]
fix addressing comments

This is almost perfect, but it would be helpful for nsMsgEmbeddedImageData to live in either mailnews/base/util or somewhere under mailnews/import so that we can link to it in external linkage builds; any ideas?
Attachment #632387 - Flags: review?(neil) → review+
(Assignee)

Comment 11

5 years ago
(In reply to neil@parkwaycc.co.uk from comment #10)
> Comment on attachment 632387 [details] [diff] [review]
> fix addressing comments
> 
> This is almost perfect, but it would be helpful for nsMsgEmbeddedImageData
> to live in either mailnews/base/util or somewhere under mailnews/import so
> that we can link to it in external linkage builds; any ideas?

Ah, I forgot the part of the patch that adds this to nsMailModule.cpp, so code can create this object via xpcom. But it's such a trivial class that that's not important. So mailnews/import/src is probably the better place for it. I'll have a whack at that later today.
(Assignee)

Comment 12

5 years ago
Created attachment 633701 [details] [diff] [review]
move embeddedimage object into import code
Attachment #632387 - Attachment is obsolete: true
Attachment #632387 - Flags: superreview?(mbanner)
Attachment #633701 - Flags: superreview?(mbanner)
Comment on attachment 633701 [details] [diff] [review]
move embeddedimage object into import code

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

::: mailnews/import/eudora/src/nsEudoraEditor.cpp
@@ -286,5 @@
>    return bHasEmbeddedContent;
>  }
>  
> -
> -nsEudoraHTMLImageElement::nsEudoraHTMLImageElement(const nsAString & aSrc,

I think you're missing the equivalent removal from nsEudoraEditor.h
Attachment #633701 - Flags: superreview?(mbanner) → superreview+
(Assignee)

Comment 14

5 years ago
fixed on trunk, with comment addressed. http://hg.mozilla.org/comm-central/rev/5edf3019ddac
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 16.0
Created attachment 635973 [details] [diff] [review]
Fix external linkage builds
Attachment #635973 - Flags: review?(dbienvenu)
(Assignee)

Updated

5 years ago
Attachment #635973 - Flags: review?(dbienvenu) → review+
You need to log in before you can comment on or make changes to this bug.