bug PR-4.3-dev Pending

User tests: Successful: Unsuccessful:

avatar BrainforgeUK
BrainforgeUK
27 May 2023

Avoids unexpected message on template admin.

Pull Request for Issue #40666 .

Summary of Changes

Improved handling of getMediaFiles() function.

Testing Instructions

Install a child template without any media files.
tpl_cassiopeia_test1.zip

Actual result BEFORE applying this Pull Request

Error message on template admin page
unexpectedmessage

Expected result AFTER applying this Pull Request

unexpectedmessage

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • [*] No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • [*] No documentation changes for manual.joomla.org needed

avatar joomla-cms-bot joomla-cms-bot - change - 27 May 2023
Category Administration com_templates
avatar BrainforgeUK BrainforgeUK - open - 27 May 2023
avatar BrainforgeUK BrainforgeUK - change - 27 May 2023
Status New Pending
avatar ReLater
ReLater - comment - 27 May 2023

I'm not a friend of creating a bunch of empty folders if I explicitly don't need them or don't want them. Simply remove the tag <inheritable>0</inheritable> in your manifest.

References:
#39329
But has been reverted: #39943

  1. Question:
    If your solution is accepted. Will these folders automatically removed on uninstallation of the child template?
avatar dgrammatiko
dgrammatiko - comment - 27 May 2023

Install a child template without any media files.

This is not supported by Joomla, Joomla's own functionality WILL NEVER create a child without the expected folders. Instead of trying to bend Joomla to your way why not align your way to the Joomla's expectations?

avatar BrainforgeUK
BrainforgeUK - comment - 27 May 2023
  1. Agree with empty folder comment.
    Don't know why relevant.

  2. As I mentioned in the related issue, this may not be relevant in 'the real world'!

I suppose when removing 'mytemplate', for completness, the installer should remove everything in the following places regardless of what the manifest XML says ... it may already do that, I have not investigated!

templates/mytemplate
media/templates/site/mytemplate
language//mytemplate.ini
language/
/mytemplate.sys.ini

There might be other areas to which a similar installer 'for completeness' comment might apply!

avatar BrainforgeUK
BrainforgeUK - comment - 27 May 2023

Then why does Joomla allow me to install a template without media files?

Surely the template media files will only be relevant to the template or other template related extensions. That's an issue for template / extension developers to address.

Joomla should just allow the situation without confusing messages during template testing / development.

avatar dgrammatiko
dgrammatiko - comment - 27 May 2023

Also if you must do the magic creation either do it in the install or discover on the installer adapter:

class TemplateAdapter extends InstallerAdapter

I'm not a fan of code that does something different than what is defined in the XML...

avatar brianteeman
brianteeman - comment - 27 May 2023

Joomla should just allow the situation without confusing messages during template testing / development.

absolutely not. its not a confusing message it is the 100% correct message.

avatar BrainforgeUK
BrainforgeUK - comment - 27 May 2023

I agree a change to the installer adapter would avoid this situation happening - and would need further investigation.
This change addresses the situation after it has happened in a dev / test environment.

This change provides a safe display of an empty media location to which the admin can then add folders / files in the usual manner.

A better way might be to create an empty index.html file in that location - but that does not solve the problem as index.html is not a media file! See functions getDirectoryTree() and checkFormat() ... would need an empty file called 'empty.jpg' or similar.

Just discovered that if admin deletes all items under media/templates/site/mytemplate there is no way they can add something. They have to add what they want and then delete what they don't want! My solution solves that problem, although they may not want the css, images, js and sccs folders I created ,,, they still have to create what they want and delete what I created! At least they can create them.

avatar BrainforgeUK
BrainforgeUK - comment - 27 May 2023

The message has not changed. The code change only avoids the message when it is unnecessary.

avatar dgrammatiko
dgrammatiko - comment - 27 May 2023

This change addresses the situation after it has happened in a dev / test environment.

Again: Joomla's own functionality produces child templates with the expected structure. To get to a point where the structure is not the expected you, probably, tried to manually create a child but missed the media folders. Joomla is not broken.

This change provides a safe display of an empty media location to which the admin can then add folders / files in the usual manner.

Sorry this is kinda unacceptable (imho), the templates file manager is not supposed to correct faulty XML definitions, separation of concerns.

Maybe https://github.com/dgrammatiko/mod_child_export will help your situation (?).The way you produce the installable child templates is quite crucial!

avatar BrainforgeUK
BrainforgeUK - comment - 27 May 2023

This is not a faulty XML definition.
It is a valid child template manifest.

avatar dgrammatiko
dgrammatiko - comment - 27 May 2023

It is a valid child template manifest.

Can you post the XML?

BTW, the <media> section is required, so are the media folders. ie: Joomla will never create a child without the media section in the XML or missing the folders...

avatar BrainforgeUK
BrainforgeUK - comment - 27 May 2023

Refer to pull request #40665

avatar dgrammatiko
dgrammatiko - comment - 27 May 2023

Refer to pull request #40665

You know what? Enough time wasted here, do whatever you want. Unsubscribing

avatar richard67
richard67 - comment - 28 May 2023

I agree with the above concerns that this PR is not the right way. But let's see if other opinions come it.

avatar BrainforgeUK BrainforgeUK - change - 28 May 2023
Labels Added: PR-4.3-dev
avatar BrainforgeUK
BrainforgeUK - comment - 28 May 2023

Better idea occurred to me avoiding the creation of dummy css, images, js and sccs folders. Appears to solve the problem and allows the site admin to add files / folders as needed.
screenshot

avatar brianteeman
brianteeman - comment - 28 May 2023

The only correct fix as stated before is to include the media folders.

avatar BrainforgeUK
BrainforgeUK - comment - 28 May 2023

That implies that the installer needs to be changed so that it either:
(a) It prevents attempts to install a template without a media section in the manifest. But what if there is no media content? Maybe unlikely but sounds to me like this places an unnecessary restriction on template developers. Anyway there is currently nothing to prevent a site admin using the 'Templates: Customise' page to manually remove all template media content.
or (b) If there is no media section in the manifest it creates the folder media/templates/site/tpl_mytemplate anyway.

In both cases the changes in this pull request serve 3 useful purposes which obviate the need for the above installer changes.
(1) On entry to the 'Templates: Customise' page ensures the media/templates/site/tpl_mytemplate folder exists anyway.
(2) If the media/templates/site/tpl_mytemplate folder is empty (missing in manifest or otherwise purged in some way) allows the site admin to use the 'Templates: Customise' page to see that it is empty and (re-)add folders and files as required.
(3) Only display the non-writtable warning if the media/templates/site/tpl_mytemplate folder is genuinely non-writtable. Currently the message is displayed if the folder does not exist, which is confusing - being non-writtable and missing are not the same thing!

avatar HLeithner
HLeithner - comment - 3 Jun 2023

Thanks for your contribution.
But as already explained, you have to provide everything Joomla needs on installation and add it to the manifest.

I'm closing this since it does unexpected magic from my point of view. You can always open it again if you think it's needed.

avatar HLeithner HLeithner - close - 3 Jun 2023
avatar HLeithner HLeithner - change - 3 Jun 2023
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2023-06-03 09:14:00
Closed_By HLeithner
Labels Added: bug

Add a Comment

Login with GitHub to post a comment