[#23553] - [4.0] Deleting duplicate module parameters
- Closed
- 8 Jul 2019
- Medium
- Build: 4.0-dev
- # 23553
- Diff
- Hackwar:j4modules
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
- Success continuous-integration/drone/pr the build was successful Details
User tests: Successful: 0 Unsuccessful: 0
In Joomla 1.5, these parameters were in the default XML that com_modules loads. Then someone decided in 1.6 that it would be better to remove them from the default XML and instead copy all of them into each and every modules own XML file. Then in 3.0 we again added such parameters to the default XML. Sounds totally logical to me...
Anyway, this removes all that duplicate code and moves these duplicate parameters back into the advanced.xml. We should consider moving the content from advanced.xml into module.xml and moduleadmin.xml, where the caching would be dropped for the admin modules. (Caching is always disabled for logged in users anyway...) But that is something for another PR.
This has a downside, that some parameters might not be supported by the module and thus not work. Since the caching is done in the ModuleHelper class and the moduleclass suffix is now done in the chrome, that leaves the layout. I would say that that is acceptable and would push third party devs to better adapt the Joomla style of writing modules to support this parameter.
Going through all of this also showed that a bunch of modules had wrong paramters for caching. This should fix that one, too.
How to test?
Edit a module, see the parameters under advanced. Apply the patch, check the parameters again and see that they are all present and accounted for.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_modules Modules Front End |
| Labels |
Added:
?
|
||
| Category | Administration com_modules Modules Front End | ⇒ | Administration com_modules Modules Libraries Front End |
So half the core modules have the parameter named 'cache', the other half has it named 'owncache'. I'm not messing around with that here, please someone decide which name to use.
| Category | Administration com_modules Modules Front End Libraries | ⇒ | Administration com_modules Modules Front End |
I've rearranged the parameters a bit by having the identical parameters in a module.xml and the additional, frontend specific parameters in a site.xml, dropping the advanced.xml.
The language field for frontend modules depends on the enabling of the languagefilter plugin.
The type HAS to be contentlanguage and never language.
The language field display and use for admin modules depends on the parameter set in com_modules general Options.
It uses language as it is NOT related to Content Languages at all but only installed languages.
For example, on a multilingual site where 4 languages are installed, if one deletes (Trash and Empty Trash) one Content Language BUT does not uninstall the language, then that language will still display in the site module language field.
Here I deleted the Persian content Language and this is the language field of a site module which I get after your patch:
I have tested this item
For the reason stated above.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23553.
I have tested this item
For the reason stated above.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23553.
Fixed it
Ok, so since the parameters are already identical for backend and frontend, I've changed this around a bit to make this clearer. To repeat: Right now (without this PR) we already have the same parameters in the frontend and backend. This PR cleans up the code a bit and moves the parameters that are identical across all modules into the common parameters file for all modules.
Did not test all aspects, but my main remarks above are now solved. :)
Could you then change your test result?
Closing as it does not take into account modules not using cache.
Can be reopened if desired after correction.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-07-08 08:21:40 |
| Closed_By | ⇒ | infograf768 |
Personally, I wouldn't move the layout parameter as there may well be 3rd party modules that don't support layouts. And I don't like parameters that don't work.
The module class makes sense to me, since we move the module class out of the module to be only in the chrome. Which means it's supported as long as the template (and not the module) supports it.
The cache makes sense if it works even if the modules don't have any code for it. I honestly don't know if the modulehelper has some default behavior to use static caching if the module doesn't ask for something else.