Hi,

While going through my error-log, I realized that our comment RSS feed consistently gave out notices. So I set out to investigate why, and the reason is really simple..

In modules/comment/views_plugin_row_comment_rss.inc, line 39 it tries to read $this->options['item_length'], no such option exists - neither views_plugin_row_comment_rss, nor it's two parent classes views_plugin_row and views_plugin has the option 'item_length' listed in their option-definitions.

Is the item_length option a left-over of a local option that was removed, or did some of the parent classes change? In either way, it's a, in my eyes, a bug.

Regards
Morten

CommentFileSizeAuthor
#6 1355520.patch2.44 KBdawehner
#2 1355520.patch1.56 KBdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fangel’s picture

Also, the same is true for the option 'links' used further down in line 81.

AND a third bug is using the wrong index into the $this->nodes-array in line 73

$build = comment_view($comment, $this->nodes[$comment->cid], 'rss');

it should be

$build = comment_view($comment, $this->nodes[$comment->nid], 'rss');

because it doesn't make sense to use the cid as the index into the nodes array.

dawehner’s picture

Status: Active » Needs review
FileSize
1.56 KB

This problems got introduced by #1243220: Cleanup RSS content building and handling

So what about this patch?

fangel’s picture

That's a good start, although the method options_form_summary_options() isn't defined in either views_plugin_row_comment_rss or views_plugin_row.
So either the parent-class needs to be updated, or that method needs to be added to the class..

dawehner’s picture

What's the problem with the not implemented options_form_summary_options ? This sounds a bit more like a feature request though.

fangel’s picture

The problem is that you use it as the #options-directive in the options-form.. Hence not implementing it is sort of a problem, as you would get fatal errors otherwise ;)

+ '#options' => $this->options_form_summary_options(),

dawehner’s picture

FileSize
2.44 KB

Ups! Thanks for spotting the problem!

Never trust me if i write something like "what about this patch" :)

Here is a patch which should solve the problem.

dawehner’s picture

Status: Needs review » Fixed

Some manual testing later i committed a working version of the patch.
Thanks for helping on this issue.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.