When hide_empty_terms is FALSE and display_nums is TRUE, terms will still be hidden if empty.
Attached is a trivial fix that tests for $hide_term before $is_hidden

CommentFileSizeAuthor
#11 1902192-11-hide_empty_terms_fix.patch658 byteshles
#6 taxonomy_menu-count-nodes-test-1902192-hide-term-fix.patch570 bytesAnonymous (not verified)
#4 taxonomy-menu-hide-term-fix-1902192.patch714 bytesAnonymous (not verified)
hide-term-fix.patch526 bytesAnonymous (not verified)

Comments

Anonymous’s picture

Anonymous (not verified) commented
Version: master » 7.x-2.x-dev
hles’s picture

hles commented
Status: Active » Needs review
hles’s picture

hles commented
Status: Needs review » Reviewed & tested by the community

Argh, good catch ! ... Looks good, I'll commit it as soon as I can.

Anonymous’s picture

Anonymous (not verified) commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new taxonomy-menu-hide-term-fix-1902192.patch714 bytes

The patch is wrong and am sorry for that. It has been diffed against 7.x-2.x-dev but only tested on 7.x-2.0-alpha2. There is a one-line that tests for nodes_count being greater than 0 while it was only testing for isset() in 7.x-2.0-alpha2.

Attached patch should be more correct but still intested... looks good though.

Status: Needs review » Needs work

The last submitted patch, taxonomy-menu-hide-term-fix-1902192.patch, failed testing.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review
StatusFileSize
new taxonomy_menu-count-nodes-test-1902192-hide-term-fix.patch570 bytes

Add the zero count for the empty terms in testTaxonomyMenuCountNodes()

Status: Needs review » Needs work

The last submitted patch, taxonomy_menu-count-nodes-test-1902192-hide-term-fix.patch, failed testing.

hles’s picture

hles commented

Please, do not change the tests. At the moment, when the number of nodes for a taxonomy term is 0, taxonomy menu does not display the count in the title link, this is by design. If you think it should be otherwise, please open another issue so it does not conflict with this one.

Anonymous’s picture

Anonymous (not verified) commented

I somehow felt this was not the proper way to submit a fix -- sending a patch and changing the tests to make it works! I am new here so I appreciate your guidance and your comments.

Thanks!

hles’s picture

hles commented

Don't worry, basically if the tests are broken and the patch can't be green because of that, you can do it. But if changing the tests implies that the behavior of the module should change too, then this needs to be discussed.

hles’s picture

hles commented
Status: Needs work » Needs review
StatusFileSize
new 1902192-11-hide_empty_terms_fix.patch658 bytes

Does this one solve this issue ?

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Reviewed & tested by the community

Patch at #11 works as expected. Thank you !

hles’s picture

hles commented
Status: Reviewed & tested by the community » Fixed

Thank you, commited !

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.