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.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 -- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
