How can you ever get a module with an id of 0

New behavior is related with #11063 but I'm also curious why you have id=0

To me the bug is that you have an id=0 and its that which should be fixed

By looking at the code I'm guessing it fixes a notice if you try to load module by using its title and the module cannot be found.

well you should get a notice in that case

On 5 August 2016 at 10:54, Matias Griese notifications@github.com wrote:

By looking at the code I'm guessing it fixes a notice if you try to load
module by using its title and the module cannot be found.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#11468 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8S2--b69CR3KaMCHFwO-vOI0sY5Iks5qcwhjgaJpZM4JdgiO
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

Firstly, your data is wrong you should never have a module with an id == 0, in fact nothing valid should have an id == 0. With that in mind the code is correct, I might would use "!empty($mod->id)" as condition but that is not needed.

Actually looking further into the code, it looks like that JModuleHelper always sets id... So basically the new code ends up working just like the if () wasn't there.

Maybe user has overridden JModuleHelper class which allows modules without id? I don't know... Or maybe the point is to make module visible even if it doesn't exist..

Id=0 may be from that line:
https://github.com/Kekke88/joomla-cms/blob/76f792e5ca7547e1d9e3bd029437d2de2030de2d/libraries/cms/module/helper.php#L56

but why joomla have to create a dummy module if the module name starts with 'mod_'?

@csthomas that's a good question

BTW: It looks like this commit broke {loadmodule mod_login} call:

Kekke88@7590355

I think the check should be if ($mod->id || $title === '') to remove zombie modules and still display module instance if you do not specify module to be shown.

Sorry for the confusion - I'm new to github.

@mahagr Yes - it seems like that commit broke all modules - at least on my live website.

Yes - it seems like that commit broke all modules - at least on my live website.

are you using some 3rd party extension to load them ?

do you know if you joomla template loads them in some "special" way so that it removes "empty" columns ?

or are you just using "loadmodule" inside e.g. article text ?

So, if don't have to load modules with ID == 0, why this PR is still open? @brianteeman

No extensions are needed to reproduce the bug, you just need to add {loadmodule mod_login} into an article and see that it works in J3.5 but not in latest J3.6. The issue was not with modules with id=0, but plugin not showing module unless you specify the title (or instance) of the module.

I'm unable to reproduce on current staging both

{loadmodule mod_articles_latest}

and

{loadmodule mod_articles_latest,Latest}

are working

I am unable to reproduce this on current staging with the default protostar template. Both

{loadmodule articles_news,News Flash}
and
{loadmodule articles_news}
Work correctly

@mahagr two if us have checked and are unable to replicate your issue

@kekke88 please respond to the question https://issues.joomla.org/tracker/joomla-cms/11468#event-189047

I am closing this at this time as not reproducible. If more information is provided as already requested it can always be reopened.

i can reproduce that. The problem was that there was no instance of the module in the backend. it is installed but there was no instance under BE -> Extensions -> Modules. So you just need to add one which is not assiged to any position and it works again with {loadmodule mod_xxx}

PR was: #11063 but that PR is correct and fixes a valid issue so you need to create a instance of the mod befor you can use it.

That seems like a B/C break and imposing a restriction inconsistently to me. Ultimately JModuleHelper::renderModule() can render anything, in the database or otherwise, as its $module parameter is just a stdClass object with a set of required keys (looks like title, module, and params is all that's needed). So anything in front of that imposing a hard requirement on a record in the database to me seems like an inconsistent move. Especially since it is only the loadmodule plugin that is doing it.

I get #11063 may be a valid issue and may have unwanted side effects in different scenarios, but the PR broke what was previously valid behavior. Even if it doesn't make sense.

Now if you want to start a discussion for 4.0 to impose said requirement to the database, have at it. But the way the module system is coded today, blocking rendering without an ID is a breaking change.

Than we need to revert #11063