On Mon, Jan 7, 2013 at 12:12 AM, Vincent Massol <[email protected]> wrote:
On Jan 6, 2013, at 10:54 PM, Thomas Delafosse <[email protected]> wrote:
The advantage of jSoup is that it handles the problems I could have if the html part is not well written,
Actually this is exactly what the XWiki HTML parser does too :)
I really think you should use our parser.
Plus de HTML parser is very important so the more we test it the better.
Thanks -Vincent
so I was thinking it was more secure to use it than just parsing the "<>" in the html part . But I must admit that I haven't had a close look on the xwiki renderers, and if there are some that do it well, I should probably rather use them... I will give it a closer look tomorrow.
Cheers,
Thomas
On Sun, Jan 6, 2013 at 8:53 AM, Jeremie BOUSQUET <[email protected]
wrote:
Le 2 janv. 2013 18:44, "Thomas Delafosse" <[email protected]> a écrit :
On Fri, Dec 28, 2012 at 9:01 PM, Sergiu Dumitriu <[email protected]>
wrote:
On 12/26/2012 10:18 AM, Vincent Massol wrote:
On Dec 26, 2012, at 4:01 PM, Thomas Delafosse <
[email protected]> wrote:
> On Wed, Dec 26, 2012 at 3:23 PM, Vincent Massol <[email protected]
wrote:
> >> >> On Dec 26, 2012, at 3:15 PM, Thomas Delafosse < [email protected]> >> wrote: >> >>> Ok, so I would rather have a component API like >>> >>> - Mail prepareMail(from, to, cc, bcc, subject) >> >> createMail is better than prepareMail IMO. >> >> I'd make the cc and bcc not part of the constructor and instead
move
them
>> as setters since they're optional. >> >>> - int sendMail(Mail mail) >> >> Either that or add a send() method in Mail. >> >>> while the methods addToContent, addHtml, addAttachment, etc... would be >>> directly used from the Mail class. >> >> I don't understand what addToContent is and what different it has to >> addHtml. >> > > addToContent (String mimeType, String partToAdd) is more generic : you > specify the Mime Type of the part you want to add. So addHtml(String s) is > just the same as addToContent("text/html", s). But as most of the time you > add only Html or text, I was thinking it was better to have a specific > method to add an Html part in the scripting API. I can do the same with a > addTextContent method.
I think I prefer addContent instead of addToContent.
So just to be sure, doing the following will work:
addContent("content1", "text") addContent("content2", "text") addContent("content3", "html")
right?
It's going to create a multipart email?
I think a single addContent method is good enough, passing an enum as the second parameter (the mimetype). Enums are magically constructed from velocity with our custom uberspector.
-1 for enums. That limits the possible content types we can add.
I agree on that point : there are simpler methods such as $services.mailSender.sendHtmlMail(from, to, subject, html, alternative) for people who don't know much about mimeTypes, so it would be a shame to limit this method.
I prefer:
addMimePart(String content, string mimeType)
So far it's exactly the way my addContent method works. But I can change its name to addMimePart if you prefer.
There's also a MimePart in the standard javax.mail library, and we could actually use this one, since it's more standard, and more flexible:
http://javamail.kenai.com/nonav/javadocs/javax/mail/internet/MimePart.html
http://javamail.kenai.com/nonav/javadocs/javax/mail/internet/MimeBodyPart.ht...
But this might be a bit too verbose and complex to use.
I hope that the implementation will be smart enough to send a plain message when only one body part (of type text or html) is present.
If there is only a text or html part to the mail, I add an alternative text/plain part to the mail, using jSoup to convert the html content into text, if it's what you mean.
On mail reader side, I used xwiki parsers/renderers to convert html to plain text. What is added value of jsoup ?
> Can I call addContent several times? >> > > Yes, so for example if you want to have an email with an html part
and a
> calendar part, you call addToContent("text/html", html Text) and then > addToContent("text/calendar", calendar Code). > > >> >>> So a use-case would rather be : >>> {{velocity}} >>> $mail = $services.mailSender.prepareMail(from, to,...) >>> $mail.addHtml('<p>Blabla</p>') >> >> addHTMLContent would be nicer. So you need also a addTextContent? >> why not have an addContent(String, boolean isHTML) >> or a more generic addContent(String, String mimeType) >> or both >> >>> $mail.addCalendar() >> >> What is a calendar? >> > > It is either a vCalendar or an iCalendar (it is used by Gmail to send > invitations). It corresponds to the Mime Type "text/calendar". Here again > addCalendar(String calendar) is just the same as > addToContent("text/calendar", calendar). It's just to make it easier to > use.
ok. So I think in the future we could add some calendar helper that will create the calendar string information.
-1 for a specific addCalendar method. Why not addVCard, addImage, addPDF, addDoc and so on? This makes a stuffed API, where anything that doesn't have a dedicated API method will feel like a second-class citizen.
For now this is good enough IMO: addContent("calendar info content as per RFC 2445", "calendar")
And then later on something like:
addContent($mailsender.createCalendarMimeTypeData(param1, param2, ….), "calendar")
>> You should also show an example when using the Java API. >> > > On Java it would give something like : > > @Inject > private MailSender mailSender > > Mail mail = this.mailSender.newMail(from,to,subject) ;
I don't like this too much. Why not use a constructor on the Mail object?
Constructors are bad, in a component-based world. I'd rather have the Mail object an interface, with an internal implementation used by the MailSender implementation.
(The other option is a perlookup component is you really need to have some other components injected in the Mail object; in that case you'll need setters to from/to/subject since we currently don't support constructor injection).
> String htmlCode = "<p>Blabla</p>" ; > String calendar = "BEGIN VCALENDAR... END VCALENDAR" ; > mail.addToContent("text/html", htmlCode) ; > mail.addToContent("text/calendar", calendar) ; > this.mailSender.sendMail(mail) ;
why sendMail and not send? We're in a MailSender component so we know we're sending mail already ;)
+1 for send.
By the way, I've put a first version of my component on github : https://github.com/tdelafosse/mailSender. Feel free to have a look and to tell me if there's things to change / add / enhance.
Cheers,
Thomas
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne