I suspect (not checked) that this is the module chrome of the module position causing this

I managed to replicate this issue if the module does get loaded twice.

Steps to reproduce with protostar template

1. Create custom html module with title "Test Custom Module".
2. Assign to position "Left [position-8]" and assign to all pages.
3. Create a featured article that loads that same module. {loadmodule mod_custom,Test Custom Module}

Result

The module in the left position will now have been doubled up in the html markup.

So thats exactly what i suspected. The module chrome for that template position is xhtml.
The code for that is coming from here
https://github.com/joomla/joomla-cms/blob/staging/templates/system/html/modules.php#L89

So what you are seeing is absolutely expected behaviour

Happens with other chromes too.

yes it depends on the chrome being used
For example it wont happen with chrome:none

Changing the module chrome from "xhtml" to "none" stills doubles the markup which in this case is just outputting $module->content.

I loaded the same module again in the featured article for a second time. The module in the left position is then tripled in the markup. This is run through JModuleHelper::renderModule each time the module is loaded. The renderModule method is modifying the $module->content param here: https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/module/helper.php#L199

This is then passed again to the mod_custom layout here: https://github.com/joomla/joomla-cms/blob/staging/modules/mod_custom/tmpl/default.php#L15, where it is again wrapped in a div with a class of custom.

I would have thought that expected behaviour is to be rendered once and then reused output for the other two modules?

I would have thought that expected behaviour is to be rendered once and then reused output for the other two modules?

Joomla doesn't hold in memory the results of rendering a module. The $module object that method is processing is a plain PHP stdClass object, it doesn't correspond directly to the database record (because the API also could conceivably support rendering a module without needing any data from the database). So if what you're saying is true in that the same module's contents are being echoed out multiple times, it would seem to me whatever is feeding the data object into JModuleHelper::renderModule() is incorrectly building/structuring that data, not necessarily a bug in that method (since also if you look in that method, at no point does it prepend/append the content but rather sets it and overwrites anything that might already be assigned to the content key of the object).

What could possibly be causing an issue (with all modules in general) is an issue of scope creep. When a module is rendered, the module's base file includes these variables defined in JModuleHelper::renderModule(). Since $module->content is the result of whatever the module echoes out plus whatever is assigned to the $content variable, if a module is actually setting that variable in some form it would result in unexpected output changes.

Ok that makes sense. In that case should the JModuleHelper::renderModule method clone the $module param so it does not modify the $module->content? Since $module is an object it is modified by reference and changes the original object in the code block that calls the renderModule method.

As a test I have put clone $module like so:

    public static function renderModule($module, $attribs = array())
    {
        static $chrome;
        $module = clone $module;

        // Check that $module is a valid module object
        if (!is_object($module) || !isset($module->module) || !isset($module->params))
        ....

at the top of the renderModule method and this does stop the duplication issue. As this method is just used for generating the output, I don't think it should modify the original $module object.

Does that make sense?

Edit:

Not sure what the value of the $content variable here: https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/module/helper.php#L196-L199. Why is it always appending '' to $module->content?

I don't think we'd need to clone it at first glance. My gut feeling is whatever upstream provider is passing the data in might be reusing an object when it shouldn't be (depending on the path this might come back to the plugins that render modules in content or the JDocument renderers).

As for the $content variable, I honestly haven't a clue. Seems like someone could create an edge case though where they assign something to that $content variable in their module and its value is spit out after the module's been processed (the require JModuleHelper::getLayoutPath() line in most modules). They'd be better off just spitting it out themselves in the layout or after the require line, but thanks to variable scope creep, it's entirely possible to do something goofy (and other damaging things if you look at what variables are declared and creep in).

I've had a closer look through the process of rendering modules via JDocumentRendererHtmlModule and the loadmodule plugin (the plugin uses that renderer anyway). They both at some point end up calling JModuleHelper::load where it has stored a static array of module objects. It seems like it is here it is reusing the module objects for the renderModule method.

I think the solution to this is to either clone the $module object in the renderModule method so it does not modify the original object or we update the mod_custom module to not use $module->content in its layout. Instead it should load the contents from the table row via a helper to prevent the variable scope creep.

The module table's content row should be renamed if it's causing this deep of a conflict, or the object's key name should be changed. Either way it's a B/C break so not an immediate fix.

I personally don't think cloning in the render method is the right fix, it seems more like that is addressing a symptom of the issue versus fixing it.

Honestly, we shouldn't be trying to render the same object multiple times. Seems like there should be better tracking internally to prevent that. If you're really displaying the same mod_custom instance three times on the same page, the actual module should only be rendered once and the other times it's rendered should make use of that first run then apply template chrome since that can be configured differently based on where in the page it's displayed.

Would you still have the same issue with allowing the chromes to run once for each instance? It seems it will keep wrapping $module->content here as well.

I'm having trouble working out what should keep track internally of which parts of the module has been rendered. Is this a task for JDocumentRenderer or the JModuleHelper::renderModule method?

So here's the issues I see:

• The $module->content key is reused in several ways; as a database record it corresponds to the row in the modules table, and its uses in the render method
• Modules are inherently rendered the same number of times they are configured to display on a page; a single module instance (one record from the database for most conventional uses) should only trigger the module's mod_whatever.php file once since the results shouldn't change unless you're manipulating the parameters or something outside the normal rendering process, but honestly in that case you should be working with a different object (i.e. clone it)
• The wrapping chrome is assigned to $module->content as well, and because of the reuse, it causes the recursion issues
• Why do we even need to assign the processed content to $module to begin with? In the render method we're injecting the module object and returning the processed string, but the processed string is assigned to the module object too and thanks to references in PHP that contributes to the reuse/recursion issues. We'd really be better off here using a local variable (that questionable $content variable perhaps?) to keep the rendered contents separated from the $module object (and inherently the content key that exists from the database)

I'm having trouble working out what should keep track internally of which parts of the module has been rendered. Is this a task for JDocumentRenderer or the JModuleHelper::renderModule method?

I would probably have a static var in JModuleHelper for this. Create an associative array with the key being an MD5 hash of the module object and the value being whatever's processed by including the module's mod_whatever.php file. Then do something like this:

if (isset(static::$processed[$key])) {
    $content = static::$processed[$key];
} else {
    $content = include $modulePath;
}

So you're avoiding the scoping and reference issues now and can pass that separate content to the chrome method as an additional variable for that to be wrapped and in the end we end up in a state where we don't process the same data multiple times where we can help it and get rid of some of the internal issues.

Unfortunately, all of this would have to go into the 4.0 branch because it has major B/C implications (it would affect the internal handling in the module helper, change how module chromes function, and how the events dispatched from the module helper interact with the module). So maybe cloning can be a short term fix for the immediate issue while a bigger (and IMO more correct) fix be worked on separately.

I have come across the same issue. I am displaying a custom module assigned to all pages. This module is also integrated in an article via loadmodule (loadposition has the same effect). I didn't duplicate the module so its' content only needs to be changed once when being updated.

To avoid the multiple-wrapping-issue without altering the joomla core files, I created an alternative layout for mod_custom, assigned it to the module and added $module = null; at the end of the layout file.
Maybe someone finds this helpful. Dirty fix, please don't judge :)

I can still see this issue happening since my original post and looking at our issues being posted, it does not seem like i'm the only person having this issue.

The problem with the current setup of the module rendering is that anything modifying/referencing $module->content is getting stuck in a loop depending on the number of times the renderModule method gets called during the page load.

The process:

1. ModuleHelper::getModule, stores the $module reference in a static list
2. ModuleHelper::renderModule, wraps $module->content in the module chromes and then overwrites $module->content. As $module is an object, this is passed by reference in the renderMethod which commits the $module reference back to the static array in ModuleHelper::getModule
3. ModuleHelper::getModule is called again at some point in the process, this will search the static array for the $module object which at this point will now have the $module->content wrapped in the module chome markup.
4. ModuleHelper::renderModule is called again (second loadposition, 3rd party extension), this time it will wrap $module->content in the module chrome again. This is where the duplicate wrapping starts.

I'm not sure why the clone fix in the renderModule method would be a B/C? It runs the same logic the multiple times anyway, except it does not modify the original reference.

Also does this highlight that the module rendering is not efficient? Its only highlighted in the custom html module, but would this be running the module layout code the number of times renderMethod is called?

I can't reproduce the Issue on J4 - @chmst can you crosscheck and maybe close ?

@coolcat-creations I can reproduce. If you load a custom module as describend abvoe in an article you can see it. But no clue how to resove it.

@chmst ah now I got it - I maybe have an idea! thanks

@chmst no still can't reproduce it ...

I would look to solve it in the loadmodule plugin but I can't see it happening at all

@coolcat-creations thanks a lot for testing. 'We both repeated the tests on 3.9 and 4.0 and could not reproduce the issue. Closing this now as it seems resolved.

Not solved...

@SharkyKZ Can you reproduce it?

In Joomla 4 the issue is still there.

@coolcat-creations @chmst @bembelimen

Steps to reproduce

• Create custom html module with title "Test Custom Module"
• Assign any position an all sites
• Create a article that loads that same module.

I still cannot find a bug when doing this

I use cassiopeia.
Add a custom module at the position banner -> Module Assignment-> all pages
Use module-Style -> html5
Create an article -> Load this module via moduleid inside the article.
Create article menu item to this item. And it happens.

Use module-Style -> html5

That was the missing bit. I can confirm weirdness happens when html5 is selected

Please test #38571