WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#8632 closed defect (bug) (fixed)

"Search Categories" does not list/search sub-/childcategories, Tree is broken in "Edit Post"

Reported by: lloydbudd Owned by: Denis-de-Bernardy
Milestone: 2.8 Priority: normal
Severity: major Version: 2.7
Component: Administration Keywords: has-patch needs-testing
Focuses: Cc:

Description

Categories Search does not search children categories

Env: 2.8-bleeding r10204

Repro: Always

STEPS

  1. Create category 'Cities'
  2. Create category 'Victoria' with parent category 'Cities'
  3. Search categories for 'Victoria'

ACTUAL RESULTS
No matching results are found.

EXPECTED RESULTS
The category Victoria is listed as a result.

ADDITIONAL DETAILS

I think category search was added in WP 2.5, I confirmed that it and 2.7 exhibit this problem.

Attachments (6)

categories.png (73.4 KB) - added by suit 5 years ago.
category overview
categories2.png (20.3 KB) - added by suit 5 years ago.
categories (edit post)
8632.diff (412 bytes) - added by Denis-de-Bernardy 5 years ago.
pagination_issue.png (14.1 KB) - added by pp19dd 5 years ago.
Pagination issue, after the patch.
8632-notice.diff (565 bytes) - added by Denis-de-Bernardy 5 years ago.
8632-pagination.diff (784 bytes) - added by Denis-de-Bernardy 5 years ago.

Download all attachments as: .zip

Change History (43)

comment:1 minusonebit5 years ago

  • Keywords search catagory catagories added
  • Severity changed from normal to critical
  • Version changed from 2.5 to 2.7

See also #8955 (closed as duplicate of this).

I have confirmed this behavior in the most current stable release (2.7).

comment:2 minusonebit5 years ago

See also #8619. Possible fix here?

comment:3 _timk5 years ago

  • Keywords category categories added; catagory catagories removed

suit5 years ago

category overview

suit5 years ago

categories (edit post)

comment:4 suit5 years ago

  • Milestone 2.8 deleted
  • Summary changed from Categories Search does not search children categories to "Search Cateogries" does not list/search sub-/childcategories, Tree is broken in "Edit Posts"

Also if no search is active, the category listing is broken (see categories.png)

Subcategory Mac OS is present in the dropdown, but not in the listing.
If "Mac OS" is moved to another category, the category "Linux" gets lost.

As you can see in categories2.png, the category listing is also broken in the edit screen

If no category is attached to the post, the tree works fine. but once if a category is selected, the tree gets messed up (Nintendo Wii and Nintendo DS should be subcategories of "Spiele")).

I've tested this with a new installed WP2.7 without any plugins and default theme (just a few posts for testing).

comment:5 suit5 years ago

the tree-broken-problem also described here:
#8613

comment:6 mrmist5 years ago

  • Milestone set to 2.8

Everything should have a milestone.

comment:7 _timk5 years ago

  • Summary changed from "Search Cateogries" does not list/search sub-/childcategories, Tree is broken in "Edit Posts" to "Search Categories" does not list/search sub-/childcategories, Tree is broken in "Edit Post"

comment:8 pp19dd5 years ago

I believe this is not a search problem, but, a display bug. For example, if you comment these two lines (64, 65) out of 2.7.1's /wp-admin/includes/template.php :

		if ( $category->parent != $parent )
			continue;

Then the search results appear as they should (well, almost- the display is out of whack and pagination appears to be always out of touch).

I think the problem comes from the way the display routine is done, or specifically how it tries to reconstruct the search results visually against the entire category list, and then tie that with the _get_term_hierarchy('category') output.

comment:9 Denis-de-Bernardy5 years ago

  • Keywords search category categories removed
  • Severity changed from critical to major

comment:10 Denis-de-Bernardy5 years ago

Confirming it's a display bug. Two potential fixes:

  • We list all parents for every matched category
  • We ignore the hierarchy on searches

Assuming we list all parents for every matched category, we'll fix #8613 as well.

comment:11 Denis-de-Bernardy5 years ago

  • Component changed from General to Administration

comment:12 Denis-de-Bernardy5 years ago

err, nevermind on the #8613, wrong ticket.

Denis-de-Bernardy5 years ago

comment:13 Denis-de-Bernardy5 years ago

  • Keywords has-patch tested added; needs-patch removed

patch removes the wacky parent check for categories, so that the hierarchy is ignored in searches.

comment:14 Denis-de-Bernardy5 years ago

I considered listing all of the parents, but it seemed messy from a performance standpoint. Also considered changing get_terms() to return the parents directly in searches, but it felt wrong.

comment:15 ryan5 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [11123]) Include subcats in search results. Props Denis-de-Bernardy. fixes #8632

comment:16 ryan5 years ago

Good enough for now. Maybe MPTT will help, if and when we switch to that.

pp19dd5 years ago

Pagination issue, after the patch.

comment:17 pp19dd5 years ago

The previously submitted patch works as a pretty good bandaid; however, as you can see, the pagination is still slightly out of sync with reality. The query pagination behaves like it should, when applicable - meaning that the actual query and consequent results are realistic. However, the display issue still persists. In the screenshot I uploaded, 21 pages are offered for 1 search result.

comment:18 ryan5 years ago

We get to pick which bug we like least. At the moment there is no fix for everything.

comment:19 follow-up: ryan5 years ago

"PHP Notice: Undefined variable: my_parent in /Applications/MAMP/htdocs/trunk/wp-admin/includes/template.php on line 70"

comment:20 in reply to: ↑ 19 pp19dd5 years ago

First of all, I'm a pragmatist and I agree with you that bugs get picked one at a time depending on their severity.

Replying to ryan:

"PHP Notice: Undefined variable: my_parent in /Applications/MAMP/htdocs/trunk/wp-admin/includes/template.php on line 70"

Sounds like you have strict error reporting on. That variable is undeclared prior to looping. I think inserting a $my_parent = true; prior to that while statement will take care of the issue; however, these are still bandaids. That particular coding style is repeated in several places throughout that file.

On a bigger scale, I think that this routine should be as primitive as possible and that the actual display, pagination and filtration should take place through AJAX. In terms of code efficiency - as it is, *all* the categories/subcategories are already queried prior to displaying, regardless of search return. Something as simple as dojo's Dijit.FilteringSelect could provide the functionality in one fell swoop.

However, that's overtly ambitious. It seems like this pagination question (and the undeclared variables) should be a larger philosophical issue of how to make searches and search options consistent. For example, the public-facing site search ignores subcategories as a listing, etc. Perhaps categories, content items, and others should have a "searchable?" checkbox and maybe a priority weight.

I'd reopen this bug, but, the patch seems 99% effective to me. As it is, the WP backend is being redesigned and who knows which direction it will follow. Think we should be aware of this limitation but keep the bug closed.

comment:21 Denis-de-Bernardy5 years ago

I'll get the pagination issue fixed. It's actually related to something else -- the count is based on the total number of cats rather than the returned number of cats.

comment:22 Denis-de-Bernardy5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:23 Denis-de-Bernardy5 years ago

  • Owner changed from anonymous to Denis-de-Bernardy
  • Status changed from reopened to new

comment:24 Denis-de-Bernardy5 years ago

patch for the notice is attached, and I'm pretty certain it'll fix a separate bug related to category trees as well (couldn't find it)

comment:25 Denis-de-Bernardy5 years ago

  • Keywords needs-testing added; tested removed

comment:26 Denis-de-Bernardy5 years ago

Pagination patch is also attached. it and the notice fix both need some testing.

The notice fix in particular, because it will actually make some code work, that didn't until now.

comment:27 ryan5 years ago

Related: #9661

comment:28 Denis-de-Bernardy5 years ago

ah, there it is. I knew I had read about this somewhere. :-)

comment:29 hailin5 years ago

this is fixed in #9661, which fixes the root cause and other issues of cat_rows.

comment:30 hailin5 years ago

  • Resolution set to duplicate
  • Status changed from new to closed

comment:31 Denis-de-Bernardy5 years ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

Re-opening, as I'd rather let Ryan or another core dev close this one.

Your notice fix seems better (mine is a quick and dirty fix that is clearly marked as needs testing), but the pagination patch is definitely committable and might get forgotten if this gets closed as dup

comment:32 follow-up: hailin5 years ago

oops, sorry, I missed your pagination patch.

comment:33 in reply to: ↑ 32 Denis-de-Bernardy5 years ago

Replying to hailin:

oops, sorry, I missed your pagination patch.

hehe. part of the issue I had with the notice fix is that I wasn't entirely certain of what should be testing, so I had mostly no idea of the bugs it was introducing -- but you certainly seem to understand that part.

comment:34 ryan5 years ago

(In [11139]) cat_row fixes. Props hailin. fixes #9661 see #8632

comment:35 Denis-de-Bernardy5 years ago

@Ryan: There is one patch left (pagination) and I believe we're good to go on this one.

comment:36 ryan5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [11140]) Fix category search pagination. Fix caching of empty term results. Props Denis-de-Bernardy. fixes #8632

comment:37 ryan5 years ago

Fixed caching of empty get_terms() results. That way our extra call to get_categories() during searches is always cached for when we call it again from cat_rows().

Note: See TracTickets for help on using tickets.