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. 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.html >>>> >>>> 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
