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.