Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
aggregator categorize feature appears to be broken in drupal HEAD (just downloaded and installed). I have this errors when tryng to access
drupal/aggregator/sources/1/categorize
Notice: Trying to get property of non-object in aggregator_page_source() (line 39 of /Applications/MAMP/htdocs/drupal/modules/aggregator/aggregator.pages.inc).
Notice: Trying to get property of non-object in template_preprocess_aggregator_feed_source() (line 491 of /Applications/MAMP/htdocs/drupal/modules/aggregator/aggregator.pages.inc).
Notice: Trying to get property of non-object in template_preprocess_aggregator_feed_source() (line 491 of /Applications/MAMP/htdocs/drupal/modules/aggregator/aggregator.pages.inc).
Notice: Trying to get property of non-object in template_preprocess_aggregator_feed_source() (line 492 of /Applications/MAMP/htdocs/drupal/modules/aggregator/aggregator.pages.inc).
Notice: Trying to get property of non-object in template_preprocess_aggregator_feed_source() (line 493 of /Applications/MAMP/htdocs/drupal/modules/aggregator/aggregator.pages.inc).
Notice: Trying to get property of non-object in template_preprocess_aggregator_feed_source() (line 494 of /Applications/MAMP/htdocs/drupal/modules/aggregator/aggregator.pages.inc).
Notice: Trying to get property of non-object in template_preprocess_aggregator_feed_source() (line 496 of /Applications/MAMP/htdocs/drupal/modules/aggregator/aggregator.pages.inc).
Notice: Trying to get property of non-object in aggregator_feed_items_load() (line 93 of /Applications/MAMP/htdocs/drupal/modules/aggregator/aggregator.pages.inc).
Comment | File | Size | Author |
---|---|---|---|
#16 | andagain.patch | 5.42 KB | catch |
#10 | notices.patch | 55.13 KB | catch |
#3 | aggregator.patch | 4.45 KB | mr.baileys |
#2 | aggregator.patch | 4.12 KB | mr.baileys |
Comments
Comment #1
mr.baileysThis is the result of "The signature of the callback from drupal_get_form() changed to add $form"
Comment #2
mr.baileysThe aggregator module uses aggregegator_page_category() and aggregator_page_source() both as normal callbacks as well as through a drupal_get_form callback. This leads to a strange function signature:
The fact that $form was added to the drupal_get_form callback signature causes this to fail.
I'd opt to split the function into a regular callback and a callback for drupal_get_form. This fixes the issue, and is more developer friendly.
Comment #3
mr.baileysMarked #725640: Module fails to load RSS Feed with errors as a duplicate. Improved the doxygen comments in the previous patch some more.
Comment #5
mr.baileys#3: aggregator.patch queued for re-testing.
Comment #6
catchPatch looks sane enough. Could we add a quick test - just visiting the path ought to be enough to trigger it no?
Comment #7
mr.baileysWe could add tests (you are right, a simple page visit will trigger the notices), but I'd prefer to have this handled by #276486: Tests needed: aggregator.admin.inc and #276499: Tests needed: aggregator.pages.inc to prevent scattered bits and pieces of tests. On the other hand, those two issues haven't seen much movement lately.
Comment #8
dodorama CreditAttribution: dodorama commentedI applied the patch everything works now.
Comment #9
catchI've actually been closing out those "tests needed" issues because they're usually so stale that there's no useful summary of what's actually missing or not - since we moved to adding tests when and if rather than just trying to get coverage up overall, but this could probably go in as a quick fix as well.
Comment #10
catchHere's the test.
edit: this was supposed to be just the test by itself, but other patches got in with it, however the 8 exceptions are the ones we wanted to see. Following post is the patch + the test together.
Comment #16
catchAnd the patch + test combined.
Comment #17
mr.baileysThanks catch! Test looks good to me, and confirmed that the test correctly catches the bug without the rest of the patch applied. Now we just need someone else to review & RTBC...
Comment #18
catchI reviewed #3, mr.baileys reviewed #16. Test bot reviewed both, I think that's a quorum.
Comment #19
Dries CreditAttribution: Dries commentedGood fix. Committed to CVS HEAD. Thanks.