joomla / joomla-cms

Home of the Joomla! Content Management System
https://www.joomla.org
GNU General Public License v2.0
4.71k stars 3.64k forks source link

It is possible to overwrite variables in JModuleHelper\renderModule #8276

Closed rdeutz closed 6 years ago

rdeutz commented 8 years ago

The main effect is that the Module Style function is not working as expected. If someone used in a module file a var $template then this will overwrite the $template var in JModuleHelper\renderModule.

An example here https://github.com/BabDev/Tweet-Display-Back/pull/97

Atm I don't have a good idea how to fix this, but I am open the issue so that it doesn't get lost.

mbabker commented 8 years ago

This probably needs to be chucked into the 4.0 pile. JComponentHelper::renderComponent() deals with this same issue by using a scope isolated method to execute the component, but making the same change in JModuleHelper implies a B/C break. I figured out a list of all the variables that have "scope creeped" into the module's file (see https://github.com/BabDev/Tweet-Display-Back/blob/f699f1472e3acbb5966285b6a837d2706770145a/mod_tweetdisplayback.php#L14-L27) and I know those variables get used in my own code and in core extensions.

mbabker commented 7 years ago

Based on this list, we need to decide what needs to stay accessible for B/C and what could be "safely" removed:

/**
 * Module variables
 * -----------------
 * @var   object                     $module    A module object
 * @var   array                      $attribs   An array of attributes for the module (probably from the XML)
 * @var   array                      $chrome    The loaded module chrome files
 * @var   JApplicationCms            $app       The active application singleton
 * @var   string                     $scope     The application scope before the module was included
 * @var   \Joomla\Registry\Registry  $params    Module parameters
 * @var   string                     $template  The active template
 * @var   string                     $path      The path to this module file
 * @var   JLanguage                  $lang      The active JLanguage singleton
 * @var   string                     $content   Module output content
 */

Based on the result of that, we can create a scope isolated method to include modules (similar to the component helper).

Just shooting from the hip, $module and $params probably need to stay. I'm not sure if there is a valid requirement for the module to access $path (as at that point it should basically equate to __FILE__, the application, language, and template data are accessible through JFactory if needed, $chrome usually doesn't apply in the context of a module (that's a template detail), $scope is an internal thing, and I'm not quite sure what $attribs is for.

mbabker commented 6 years ago

Covered by https://github.com/joomla/joomla-cms/pull/19834 in an indirect way.

rdeutz commented 6 years ago

cool