On Sun, Jan 17, 2010 at 08:41, Roland Liebl roland@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.
- 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'));
- 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")));
- With the recent changes in the svn trunk version you also need to
register the plugin in task "login" public $task = "mail|login";
- Personally I don't want to see an extra checkbox on the login
screen. Maybe this could be make configurable.
- Adding $item->from to the subject seems not necessary since most RSS
readers also display the autor-field where we have the from address.
- 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.
- And why do you encode all chars with hex-entities? IMO
htmlspecialchars() should do the trick.
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 _______________________________________________ List info: http://lists.roundcube.net/dev/