Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Yeah, let's call taxonomy_get_tree() on every vocabulary on a site to make a form no one will ever, ever be able to use, because if it doesn't crash the server, it still might crash their browser.
Comment | File | Size | Author |
---|---|---|---|
#8 | taxonomy_form_all_die.patch | 8.73 KB | catch |
#4 | taxonomy_form_all.patch | 1.02 KB | Dave Reid |
taxonomy_form_all.patch | 1.02 KB | catch | |
Comments
Comment #1
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks!
Comment #2
Dave ReidThis is the second culprit of test failures. When I rolled back this patch, the Taxonomy term filters in node forms (TaxonomyNodeFilterTestCase) test passed locally in contrast to 18 fails with current non-rolled-back clean HEAD.
Comment #3
Dave ReidIn fact the test bot was going to mark this as a fail (http://qa.drupal.org/pifr/test/26456) but I think it was committed before it had the chance to report back.
Comment #4
Dave ReidReversal of the original patch for now.
Comment #5
catchNode module does module_invoke('taxonomy', 'form_all'). That's evil.
Comment #6
Dries CreditAttribution: Dries commentedRolled back. Sorry about that.
Comment #7
catchComment #8
catchSo the reason tests fail is because admin/content builds a list of every single term in every vocabulary for a select list, and search/node does the same. Both of these are nasty, nasty resource hogs once you go past a certain number of terms. They're also use cases which are much better served by #497804: [meta] Search entities (nodes, terms, etc.) within the administrative interface for admin/content, faceted search instead of advanced search, or views (where you can use autocomplete, or restrict filters to a vocabulary which doesn't have thousands of terms in it).
So here's a patch to just rip that functionality out completely. If that's not acceptable for D7, then we should at least do a COUNT(*) query here and bail out after a certain number, lots of sites use tagging.
Comment #9
moshe weitzman CreditAttribution: moshe weitzman commentedI agree with stripping this feature. Core just can't adequately do what it is trying to do here (show all terms in a select).
Comment #10
Dries CreditAttribution: Dries commentedI support this change as well, and committed the patch to CVS HEAD. It is just not a scalable solution for some sites, and better alternatives are available if necessary.