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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch failed testing.

mustafau’s picture

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

mustafau CreditAttribution: mustafau commented
FileSize
565718-3-aggregator_api_cleanup.patch9.36 KB
sun.core’s picture

sun.core CreditAttribution: sun.core commented
Priority: Critical » Normal

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

retester2010’s picture

retester2010 CreditAttribution: 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 CreditAttribution: 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 CreditAttribution: 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.