Closed
Bug 761790
Opened 12 years ago
Closed 12 years ago
make import not depend on nsIDOMNode and nsIDOMHTMLImageElement for importing messages with images.
Categories
(MailNews Core :: Import, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 16.0
People
(Reporter: Bienvenu, Assigned: Bienvenu)
References
Details
Attachments
(2 files, 4 obsolete files)
76.22 KB,
patch
|
standard8
:
superreview+
|
Details | Diff | Splinter Review |
921 bytes,
patch
|
Bienvenu
:
review+
|
Details | Diff | Splinter Review |
Assignee | ||
Comment 1•12 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.
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
Assignee | ||
Updated•12 years ago
|
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Assignee | ||
Comment 4•12 years ago
|
||
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•12 years ago
|
||
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•12 years ago
|
||
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)
Updated•12 years ago
|
Status: REOPENED → ASSIGNED
Comment 8•12 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•12 years ago
|
||
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 10•12 years ago
|
||
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•12 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•12 years ago
|
||
Attachment #632387 -
Attachment is obsolete: true
Attachment #632387 -
Flags: superreview?(mbanner)
Attachment #633701 -
Flags: superreview?(mbanner)
Comment 13•12 years ago
|
||
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•12 years ago
|
||
fixed on trunk, with comment addressed. http://hg.mozilla.org/comm-central/rev/5edf3019ddac
Status: ASSIGNED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 16.0
Comment 15•12 years ago
|
||
Attachment #635973 -
Flags: review?(dbienvenu)
Assignee | ||
Updated•12 years ago
|
Attachment #635973 -
Flags: review?(dbienvenu) → review+
You need to log in
before you can comment on or make changes to this bug.
Description
•