I also took the occasion for cleaning up the code in JCategories::_load() for the 'countItems' option...

Travis not very clear on this...

FILE: ...avis/build/joomla/joomla-cms/libraries/legacy/categories/categories.php
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
270 | ERROR | Expected 1 space after "%s"; %s found
275 | ERROR | Expected 1 space after "%s"; %s found
--------------------------------------------------------------------------------

I think it is the double spaces after the .= operators... let's try!

There is conceptual error: the active language must be taken into account when computing the instance hash...
Going to fix this

I have a fix ready but I cannot sync my local repo with upstream... I hope it is GitHub momentary issue...

Fixed!

Don't panic: "smz" is my new GitHub account

Confirmed!

Successfull test.

@AlexRed Thank-you for testing, Alessandro!

What I forgot to underline in the presentation of this PR is that this issue is particularly nasty when the returned count should be zero and instead it is non-zero: there you have an empty category displayed while it should not.

I know that mixing articles tagged for different languages inside a common category is not considered a best practice and common wisdom is against this, but I think instead that this gives you a further degree of freedom that may be leveraged in different scenarios. Sometimes best practices and common wisdom derive from the necessity to work around existing issues...

@test:
Patch tested, it solves the issue.

@stellainformatica Thanks for testing, ste, appreciated!

C'mon guys/girls, some more tests and comments, so we have the opportunity of this bug being fixed!

@smz
Please kindly look at
#4937

I had already found that issue while testing your patch when I saw that report.
Looks like in config.xml as well as categories/default.xml, the field
<field name="maxLevelcat
is wrong.
I.e. it does not include "None" with value "0"
Therefor the code to diisplay the subcategories should also include "None" and the display conditions should take into account "All" and "None"

Looks also that the issue exists for other core components.

@infograf768 JM, I may be, as they say, "thick as brick", but I fail to understand if you're telling me that what I'm trying to solve here is a consequence of #4937 (and hence once fixed that, this issue will be automatically fixed as well) or if you are asking me to try to fix #4937 too while I'm trying to fix this...

Sorry, sometimes is not so easy to understand between each other, especially when both are not using their native language!

Just asking if you have time to also fix #4937 as you have been working on categories. :)
This would be another PR, as I see it, as it concerns quite a few files and tests and the issue is different

Ah, OK! Will look into it and let you know if I'm able to... Right now I'm preparing a new commit to #5463 as I discovered that the dates conditions ("before", "after" and "exact") are not translated in the query description and so I'm adding 3 new strings for i18n...

@smz
Don't we also have to port this PR here to the other core components as you did in #5469 ?

@infograf768 JM, if we are convinced this is the correct behavior for all components, I think we can just get rid of the new option and modify the library with the following:

        $options['currentlang'] = JLanguageMultilang::isEnabled() ? 1 : 0;
        if ($options['currentlang'] == 1)
        {
            $options['currentlang'] = JFactory::getLanguage()->getTag();
        }

doing that we will have it automatically applied to all current and future components. What do you think?

P.S.: I already checked that it does not have any detrimental effect in backend...

or better:

$options['currentlang'] = JLanguageMultilang::isEnabled() ? JFactory::getLanguage()->getTag() : 0;

Have you tested?

Not yet, TBH... If all is well I'll push a commit, OK?

I mean: I already tested in com_content, but not with other components.

Whilst you guys are making decisions removing this from RTC

@infograf768 I tested the latest proposed modification (that makes this the normal behavior) with com_content, com_contact and com_newsfeeds. Everything is working as expected without any modification needed to the components. I then committed the modification to this PR.

@wilsonge Nothing has been modified to the logic and "active" part of the code: the only modification done has been to make the behavior proposed by this PR the "normal" behavior and thus eliminated the "switch" to activate it. This is to say that the tests performed before this latest commit are still valid and if you feel confident you can reset this to RTC

One thing that emerged while testing is that "List all contact categories" has a small bug: it does not display the items count when requested to do. Will fix in a separate PR...

I have a last minute coding doubt: while building the subquery I use the construct i.language = \'*\'. Should this be changed to i.language = $db->quote(\'*\') for compatibility with other DBs beside MySQL?

Travis has something to say which I really don't understand...

JApplication is used in JLanguageMultilang but it's not setup in the unit test. I'm pretty busy with wrapping presents atm. Will PR in a fix in a bit.

No hurry! (I may have found a small issue too... unsure... will report)

@wilsonge Nope, false alarm: everything OK here. Still no hurry!

All fixed with smz#2

@wilsonge Thanks, George, merged!

com_contact items count issue is fixed in #5515 (I think this is so trivial it may jump to merge...)

OK here. Need one more tester?

@infograf768 JM, we also have @AlexRed and @stellainformatica tests (if you still consider those as valid), but what do you think about my concern about the need to use $db->quote with string constants inside a query [see: #5416 (comment)]?

We do not use db->quote in
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_languages/helpers/multilangstatus.php#L138
but . $db->quote('*') is used in
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_languages/models/languages.php#L126
and quite a few other places.

If test OK, I guess it would be better.

Seems to be OK here...

@infograf768 JM, what do we do with this? In the meanwhile I discovered that it solves the same issue (wrong articles count) also for mod_articles_categories...

Sorry, guys, may I ask why this PR which fixes a bug and has been extensively discussed/tested isn't apparently considered for merging?

OK here. Need one more tester?

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5416.}

That looks more as a question than a statement and http://issues.joomla.org/tracker/joomla-cms/5416 says we already have 2 successfull test, but, please, feel free to test yourself.

@test OK
Sorry Sergio for the very late response here, I thought this has been merged already

@dgt41
No problem, Dimitris! Actually I think this PR already had 2 positive tests (three counting JM)

Merged - thanks for bringing to my attention. For some reason I thought I'd merged this earlier in the week. Totally my bad! Sorry!

It's OK, George! Thanks!