Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.So, silly person that I am, I thought I'd start with aggregator when doing the page-split implementation. I figured that since it wasn't as actively developed I'd be less likely to bump into someone else's patch. Of course, along the way I discovered that aggregator is, currently, totally broken. Half the forms don't implement FAPI 3 right, half the menu callbacks are setup wrong, and there's Notices coming out of its ears. Yay!
So, the attached patch is a work-in-progress to implement the page-split and fix the various and sundry problems in aggregator; or at least to properly implement the Drupal 6 APIs. Feedback and collaboration welcome. :-)
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | aggregator_love_3.patch | 16.89 KB | chx |
| #10 | aggregator_love_2.patch | 16.3 KB | webchick |
| #9 | aggregator_love_1.patch | 23.53 KB | webchick |
| #8 | aggregator_love_0.patch | 14.71 KB | webchick |
| #6 | aggregator_love.patch | 13.12 KB | chx |
Comments
Comment #1
Crell commentedNew version. I think I got everything, with one exception. When viewing aggregator/categories/1 (or similar), there is a string of errors from user module. Tracing it through, I was able to determine that inside theme('page') user_access() gets called with an $account parameter of 1, which is invalid. Why that's happening I have no idea, nor do I know if it's being caused by aggregator itself. If anyone has a suggestion on that front I'd appreciate it. Otherwise, here's a split and not-completely-broken aggregator.
The stack trace for the bad user_access() call is:
user_access('administer news feeds')
_menu_check_access()
_menu_translate()
menu_local_tasks()
menu_tab_root_path()
menu_get_active_help()
theme_help()
theme()
template_preprocess_page()
theme()
main()
Comment #2
Crell commentedHold that thought. I realized after talking to pwolanin that I totally b0rked the menu worse than it was originally. :-) New patch forthcoming.
Comment #3
webchickBunch of hunks fail, which I guess means that a lot of this was fixed, since I can actually use aggregator module, seemingly, except for all the notices.
I marked http://drupal.org/node/156378 a duplicate of this as it seems this patch has those fixes, but it needs a serious re-roll, as currently it causes a WSOD at /aggregator.
Comment #4
webchickWow, this module is so unbelievably hosed:
Add CNN news feed: http://rss.cnn.com/rss/cnn_topstories.rss, hit update items (admin/content/aggregator/update/2) get tons of undefined index notices
more link on aggregator/categories/1 - prints tons of notices/warnings
aggregator/sources
$feed_link not found
/aggregator
RSS feed icon is wrong ("XML" rather than feed thing).
enable one of the feed blocks,get notice: Undefined variable: output in /Users/webchick/Sites/head/modules/aggregator/aggregator.module on line 1363.
Inside the block, each item starts with <img src="/head/misc/blog.png" alt="blog it" title="blog it" width="12" height="12" /> (literally, in the text)
Comment #5
Crell commentedYeah, I think there were some other Aggregator-fixing patches recently. I back-burnered page-splitting Aggregator because it was so buggy until I took care of the more important modules. I'll try to get back to it soon. :-/ I'd be surprised if the latest patch here still applied at all.
Comment #6
chx commentedThis patch does not split aggregator but fixes a huge number of bugs.
Comment #7
snufkin commentedTested with the issues webchick mentioned on a fresh install, those are solved now.
However when i click categorize (/aggregator/sources/1/categorize) I get a lot of notices, and some errors from SQL. Log here: http://drupal.pastebin.com/m1a5668ca
Comment #8
webchickThis doesn't address snufkin's feedback (which I can verify), but does fix the following things:
- uses theme_feed_icon rather than theme_xml_icon, for consistency with the rest of core (created an issue to remove theme_xml_icon in 7.x here: http://drupal.org/node/172571)
- Fixes some minor notices when deleting a feed.
Another issue I found is that URL validation is not taking place on the add feed form.
Comment #9
webchickOk, I believe this fixes all the error spewage, and now validates the feed URL.
Comment #10
webchickOops. Just aggregator, not everything else. ;)
Comment #11
webchickTagging this as a "Beta-Breaker" ... we definitely should NOT release a beta until this is fixed.
Comment #12
chx commentedOnly change to Angie's patch: I dropped menu_rebuld calls -- not needed any more because we got rid of every loop in hook_menu in favor of object placeholders.
Comment #13
snufkin commentedTested with adding, removing, changing feeds, playing around with categories. Everything works now.
Comment #14
dries commentedCommitted to CVS HEAD. Ole! :)
Comment #15
(not verified) commented