[RCD] RSS Plugin

Roland Liebl roland at roland-liebl.de
Tue Feb 23 17:06:30 CET 2010


On Tue, 23 Feb 2010 09:05:11 +0100, Thomas Bruederli <roundcube at gmail.com>
wrote:
> On Sun, Jan 17, 2010 at 08:41, Roland Liebl <roland at roland-liebl.de>
wrote:
>> Hi all,
> 
> Hi Roland
>>
>> the default RSS of Roundcube does not play nice with IE. Also HTTP
>> Authentication is missing. I've made a plugin for RSS support. Please
>> test
>> it! Any feedback is welcome.
> 
> I finally had a look at your plugin and it (almost) works for me. Over
> all it looks way too complicated to me. Here are some suggestions:
> 
> * Don't use imap_mime_header_decode() because the IMAP extension of
> PHP might not be available. Use $IMAP->decode_header() instead.
>

done
 
> * You have added loads of code for charset detection and conversion.
> rcube_imap returns values already encoded in UTF-8 so you shouldn't
> need to care about that. Just make sure that whithin the foreach loop
> you set $IMAP->set_charset($item->charset ? $item->charset :
> $rcmail->config->get('default_charset', 'UTF-8'));
>

done
 
> * Your plugin adds an RSS button somewhere to the page. Why not using
> the default way and adding a <link rel="alternate" ...> and let the
> browser do the rest? In this case you don't need to worry about skins
> and different styles. This could easily be achieved with
>   $rcmail->output->add_header(html::tag('link',
>     array('rel' => 'alternate', 'type' => 'application/rss+xml',
> 'href' => './?_action=plugin.rss', 'title' => "RSS Feed")));
>

Perhaps you misunderstood this part of the code. A click on the button
will popup the RSS url which the user has to enter into his rss reader.
 
> * With the recent changes in the svn trunk version you also need to
> register the plugin in task "login"
>   public $task = "mail|login";
>

done: public $task = "mail|login|logout";

 
> * Personally I don't want to see an extra checkbox on the login
> screen. Maybe this could be make configurable.
>

done
 
> * Adding $item->from to the subject seems not necessary since most RSS
> readers also display the autor-field where we have the from address.
>

done (IE does ... I have to check with firefox)
 
> * Why adding numbers to the subjects? These numbers can change if you
> delete a message or if new ones arrive and then my RSS reader gets
> confused. I'd prefer only to see the subjects.
>

removed
 
> * And why do you encode all chars with hex-entities? IMO
> htmlspecialchars() should do the trick.
>

There are some special chars which are not handled correct this way.
But I will think about to simplify it.
 
> It's definitely a good replacement for the crappy RSS output Roundcube
> currently offers. Maybe you should rename it to rss_feed for better
> understanding. However, feel free to add your plugin to our
> repository. I'll definitely remove the rss step from the core codebase
> of Roundcube.
> 
> Regards,
> Thomas

Thanks for the review.


 --- 8< --- detachments --- 8< ---
 The following attachments have been detached and are available for viewing.
  http://detached.gigo.com/rc/1Q/Si5pYWTD/vcard.vcf
 Only click these links if you trust the sender, as well as this message.
 --- 8< --- detachments --- 8< ---

-------------- next part --------------
_______________________________________________
List info: http://lists.roundcube.net/dev/


More information about the Dev mailing list