Category search bug

[arena]

with 2.9

OnTheFly013.jpg : normal list

OnTheFly014.jpg : when searching string 't'

+ the code needs a strong review (echo's beeing trapped in a buffer that is then echoed ...)

[arena]

list ok

[arena]

search ko

[arena]

Replying to arena:

[Denis-de-Bernardy]

see also #8248

[ryan]

This is too big for 2.9.1, moving to 3.0.

[arena]

@ryan

anyway found a new test that breaks the code patched

imagine searching 'ss' in the following list (stars are for levels)

• categ 01

ss categ 02
* categ 03
ss categ 04

search will retrieve (ss categ 02, ss categ 04)

code patched will display

• categ 01

ss categ 02

and ss categ 04 will never appear.

working on it !!!

[arena]

definitive patch !

solve all cases

[arena]

last version of patch contains some code and db access optimization !

[arena]

new search for 't'

[arena]

new search for '02'

[arena]

patch

[arena]

no big changes in patch, just code optimization.

[arena]

Apparently someone did a code review as mentionned in the original bug description.
However this was not fixing the search bug.

File #11575.patch must be discarded

File 11575.patch is the last patch from wordpress trunk asof 2010/01/12

[nacin]

Shouldn't we be worrying about backwards compat with the rearranging of arguments in cat_rows() and _cat_rows()?

A core dev has already moved this from a 2.9.x point release to 3.0 previously. Unless this is a 2.9 regression this should be returned to 3.0.

Also, in general it safe and arguably better to omit compressed styles and scripts from patches, as it is easier to review the patches and since the committer will re-compress anyway.

[arena]

@nacin

• 11575.patch omitting compressed styles.

• yes this is a 2.9 regression

• cat_rows() : no worry about arguments changed. This function is called once /wp-admin/categories.php.

• _cat_rows() : no worry about arguments changed. This function is called twice, once in cat_rows() (see above) and once by itself for recursivity.

• _cat_row arguments unchanged : retrieves the category table row in the category list and for ajax

[arena]

After a short talk with filosofo (owner of this ticket), i change the owner to ryan to get a chance to have it commited.

thank you

[dd32]

Just taking ownership of this as it ties into #11838.

The attached patches have a issue with pagination and a few other edge cases that i'd have to try before confirming.

Setting to 3.0 as any cleanups that remain from this will not be 2.9.x material.

Can you explain what is the regression exactly? I've not been able to work out what previously worked, and now doesnt. Is it simply the weird ordering of the table (and children)?

[arena]

@dd32

read ticket #11838

hard to show you screenshots on nightly build wordpress because categories admin page displays a left column with tags and a right column with categories. Waiting for the admin page to be more stable.

However i noticed the search form does not works properly as hierarchy is not respected !

[dd32]

hard to show you screenshots on nightly build wordpress because categories admin page displays a left column with tags and a right column with categories.

The strings may say Tags/Categories but the terms listed will all be categories or whatever else you're expecting. Its just the strings havnt been updated.

However i noticed the search form does not works properly as hierarchy is not respected !

No, I specifically disabled the hierachy with the latest changes there, In that case, simply as it was completely broken, which it seems is what you're saying here :) - My reasoning being that you cant display half of a hierachy, My opinion was that you should only see a list of matching nodes, not their parents.

From what i can gather, ​http://core.trac.wordpress.org/attachment/ticket/11575/OnTheFly013.jpg is the original listing, ​http://core.trac.wordpress.org/attachment/ticket/11575/OnTheFly014.jpg is after the search?

​http://core.trac.wordpress.org/attachment/ticket/11575/OnTheFly015.jpg and ​http://core.trac.wordpress.org/attachment/ticket/11575/OnTheFly016.jpg are how you'd expect it to be shown? (With parents dulled to show they're not part of the results)

2nd-opinion: how does everyone else expect the category searching to work? just show matching elements? show matching elements with their parents?

[nacin]

May be space prohibitive, but why not show parents inline in search results?

i.e. if the search will return "Hawk," then it shows:

Animals — Birds — Hawk

We could possibly show that with line breaks, just keeping them in the same table row:

Animals
— Birds
— — Hawk

[arena]

@dd32

First, do you want this code to work for any taxonomy (with hierarchy or not) ?

Current rule :
in wp 2.9.1 in _cat_rows there is a comment :
If the page starts in a subtree, print the parents.

Reminder :
A sub category can have the same name with different parents (not the same slug !!)

Your approach :
Searching this sub category would retrieve two rows without the ability to identifed them (unless i memorized the slug, who does!)

Other points on 2.9.1 code :
1) The search is performed correctly and retrieves the categories matching the search criterias. The point is that the recursive code is a mess, and parent and childs are not displayed properly (see OnTheFly014.jpg in that ticket).
2) Not sure current code handles correctly a hierarchy of more than 20 parents (current list limit), as all categories displayed are tallied (counted).

My approach :
a) based on the retrieved list of categories (with or without search => get_categories function),
b) the retrieved array is sliced (for pagination)
c) all found categories have an attribute added : _found
d) parent categories not in the sliced array are added.
e) then the recursive process does its job.
f) so the retrieved list can have more than 20 rows but that is necessary to allow full identification of the hierarchy.

@ nacin

Your proposal would only apply when searching categories ! not sure Jane would appreciate this !

[arena]

code shorter than long speeches

[dd32]

All that strange logic that you're removing is to do with paging, not searching.

// If the page starts in a subtree, print the parents.

Thats related to paging, If the page starts with a subchild, Print the parents (Even if they were on the previous page)

How do you deal with paging in that code? I dont see any at all.

Setting the offset + number is beneficial for non-hierachial taxonomies, as it reduces memory load.

I agree that it could be made much more efficient by lazy-loading the required terms, I'm not sure how that'll work with paging though, infact, i really cant see how that'll work with paging well.

Not sure current code handles correctly a hierarchy of more than 20 parents (current list limit)

The same happens if you have the limit set to 5. or 50.

I agree with Nacin here actually, Skip the rows, and simply display the hierachy inline with the result. It may take a bit of UI work to get something that displays it whilst not removing the context of it, but its doable. And a lot cleaner than displaying all the parent nodes.

You're not loosing any information about the search result, mearly adding context to it. It'll work with any taxonomy.

Your proposal would only apply when searching categories !

Not sure that matters, Its only categories (hierachial taxonomies) which are affected..

[arena]

All that strange logic that you're removing is to do with paging, not searching.

// If the page starts in a subtree, print the parents.

Thats related to paging, If the page starts with a subchild, Print the parents (Even if they were on the previous page)

How do you deal with paging in that code? I dont see any at all.

dealing with paging with this line :

$terms = array_slice($_terms, $start, $pagesize, true);

Setting the offset + number is beneficial for non-hierachial taxonomies, as it reduces memory load.

OK ! i will update my patch.

I agree that it could be made much more efficient by lazy-loading the required terms, I'm not sure how that'll work with paging though, infact, i really cant see how that'll work with paging well.

see above !

Not sure current code handles correctly a hierarchy of more than 20 parents (current list limit)

The same happens if you have the limit set to 5. or 50.

True

I agree with Nacin here actually, Skip the rows, and simply display the hierachy inline with the result. It may take a bit of UI work to get something that displays it whilst not removing the context of it, but its doable. And a lot cleaner than displaying all the parent nodes.

You're not loosing any information about the search result, mearly adding context to it. It'll work with any taxonomy.

I personnally don't agree on this point. Having two different ways to display hierarchical taxonomies whether you are searching or not looks weird to me. I'd like to know what are Jane thoughts on this topic.

Your proposal would only apply when searching categories !

Not sure that matters, Its only categories (hierachial taxonomies) which are affected..

And any other hierarchical taxonomies ...

currently updating my patch.

[arena]

last code optimization

[dd32]

attachment 11575.diff added

• Implements nacin's idea of prepended parent names.

Having two different ways to display hierarchical taxonomies whether you are searching or not looks weird to me

I guess i'm looking at it from a different perspective.

I look at a hierarchy as-is to see an overall expectation of what it looks like. If I'm searching for something, I want that specific item; I don't need to see Its Childs/Siblings - But the parents are useful information.

Now, Whilst the parent is useful information - its providing context, nothing more. I'm not interested in the hierarchy but i do need to know where it stands in the context of it all, Thus the prepending of the parent names.

Now thats the way I think, I am 100% open to displaying it in a better way, Its just that my opinion says to me mine is 'better' (I do concede that anything is better than the current arrangement!), and would love to get some UI perspective on it.

[arena]

I do concede that anything is better than the current arrangement!

???

and would love to get some UI perspective on it.

ask jane

[dd32]

I do concede that anything is better than the current arrangement!

???

By that i was refering to the mis-matched parent-childs.

ask jane

Have already passed it up the chain.

[arena]

So closing this ticket as wontfix

• conflicting with #11838
• your diff ( 11575.diff ) has nothing to do with this ticket.