- Fixed aggregator/sources/#/categorize tabs callback.
- Fixed aggregator/sources/#/configure tabs callback.
- Fixed form elements for categorization being displayed to show up below the feed the belong to, instead of all the feed items then a list of categorization form elements with no reference. Might be a better way to do this as my fapi mojo ain't that great.
- Fixed /categorize tab not being styled due to incorrect #id (#aggregator_page_source instead of #aggregator).
- Fixed last updated time being styled by t(), which get munged by l().

CommentFileSizeAuthor
#1 aggregator.5.0_0.patch3 KBKjartan
aggregator.5.0.patch3.33 KBKjartan

Comments

Kjartan’s picture

Kjartan commented
StatusFileSize
new aggregator.5.0_0.patch3 KB

Updated patch that fixes everything in a much more elegant way, and saving re-categorization actually works! Also merged in the missing fixes from #83573.

Thanks to chx for putting up with all my annoying fapi questions.

chx’s picture

chx commented
Status: Needs review » Needs work

$form = drupal_retrieve_form('aggregator_page_list', $sql, $header, $categorize); after this, I have no idea why that #base is needed. The form_id is aggregator_page_list, why are you setting #base then? Here is an excerpt from form.inc:

  if (!isset($form['#theme'])) {
    if (theme_get_function($form_id)) {
      $form['#theme'] = $form_id;
    }
    elseif (theme_get_function($base)) {
      $form['#theme'] = $base;
    }
  }

and

  if (!isset($form['#validate'])) {
    if (function_exists($form_id .'_validate')) {
      $form['#validate'] = array($form_id .'_validate' => array());
    }
    elseif (function_exists($base .'_validate')) {
      $form['#validate'] = array($base .'_validate' => array());
    }
  }

I skip the #submit code, but clearly, setting #base to $form_id serves no purpose.

Kjartan’s picture

Kjartan commented

Ahh, you would think so, but alas no. All drupal_retrieve_form does is get the form data. drupal_get_form does the submit/validate/theme stuff, and it is called from the menu callback:

drupal_get_form('aggregator_page_source', $feed)

aggregator_page_source() gets the $feed data, and passes it on to _aggregator_page_list, which drupal_retrive_form('aggregator_page_list'). This is fine and dandy to get the form, but since drupal_get_form() is using a completely different form id ('aggregator_page_source') none of the hooks get called. Now if 'aggregator_page_list' were just called from one function we could just rename stuff and call the right thing initially, but it is also used in drupal_get_form('aggregator_page_category'). Hence the use of #base to force drupal_get_form() to call the proper callbacks, without having to duplicate all the callbacks for aggregator_page_source/aggregator_page_category.

chx’s picture

chx commented
Status: Needs work » Reviewed & tested by the community

I am blind. I thought that retrieve is a get. Sorry.

Kjartan’s picture

Kjartan commented
Status: Reviewed & tested by the community » Fixed
Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)