After selecting an alternative parser (SimplePie module implemented one here; http://drupal.org/node/253619 ) Aggregator does not load it's aggregator.parser.inc. This behaviour makes it impossible to revert to default parser since settings page does not display default parser.

Solution is to load aggregator.admin.inc and others at every page request like other core modules (field) do.

This patch also has some cleanup. Replaces some parts with more readable code. Changes $feed->source_string to $feed->data and $feed->http_headers to $feed->headers.

CommentFileSizeAuthor
#3 565718-3-aggregator_api_cleanup.patch9.36 KBmustafau
#2 565718-2-aggregator_api_cleanup.patch9.36 KBmustafau
aggregator_api_cleanup.patch9.21 KBmustafau

Comments

Status: Needs review » Needs work

The last submitted patch failed testing.

mustafau’s picture

mustafau commented
Status: Needs work » Needs review
Issue tags: +API clean-up
StatusFileSize
new 565718-2-aggregator_api_cleanup.patch9.36 KB
mustafau’s picture

mustafau commented
StatusFileSize
new 565718-3-aggregator_api_cleanup.patch9.36 KB
sun.core’s picture

sun.core commented
Priority: Critical » Normal

I think you are effectively removing the pluggable sub-systems from Aggregator here...

retester2010’s picture

retester2010 commented
Issue tags: -API clean-up

#3: 565718-3-aggregator_api_cleanup.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +API clean-up

The last submitted patch, 565718-3-aggregator_api_cleanup.patch, failed testing.

quazardous’s picture

quazardous commented

just rewrite the _aggregator_get_variables() and put the includes outside does the trick...

include_once DRUPAL_ROOT . '/' . drupal_get_path('module', 'aggregator') . '/aggregator.fetcher.inc';
include_once DRUPAL_ROOT . '/' . drupal_get_path('module', 'aggregator') . '/aggregator.parser.inc';
include_once DRUPAL_ROOT . '/' . drupal_get_path('module', 'aggregator') . '/aggregator.processor.inc';

function _aggregator_get_variables() {
  // Fetch the feed.
  $fetcher = variable_get('aggregator_fetcher', 'aggregator');

  $parser = variable_get('aggregator_parser', 'aggregator');

  $processors = variable_get('aggregator_processors', array('aggregator'));

  return array($fetcher, $parser, $processors);
}
Andreas Radloff’s picture

Andreas Radloff commented

Since this is only a problem on the admin pages, wouldn't it be better to instead put it in the top of aggregator.admin.inc and keep aggregator.module as it is:

include_once DRUPAL_ROOT . '/' . drupal_get_path('module', 'aggregator') . '/aggregator.fetcher.inc';
include_once DRUPAL_ROOT . '/' . drupal_get_path('module', 'aggregator') . '/aggregator.parser.inc';
include_once DRUPAL_ROOT . '/' . drupal_get_path('module', 'aggregator') . '/aggregator.processor.inc';

That would perserve the original intention of (I assume) not including unneccesary files.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.