wp_list_pages cannot handle multiple exclude_tree arguments

[tbrams]

When trying to set up a menu based on wp_list_pages and a long list of chapters I did not want on the front page, I learned that wp_list_pages ignored all exclude_tree arguments - except for the first one.

For example, using:

wp_list_pages('exclude_tree=132,502,715,852,109,153,149&title_li=&sort_column=menu_order' );  

to generate the menu at ​http://ttu.no was a no go, because it returned far to many sections in the menubar (despite my efforts to explicitly exclude a lot of these)

Although it is certainly not a show stopper, it is really annoying when you need a drop down menu on your website and know exactly how it should work in theory, so I have found a solution for this already and am just trying to figure out how I can convey my suggested fix to the official Open Source Repository.

[tbrams]

this is a patch for includes/post.php

[tbrams]

This patch works nicely on my installation. Let me know if you have questions, comments or need anyting else

Cheers
Torben Brams

[filosofo]

Don't close until it's actually committed. :)

[tbrams]

sorry - this is my first path ever.
Thanks for the guidance :-)

[MichaelH]

Patch seems to fix the problem. Also a suggested fix at ​http://wordpress.org/support/topic/251227 is very similar to this patch.

[tbrams]

I posted the same fix in the support forum before I was introduced to TRAC - you can see the original thread here: ​http://wordpress.org/support/topic/234808

So, how do we move on from here?

Kind regards
Torben Brams

[Denis-de-Bernardy]

might also be related to #9580

[Denis-de-Bernardy]

patch could be optimized by grabbing the full list of children first, and then stripping them all at once from the pages list.

[hakre]

Low-Cost solution with not a single DB Call.

[hakre]

Database was already queried and since this is a exclude function only comparing against ID and post_parent is necessary. That should do the trick at first. Pages that are deeper then one level to an excluded ID are not removed. But that would stress the Database as well (querying of all pages must be done then to establish such a functionality).

[hakre]

Code Beautification

[Denis-de-Bernardy]

$wpdb->geresults($query); is fatal

[hakre]

get_result() call fixed.

[hakre]

Code-Cleanup for Excludes for 2.8 as well.

[hakre]

therefore see patch in #8683.

[ryan]

Going only one level deep is a bug. We also need to retain the preg_split that allows separating by comma or spaces.

[hakre]

Preg_Split again

[hakre]

Deepness of one level is the documented functionality and not a bug.

[ryan]

It's a change of behavior and will compromise the parent dropdown list on the edit screen.

[hakre]

then i tend to say the documentation has the bug. and lets do not forget that the old behaviour is not working. so a bugfix will be a change of behaviour anyway. to draw the line.

[Denis-de-Bernardy]

@harke: doc should be amended, and the needed behavior for the parent dropdown list should be enforced. I'll give your new patch a shot if you write it.

[hakre]

you want it n-depth? or should be a limit considered, because afaik you can create parent / child loops with the used datastructure.

[Denis-de-Bernardy]

punting pending patch

[hakre]

Didn't we had some suggestions on how to better deal with parents? The bug should be technically easy to solve. I'm just asking because of performance.

[azaozz]

Don't think changing the behavior would be good here. Excluding only the specified page + direct children bit still showing 2nd, 3rd, etc. level (grand)children would make this unusable in many cases. Perhaps the performance problems will be solved when/if MPTT is used.

[CreativeNotice]

Bit new here... can I get a brief synopsis of why this isn't fixed as it sounds rather simple. Not gripping, just don't understand the hold up. Is it a DB resources issue?

[hakre]

Replying to CreativeNotice:

Bit new here... can I get a brief synopsis of why this isn't fixed as it sounds rather simple. Not gripping, just don't understand the hold up. Is it a DB resources issue?

Changing this function would break the known behavior. That means that changes might break backwards compability.

A solution could be to introduce a new function which does it properly.

[hakre]

Another solution might be to add a new parameter to that function that does exactly what you're after. That won't break backwards compability either and would save to introduce a new function. Since we already have a patch, this might be the route to go to preserve the code from being postponed until the end of days.

[eduplessis]

Replying to hakre:

Replying to CreativeNotice:

Bit new here... can I get a brief synopsis of why this isn't fixed as it sounds rather simple. Not gripping, just don't understand the hold up. Is it a DB resources issue?

Changing this function would break the known behavior. That means that changes might break backwards compability.

A solution could be to introduce a new function which does it properly.

How can it's break backwards compability if it's not working... for the first time??

[hakre]

Replying to eduplessis:

How can it's break backwards compability if it's not working... for the first
time??

Probably because some (not me and you obviously) might rate that bug a feature.

[roothorick]

Why hasn't the original, first patch been committed yet? It's only slower in the cases where the bug manifests, so there is no sane reason why this bug has been left open for two years.

I've been over get_pages() a couple times now, and the optimization suggested by Denis-de-Bernardy isn't realistic. You'd have to rewrite get_pages() and essentially have two copies of the same function -- one that uncondititonally gathers all pages from the DB and filters them by hand, and one that filters at the database level like it does now. Unless there's a magic SQL trick to recursively look up keys by parent...

You should commit the original patch that tbrams submitted. Barring something unreasonably clever, it's the most appropriate solution possible.

[roothorick]

I've re-read the comments, and now I'm just confused. I think I misinterpreted Denis' suggestion, but hakre's patches don't make sense to me. What DB call, exactly, are you optimizing out? get_page_children() doesn't call the DB.

I'm now taking a closer look at get_page_children() to try to see what Denis was thinking. I may or may not produce a new patch.

[roothorick]

I think this is the optimization Denis is talking about.

[Denis-de-Bernardy]

I think this is the optimization Denis is talking about.

Looks like it...

Unless there's a magic SQL trick to recursively look up keys by parent...

There is (that would be a ​recursive with statement), but not in MySQL:

[lkraav]

What state is this in? It appears I'm stuck between a rock and a hard place with WPML, where a second wp_page_menu fails to be correctly ID-adjusted for translations with 'include'. And then this core bug is apparently stopping 'exclude_tree' from working with multiple trees, which would be a workaround for the WPML bug.

Is the latest get_pages_exclude_tree_multi.patch enough alone or does it depend on the earlier ones?

[treb0r23]

I would just like to confirm that this bug is still present in Wordpress 3.5.1.

What can be done?

[SergeyBiryukov]

#19478 was marked as a duplicate.

[SergeyBiryukov]

#16202 was marked as a duplicate.

[SergeyBiryukov]

#24249 was marked as a duplicate.

[SergeyBiryukov]

#25900 was marked as a duplicate.

[wonderboymusic]

No action as of late

[cgaffga]

roothorick patch updated to patch agains 3.8.1, where this problem persists

[cgaffga]

This problem seems to persist as of version 3.8.1 :( It was driving me mad. I just added the updated patch agains WP 3.8.1​ that makes the exclude_treeparameter work for more than one page ID.

Can't this patch be finally added to the core code!?

See also #27092, recreated to update the version number to WP 3.8.1.

[SergeyBiryukov]

#27092 was marked as a duplicate.

[wonderboymusic]

In 27767:

Use wp_parse_id_list() when parsing exclude_tree in get_pages(). Add unit tests to ensure a URL string, array with string as value, and array with array as value for exclude_tree can be used to specify multiple IDs.

Props cgaffga, roothorick, hakre, tbrams for patches across the years.
Fixes #9153.