User tests: Successful: Unsuccessful:
Pull Request for Issue # .
This PR
DO NOT USE PATCHETESTER FOR THIS ONE. Download the package instead
If you're using Git:
It needs npm I
and also a change in the db table #_template_styles
: the column inheritable
should become 1
for all the styles of the 2 templates
If you're installing the installable package
No extra steps
Check that the Joomla Install doesn't have any visual regression
Check that the back end template is not missing any assets and doesn't have any visual regression (skip the extension templates, there's a PR #35998 for that)
CHANGE THE DEFAULT EDITOR TO NONE!!! (tinyMCE has its own PR #36011)
Check that the front end template is not missing any assets and doesn't have any visual regression
Check the Error pages
Check that if you delete the file media/templates/cassiopeia/css/editor.css
and try to edit an article (assuming that tinyMCE is the default editor) the media/system/css/editor.css
is in the list of loaded CSS files
There are no visual differences, the changes are in the XML definition of the templates and some static files moved to the media
folder
Still needed:
Status | New | ⇒ | Pending |
Category | ⇒ | Administration Templates (admin) NPM Change Repository |
I can't see any change in the PR yet for new installations,
Will add them in a bit
or am I missing something?
Theoretically, users could have their own css/js/images in the template/{atum | cassiopeia}/{css/scss/js/images} thus we need to copy whatever exists there to media
preflight and then remove those folders post flight. Or am I stretching this too far?
Or am I stretching this too far?
@dgrammatiko I have to think about it a bit, but I think you are right. Regarding bilateral detail clarifications we can use Glip, if necessary, in order not to blow up number of comments here.
@dgrammatiko Just a question for my understanding, sorry if that was already clarified elsewhere and I missed it: With templates which support child templates, will EVERY css and js for a template be below the media folder, also custom stuff added by the user, so below the regular template folder there won't be any (s)css and js anymore?
so below the regular template folder there won't be any (s)css and js anymore?
Yup, the folder media/templates/site/cassiopeia/css
has all the super powers the templates/cassiopeia/css
had (eg placing files there will override the css file, assuming the the API was used for inserting the tag in the first place). The same goes for js and images...
@dgrammatiko Another thing is that if users did it right, they did not make any customizations (besides a user.css) in the template but made a copy of the template, especially in case of the frontend (Cassiopeia). I don't think we safely can update also such template copies to the new style, so maybe it should be documented somewhere or told in the release announcements something like "If you have made a copy of the Cassiopeia or the Atum template, ....".
I don't think we safely can update also such template copies to the new style
Oh no, template copies won't follow this pattern, this will be strictly only for the Cassiopeia and Atum. Their fork, if exists will still work (we don't touch anything in there) but will not converted to the new mode
Hi everyone, @dgrammatiko With this new feature, it would be possible for the superuser to make a template for each device, for example for mobile, tablet, or desktop and thus avoid overloading everything in in a single css? And if it is possible to detect the type of device also to avoid loading several css and only serve the css corresponding to the requesting device? I thank in advance much the answer.
@dgrammatiko GitHub shows some of the files as a deleted file and a new one instead of having been moved, e.g. the "_bootstrap.scss" file. The reason might be that you have moved the file and then made changes on the content of that file with the same commit. In such a case this happens. It can be avoided by making 2 separate commit, one for moving the file (e.g. with the "git mv" command or on GitHub's UI here, that works nice, too) and then the other commit for the changes inside.
It is not a big problem here, I think, because the history in Git is not completely lost, but I'm not sure if Git blame will work here for these files.
If @bembelimen is ok with it, leave it as it is and use my hint only for future PR's.
it would be possible for the superuser to make a template for each device, for example for mobile, tablet, or desktop and thus avoid overloading everything in in a single css?
This is already possible right now but if you use Bootstrap there's no way to have any gains as the minimum CSS served is already predefined. Will it be easier is a more interesting question and I think it will be (although I haven't explored this area of serving per device, so take this with a grain of salt)
Labels |
Added:
NPM Resource Changed
?
|
@dgrammatiko The build in npm is failing, see https://ci.joomla.org/joomla/joomla-cms/48041/1/7 :
Error: Can't find stylesheet to import.
@import "../../../../../../../../media/vendor/fontawesome-free/scss/fontawesome";
media/templates/administrator/atum/scss/vendor/fontawesome-free/fontawesome.scss 6:9 @import
installation/template/scss/template.scss 8:9 @import
installation/template/scss/template-rtl.scss 1:9 root stylesheet
@dgrammatiko Regarding the update SQL I have a question. Users might have created new template styles for atum or cassiopeia. Shall we update the inheritable
flag to 1 for all of them? If yes, then the update would be easy.
If you want I can make a PR for you, but I think you can do that yourself, too. Just add a new "4.1.0-2021-10-22.sql" for each database type with following one statement:
UPDATE `#__template_styles` SET `inheritable` = 1 WHERE `template` = 'atum' AND `client_id` = 1 OR `template` = 'cassiopeia' AND `client_id` = 0;
In the PostgreSQL file just replace the MySQL names quotes by "
:
UPDATE "#__template_styles" SET "inheritable" = 1 WHERE "template" = 'atum' AND "client_id" = 1 OR "template" = 'cassiopeia' AND "client_id" = 0;
@dgrammatiko For the files and folders deletion and copying present custom stuff I will check later. Normally we don't bother authors of PR's for 4.x with the files and folders deletion and do that just before the release. But in case of this PR it makes sense that we do it here together with the custom stuff copy thing.
Category | Administration Templates (admin) NPM Change Repository | ⇒ | SQL Administration com_admin Postgresql Templates (admin) NPM Change Repository |
@richard67 thanks, hopefully I didn't mess things 108250f
Yeah, the folders for this particular case need some custom handling as they might contain user created files and we don't want to delete these
@dgrammatiko SQL stuff looks ok to me. NPM is happy, too, up to now.
Yeah, the folders for this particular case need some custom handling as they might contain user created files and we don't want to delete these
@dgrammatiko Could you list which folders we should keep in case of present custom stuff and which we could and should delete because there should not be any user stuff?
So, as a case study let's take Atum (cassiopeia will be similar). The final structure of the template folder will be
And the media folder should have all the static folders (js,css,scss,images):
But we cannot simply delete the existing folders in the administrator/templates/atum
as these might have user files, so what I was thinking as a safe upgrade path would be something like:
administrator/templates/atum/{css,scss,js,images}
to media/templates/administrator/atum/{css,scss,js,images}
media/templates/administrator/atum/{css,scss,js,images}
with the ones from 4.1administrator/templates/atum/{css,scss,js,images}
The process should retain any user created static overrides (css,js,images)
@dgrammatiko Alternatively we don't to anything pre-flight, and post-flight before we delete files and folders we go through these specific folders and copy files which exist in the old but not in the new folder.
Both will make problems if there are files which exist in the old folder but not in the new folder and which are not custom stuff. That would happen e.g. when later after this PR has been merged changes will be done in (s)css or js for these 2 templates which remove some css or js or image files, so you have them in the old but not in the new folder when updating from 4.0.x to 4.1.0 which will include your PR.
I think we should use a hard-coded list of the core old stuff as it is now and which we use as exclusion list for that copying so we only copy stuff which really has been added by the user and not old obsolete core stuff.
Update: Forget the problem. That will be handled with the regular files and folders deletion.
Alternatively we don't to anything pre-flight, and post-flight before we delete files and folders we go through these specific folders and copy files which exist in the old but not in the new folder.
That would work as well
Both will make problems if there are files which exist in the old folder but not in the new folder and which are not custom stuff.
If the files are redundant and somehow still exist in the new path obviously is not perfect but as these are static files and mostly few kb that won't create any issues (imho). Keeping a registry with the files is a maintainers nightmare thus my naive approach copy/update/delete. Anyways, I'm ok with any solution as long as users won't start blaming me that I deleted their precious files...
@dgrammatiko Forget about the problem, I have just updated my previous comment.
@dgrammatiko Ideally we would put the copying of custom js and css and image files to here between the files removal and the folders removal: https://github.com/joomla/joomla-cms/blob/4.1-dev/administrator/components/com_admin/script.php#L7395
The cool thing if we do that there is that we will have removed all core files at the old place at this point so what is left in the relevant old folders will be custom stuff, and the folder deletion later will delete that at the old place after we have copied it because that how "Folder::delete" works, if deletes all sub-folders and files.
@dgrammatiko Here is a list of all folders and sub-folders which would be deleted. Do you think we should keep any custom file in all of these? Or are there certain sub-folders where we can exclude this, e.g. "vendor"?
administrator/templates/atum/css
administrator/templates/atum/css/system
administrator/templates/atum/css/system/searchtools
administrator/templates/atum/css/vendor
administrator/templates/atum/css/vendor/awesomplete
administrator/templates/atum/css/vendor/choicesjs
administrator/templates/atum/css/vendor/fontawesome-free
administrator/templates/atum/css/vendor/joomla-custom-elements
administrator/templates/atum/css/vendor/minicolors
administrator/templates/atum/images
administrator/templates/atum/images/logos
administrator/templates/atum/scss
administrator/templates/atum/scss/blocks
administrator/templates/atum/scss/pages
administrator/templates/atum/scss/system
administrator/templates/atum/scss/system/searchtools
administrator/templates/atum/scss/vendor
administrator/templates/atum/scss/vendor/awesomplete
administrator/templates/atum/scss/vendor/bootstrap
administrator/templates/atum/scss/vendor/choicesjs
administrator/templates/atum/scss/vendor/fontawesome-free
administrator/templates/atum/scss/vendor/joomla-custom-elements
administrator/templates/atum/scss/vendor/minicolors
templates/cassiopeia/css
templates/cassiopeia/css/global
templates/cassiopeia/css/system
templates/cassiopeia/css/system/searchtools
templates/cassiopeia/css/vendor
templates/cassiopeia/css/vendor/choicesjs
templates/cassiopeia/css/vendor/joomla-custom-elements
templates/cassiopeia/images
templates/cassiopeia/js
templates/cassiopeia/scss
templates/cassiopeia/scss/blocks
templates/cassiopeia/scss/global
templates/cassiopeia/scss/system
templates/cassiopeia/scss/system/searchtools
templates/cassiopeia/scss/tools
templates/cassiopeia/scss/tools/functions
templates/cassiopeia/scss/tools/variables
templates/cassiopeia/scss/vendor
templates/cassiopeia/scss/vendor/bootstrap
templates/cassiopeia/scss/vendor/choicesjs
templates/cassiopeia/scss/vendor/joomla-custom-elements
templates/cassiopeia/scss/vendor/metismenu
I think we could at least exclude that someone has added custom stuff to one of the present vendor folders, like "(s)css/vendor/joomla-custom-elements".
@dgrammatiko And what shall we do if someone has created a custom sub-folder, e.g. "templates/cassiopeia/css/foobar"? I guess we have to copy that, too.
And what shall we do if someone has created a custom sub-folder, e.g. "templates/cassiopeia/css/foobar"? I guess we have to copy that, too.
Yes, it's not only files but could be subfolders, thus I thought it would be easier to copy things as they are from the template/... to media/... and then do the update (the process will replace all the core files) so everything would be as if they were updating any patch version.
thus I thought it would be easier to copy things as they are from the template/... to media/... and then do the update (the process will replace all the core files)
@dgrammatiko Not sure about that, because of ongoing parallel development of 4.0.x and 4.1 it might be that we copy files which became obsolete during 4.0.x development and so would be deleted on update, but the files and folder deletion would delete them at the old but not at the new place. We would have to change the way how we generate these lists to handle that right. But I am still thinking about it.
If you want you can already have a PR from me with the files and folders deletion (but without the copying custom stuff). I have that ready here.
But I am still thinking about it.
I think we have some time here as this PR is depending on another PR which I have not even started yet
@dgrammatiko I've made a PR for you with the deleted files and folders update.
Other question: Cassiopeia has a "js" folder, but Atum doesn't. Shall we copy the "js" folder for Atum, too, if a user has added that folder?
Would it be easier to develop all the child template stuff in a branch?
Would it be easier to develop all the child template stuff in a branch?
Sure, the decision is to patch the com_templates with the missing functionality for child templates, thus the changes won't be so many. The other PR that merges the views and introducing new workflows was postponed for a later release, it's not abandoned.
Shall we copy the "js" folder for Atum, too, if a user has added that folder?
Hmm, good question, I guess we have to cover this case as well
@dgrammatiko I've made a PR for you for the custom files thing.
@richard67 that's awesome, thanks.
Now I should patch the com_templates the coming days so we can have some spare time for the docs/etc.
@dgrammatiko I'm not sure what "File:move" does when some parent folder doesn't exist, e.g. when we have to move a file "templates/cassiopeia/css/foo/bar/gaga.css" so folders "foo" and "foo/bar" would have to be created at the target path. I will check and if necessary provide a fix.
Update: I've updated my PR for you to handle this, too.
@dgrammatiko I still don't get why it needs to move the preview images. Can they really have been modified by a user for a core template and do we want to keep these modifications? If this is not the case there is no need to move them because they will be unzipped from the update package and be deleted with the file deletion, and so it does not need the special handling for them which I have added with my PR.
I still don't get why it needs to move the preview images.
You're right, we don't need some extra functionality here, the existing script.php could handle these files fine. Slow start...
@dgrammatiko See dgrammatiko#7 .
@dgrammatiko For testing instructions I suggest to test both, a new installation made with the branch of this PR, and an update of a 4.1-dev installation using the update package which has been built by drone for that PR. For the latter we could even say one should add some custom js or css folders and files and so we can test that they are moved to the right place on update. Could you update testing instructions in that way? Thanks in advance.
@richard67 I'm waiting for #35780 to solve atum and cassiopeia logo image urls (I was about to use HTMLHelper but if the other PR is merged I prefer to use the JLayout which is orders of magnitude more friendly for front enders). Also, this PR should not be merged before the com_template changes are in place (the other PR)
@dgrammatiko may be a bit off-topic but where would you put the x.webmanifest file?
I didn't reviewed the complete code so please forgive me, did you changed the favicon detection too?
@HLeithner so, x.webmanifest
is a file served directly from Apache/NginX/Etc so by definition for Joomla is treated as a static file so it should live in the media folder.
I didn't reviewed the complete code so please forgive me, did you changed the favicon detection too?
If I didn't do something stupid the favicons should already support child templates:
joomla-cms/libraries/src/Document/Renderer/Html/MetasRenderer.php
Lines 102 to 146 in 2e44061
I didn't reviewed the complete code so please forgive me, did you changed the favicon detection too?
If I didn't do something stupid the favicons should already support child templates:
joomla-cms/libraries/src/Document/Renderer/Html/MetasRenderer.php
Lines 102 to 146 in 2e44061
If I'm not wrong then this doesn't search in the media folder:
$foldersToCheck = [
JPATH_BASE,
JPATH_ROOT . '/media/templates/' . $client . $template->template,
JPATH_BASE . '/templates/' . $template->template,
];
@HLeithner so,
x.webmanifest
is a file served directly from Apache/NginX/Etc so by definition for Joomla is treated as a static file so it should live in the media folder.
of course this should be in the media folder ;-) but can it be overwritten by a child template?
If I'm not wrong then this doesn't search in the media folder:
Hmm, the second array entry: JPATH_ROOT . '/media/templates/' . $client . $template->template,
is the media folder path for the current template, so it should be covered
of course this should be in the media folder ;-) but can it be overwritten by a child template?
That's a good question. The HTMLHelper AFAIK only supports scripts, stylesheets and images. That said if you use the script method (I haven't tested this, so it might not be working) with the option to echo only the relative path and then add it to the links array it should be fine (assuming that the file is saved under the js
folder). If that (hack) doesn't work I guess we need to introduce another more generic function to get the relative functionality for other files (eg .webmanifest, .json, etc). The automatic resolution happens automatically only for the folders js
, css
, images
IIRC
If I'm not wrong then this doesn't search in the media folder:
Hmm, the second array entry:
JPATH_ROOT . '/media/templates/' . $client . $template->template,
is the media folder path for the current template, so it should be covered
haha next time I should better look at the code I post...
of course this should be in the media folder ;-) but can it be overwritten by a child template?
That's a good question. The HTMLHelper AFAIK only supports scripts, stylesheets and images. That said if you use the script method (I haven't tested this, so it might not be working) with the option to echo only the relative path and then add it to the links array it should be fine (assuming that the file is saved under the
js
folder). If that (hack) doesn't work I guess we need to introduce another more generic function to get the relative functionality for other files (eg .webmanifest, .json, etc). The automatic resolution happens automatically only for the foldersjs
,css
,images
IIRC
the other question is, is this a webasset?
Edit: and should be covered by the webassetmanager?
the other question is, is this a webasset?
It's a static asset so should be covered from weassets (IMHO)
Edit: and should be covered by the webassetmanager?
Yes but there's something deeper here, most apache configurations don't have the mime type enabled by default, so there are changes required in the server level (mime type and expire headers). Also one of the reasons we probably want this done through the webassets is the cache invalidation that the system provides (the ?12341325
)
Edit: and should be covered by the webassetmanager?
Yes but there's something deeper here, most apache configurations don't have the mime type enabled by default, so there are changes required in the server level (mime type and expire headers). Also one of the reasons we probably want this done through the webassets is the cache invalidation that the system provides (the
?12341325
)
Ok I think that's another PR, I'm asking my self why they created a new file extension... it's a json file.
Think about this I checked the W3C Editor Draft dated on 6.10.2021 which says
NOTE: File extension: .webmanifest or .json?
The IANA registered file extension for the manifest is .webmanifest. Some web servers recognize this extension and transfer the file using the standardized application manifest media type (application/manifest+json). Developers can also choose a different extension (e.g. .json) or none at all (e.g. /api/GetManifest), but are encouraged to transfer the manifest using the application/manifest+json MIME type, although any JSON MIME type is ok.
Which clearly says we could use .json files which would "allow us" to save it to the .js folder and have less problems with webservers.
Anyway the document is still in a draft state so we can can talk about this when it's finished. In the mean time the js folder should fit.
Ok I think that's another PR, I'm asking my self why they created a new file extension... it's a json file.
FWIW I always use .json for the manifest, eg https://github.com/dgrammatiko/sloth/blob/step-6-inherritable/media_src/site.json or the live part of the same file: https://sloth.dgrammatiko.dev/media/templates/site/sloth/site.json
Updated a 4.1 installation with the update package.
In both, atum and cassiopiea the preview.png and thumbnail.png are missing
In both, atum and cassiopiea the preview.png and thumbnail.png are missing
This is expected, the com_templates needs some changes to handle the new mode templates. There's a PR #35879 that fixes the thumbs, introduces the process for creating child templates but missing the file manager part (I'm currently working on it)
inheritable
was already set after I have installed the pr code. But got the following warning:
Warning
Could not find the file 'editor.css' in the template or templates/system folder. No styles are available.
The rest looks ok as I'v installed blog sample data and browsed around in the back- and frontend.
Warning
Could not find the file 'editor.css' in the template or templates/system folder. No styles are available.
What was your default editor?
Edit: Ignore my previous question. The error comes from some wrong conditional:
joomla-cms/plugins/editors/tinymce/tinymce.php
Lines 297 to 342 in a94decc
Should inheritable
be 1 after installation?
Should inheritable be 1 after installation?
Yes
Still getting the same error, did two times also npm ci
.
Still getting the same error, did two times also npm ci.
Is this for Autm?
No front end.
Admin template. No errors on frontend.
Category | Administration Templates (admin) NPM Change Repository SQL com_admin Postgresql | ⇒ | Administration com_admin SQL Postgresql com_templates Language & Strings Templates (admin) NPM Change JavaScript Repository |
Labels |
Added:
Language Change
|
Category | Administration Templates (admin) NPM Change Repository SQL com_admin Postgresql com_templates Language & Strings JavaScript | ⇒ | Administration com_admin SQL Postgresql Templates (admin) NPM Change Repository |
Labels |
Removed:
Language Change
|
Title |
|
From the installation page.
GET http://localhost/Joomla_4.1.0-dev+pr.35874-Development-Full_Package/build/media_source/vendor/fontawesome-free/webfonts/fa-solid-900.woff2
[HTTP/1.1 404 Not Found 1ms]
downloadable font: download failed (font-family: "Font Awesome 5 Free" style:normal weight:900 stretch:100 src index:1): status=2147746065 source: http://localhost/Joomla_4.1.0-dev+pr.35874-Development-Full_Package/build/media_source/vendor/fontawesome-free/webfonts/fa-solid-900.woff2
GET http://localhost/Joomla_4.1.0-dev+pr.35874-Development-Full_Package/build/media_source/vendor/fontawesome-free/webfonts/fa-solid-900.woff
[HTTP/1.1 404 Not Found 1ms]
downloadable font: download failed (font-family: "Font Awesome 5 Free" style:normal weight:900 stretch:100 src index:2): status=2147746065 source: http://localhost/Joomla_4.1.0-dev+pr.35874-Development-Full_Package/build/media_source/vendor/fontawesome-free/webfonts/fa-solid-900.woff
GET http://localhost/Joomla_4.1.0-dev+pr.35874-Development-Full_Package/build/media_source/vendor/fontawesome-free/webfonts/fa-solid-900.ttf
[HTTP/1.1 404 Not Found 1ms]
downloadable font: download failed (font-family: "Font Awesome 5 Free" style:normal weight:900 stretch:100 src index:3): status=2147746065 source: http://localhost/Joomla_4.1.0-dev+pr.35874-Development-Full_Package/build/media_source/vendor/fontawesome-free/webfonts/fa-solid-900.ttf
Oh, that’s npm related. I will check although I didn’t get any such message couple of hours ago
In \installation\template\css\template.css
, it references the build
directory.
@font-face {
font-family: "Font Awesome 5 Free";
font-style: normal;
font-weight: 900;
font-display: block;
src: url("../../../build/media_source/vendor/fontawesome-free/webfonts/fa-solid-900.eot");
src: url("../../../build/media_source/vendor/fontawesome-free/webfonts/fa-solid-900.eot?#iefix") format("embedded-opentype"), url("../../../build/media_source/vendor/fontawesome-free/webfonts/fa-solid-900.woff2") format("woff2"), url("../../../build/media_source/vendor/fontawesome-free/webfonts/fa-solid-900.woff") format("woff"), url("../../../build/media_source/vendor/fontawesome-free/webfonts/fa-solid-900.ttf") format("truetype"), url("../../../build/media_source/vendor/fontawesome-free/webfonts/fa-solid-900.svg#fontawesome") format("svg");
}
I have tested this item
@Krshivam25 what is broken?
@Krshivam25 When having tested without success, please report back what has not worked for you. Just marking a test as unsuccessful without any additional information is as helpful for us as a PITA.
@Krshivam25 what is broken?
I am unable find it
After applying patch I am unable to move or refresh
@Krshivam25 please download the installable from https://ci.joomla.org/artifacts/joomla/joomla-cms/4.1-dev/35874/downloads/48659/
I have no clue why your npm failed but it's probably irrelevant to the changes in this PR
Oh you CANNOT test this PR with patch tester
On the frontend, file not found. It should be in the Joomla_4.1.0-dev+pr.35874-Development-Full_Package
folder.
http://localhost/media/vendor/joomla-custom-elements/css/joomla-alert.css
Checked out this branch, went to sysstem->database
clicked on update structure.
Two errors
Call stack
# Function Location
1 () JROOT\administrator\components\com_admin\script.php:8508
2 JoomlaInstallerScript->moveRemainingTemplateFiles() JROOT\administrator\components\com_admin\script.php:7662
3 JoomlaInstallerScript->deleteUnexistingFiles() JROOT\administrator\components\com_installer\src\Model\DatabaseModel.php:323
4 Joomla\Component\Installer\Administrator\Model\DatabaseModel->fix() JROOT\administrator\components\com_installer\src\Controller\DatabaseController.php:57
5 Joomla\Component\Installer\Administrator\Controller\DatabaseController->fix() JROOT\libraries\src\MVC\Controller\BaseController.php:730
6 Joomla\CMS\MVC\Controller\BaseController->execute() JROOT\libraries\src\Dispatcher\ComponentDispatcher.php:146
7 Joomla\CMS\Dispatcher\ComponentDispatcher->dispatch() JROOT\libraries\src\Component\ComponentHelper.php:389
8 Joomla\CMS\Component\ComponentHelper::renderComponent() JROOT\libraries\src\Application\AdministratorApplication.php:143
9 Joomla\CMS\Application\AdministratorApplication->dispatch() JROOT\libraries\src\Application\AdministratorApplication.php:186
10 Joomla\CMS\Application\AdministratorApplication->doExecute() JROOT\libraries\src\Application\CMSApplication.php:278
11 Joomla\CMS\Application\CMSApplication->execute() JROOT\administrator\includes\app.php:63
12 require_once() JROOT\administrator\index.php:32
It might be unrelated - the first error prevented the sql from running hence the second error
@dgrammatiko thats what the database update structure should have done
ok - can confirm that the path not found error is also not related to this PR
I can also confirm that path is missing. Adding use Joomla\Filesystem\Path;
solved it for me. Probably a PR on its own?
Correction
The problem is in this PR with
protected function moveRemainingTemplateFiles()
It should NOT run when you are doing a database update structure
@brianteeman You should meanwhile know that the "Update Structure" button in the database view only updates structure, i.e. runs DDL (data definition language) statements and not updates content, i.e. not runs DML (data manipulation) statements. The update SQL scripts here do not contain any DDL, only DML ("UPDATE" statements). So these statements are not executed when using the button. The update SQL will be run when using the update package created by drone for this PR.
Either way this should not have happened and the problem code is in this pr
FWIW I added the missing import. For the logic in the script.php I think @richard67 is right, the update happens only when you use joomlaupdate or installing directly 4.1
@dgrammatiko When installing 4.1 directly, no update happens. Or did you mean install 4.1 without your PR and then update to the package with this PR? This will work.
I meant that installing 4.1 the db has the correct data (so no update is needed). The whole issue with the db is for those upgrading to 4.1 as they have to alter one of the template columns
Expected
This is the com_templates extension which needs the PR 2/3 to handle properly the templates in the new mode. Also, there's a PR for tinyMCE to handle correctly the editor.css and the HTML templates
This is the com_templates extension which needs the PR 2/3 to handle properly the templates in the new mode. Also, there's a PR for tinyMCE to handle correctly the editor.css and the HTML templates
I am just testing according to your instructions
Check that the back end template is not missing any assets and doesn't have any visual regression
I meant that installing 4.1 the db has the correct data (so no update is needed). The whole issue with the db is for those upgrading to 4.1 as they have to alter one of the template columns
@dgrammatiko Yes, that’s right. And by my review the changes in base.sql and the new update SQL scripts are correct and consistent.
@brianteeman So using the "Update Structure" button was not the right way to apply the update SQL for this PR, but it is good that you've tried it because it has shown the missing use statement.
The fact that the buttons runs also some stuff from script.php is an ugly hack which is a long standing issue, I think I remember that we even have an issue created by you here somewhere that this button does more than what it says. It is not related to this PR and should be solved somehow, but it's not an easy thing and I have no idea yet what's a good way to fix it.
@richard67 unrelated to this PR but I think for sake of sanity debugging anything on the script.php the long arrays need to be moved at the end of the file (eg with a fn getFiles, getFolders, etc). The file is a huge pain to even scroll to read the code... I always was in favour of separating the data from the actual code but as we all remember that didn't fly
@richard67 unrelated to this PR but I think for sake of sanity debugging anything on the script.php the long arrays need to be moved at the end of the file (eg with a fn getFiles, getFolders, etc). The file is a huge pain to even scroll to read the code... I always was in favour of separating the data from the actual code but as we all remember that didn't fly
@dgrammatiko I agree and was working on that, too.
Title |
|
@dgrammatiko I've just made an update from a 4.1-dev to the package of this PR using the custom URL provided by drone https://ci.joomla.org/artifacts/joomla/joomla-cms/4.1-dev/35874/downloads/48742/pr_list.xml, and it fails. I get the following error in my PHP error log:
PHP Fatal error: Uncaught Error: Class 'Joomla\\CMS\\Filesystem\\Path' not found in /joomla-cms-4.1-dev/administrator/components/com_admin/script.php:8509
Stack trace:
#0 /joomla-cms-4.1-dev/administrator/components/com_admin/script.php(7663): JoomlaInstallerScript->moveRemainingTemplateFiles()
#1 /joomla-cms-4.1-dev/administrator/components/com_joomlaupdate/finalisation.php(76): JoomlaInstallerScript->deleteUnexistingFiles()
#2 /joomla-cms-4.1-dev/administrator/components/com_joomlaupdate/extract.php(2035): finalizeUpdate()
#3 {main}
thrown in /joomla-cms-4.1-dev/administrator/components/com_admin/script.php on line 8509,
referer: https://www.joomla-41-dev.vmubu01.vmnet2.local/administrator/index.php?option=com_joomlaupdate&task=update.install&799181f416cdd0b202b9097d0fd2d515=1
I have no explanation for that because in the update package "Joomla_4.1.0-dev+pr.35874-Development-Full_Package.zip" the script.php file contains the necessary "use" statement which you had added with commit dc65503 , and the "libraries/src/Filesystem/Path.php" of course exists, too.
Update: Could be a problem on my environment only.
Update of update: No, it's not the permission problem I had on my environment.
So, it turns out that the use
is worthless in the finalization step (we don't have JLOADER at that point). We need to mock the Path here:
So, it turns out that the
use
is worthless in the finalization step (we don't have JLOADER at that point). We need to mock the Path here:
similar to the File, Folder that are already there.
@dgrammatiko You are right .. how could I forget about that? I should have known.
Let me know if you're happy doing that or I'll add it myself
I can do that, but that would create a dependency of this PR here to my PR. So maybe it's better if you or me do it with this PR here. If you want I can make a PR for you, but if you know how to do it, do it yourself, I am also happy with that.
@dgrammatiko Do we really need to use path clean here? In the files and folders removal as well as in the "fixFilenameCasing" we don't use it. The difference here now is that with your new method we deal with files and folders which we don't know in advance, so they might have weird names or be symbolic links to forbidden places, so it could make sense. But I'm not sure about that.
Do we really need to use path clean here?
I'm not sure if it's needed. Also it might be easier if we add a protected fn cleanPath with the code from
joomla-cms/libraries/src/Filesystem/Path.php
Lines 207 to 236 in 8aeae45
pravate function cleanPath($path)
{
$path = trim($path);
// Remove double slashes and backslashes and convert all slashes and backslashes to DIRECTORY_SEPARATOR
// If dealing with a UNC path don't forget to prepend the path with a backslash.
if ((DIRECTORY_SEPARATOR === '\\') && substr($path, 0, 2) === '\\\\')
{
$path = "\\" . preg_replace('#[/\\\\]+#', DIRECTORY_SEPARATOR, $path);
}
else
{
$path = preg_replace('#[/\\\\]+#', DIRECTORY_SEPARATOR, $path);
}
return $path;
}
@dgrammatiko Regarding simplify: It doesn't even need the trim. I remember now that we had to use the path clean here in order to get the same directory separators in the $oldPath
and the $oldFile
to make the substr($oldFile, strlen($oldPath)
thing work.
Possibly we could use realpath
from PHP instead to achieve the same?
@dgrammatiko I think that's it: Use realpath
instead of Path::clean
at these 2 places and remove the use Joomla\CMS\Filesystem\Path;
.
@richard67 let me know if you're ok with ebff3bb
@dgrammatiko Looks good. Will test and report back.
@dgrammatiko Now we run into the next one:
PHP Fatal error: Uncaught Error: Call to undefined method Joomla\\CMS\\Filesystem\\Folder::files() in /home/richard/lamp/public_html/joomla-cms-4.1-dev/administrator/components/com_admin/script.php:8512
So either we add that to the Folder mock class in finalisation.php, or we use glob
or readdir
here.
If glob is safe in this context I would personally go with that
If we solve this, the next one will be the Folder:create
.
@dgrammatiko I think "glob" won't be recursive into the subfolders,
@dgrammatiko Could we use "RecursiveDirectoryIterator" instead of "glob"?
So checking how things were done in the extrct.php I used mkdir
:
joomla-cms/administrator/components/com_joomlaupdate/extract.php
Lines 1220 to 1243 in 8aeae45
RecursiveDirectoryIterator
replaced the Folder::files()
.
@dgrammatiko I have made some debug output into script.php and tested the update. The iterator doesn't only contain files but also folders, including "." and "..". We have to filter those out so we have only files at the end.
@dgrammatiko Am working on a fix.
@dgrammatiko Possibly it might not work, see https://stackoverflow.com/questions/6337810/why-does-isdot-fail-on-me-php ... but I haven't tested yet, and I have to interrupt soon for an hour or two to do something else.
@dgrammatiko Maybe you just can use flag FilesystemIterator::SKIP_DOTS
when creating the directory iterator?
@richard67 I switched the code to use the flag. Docs say that dots are ignored by default, but yeah PHP...
@dgrammatiko My debug output shows there goes something wrong. I have the output just after the $newFile = $newPath . substr($oldFile, strlen($oldPath));
line.
DEBUG: oldFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/templates/cassiopeia/css/user.css",
newFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/media/templates/site/cassiopeia/css"
DEBUG: oldFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/templates/cassiopeia/images/MyImages/2021-11-27_colored.png",
newFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/media/templates/site/cassiopeia/images"
DEBUG: oldFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/templates/cassiopeia/js/gaga.js",
newFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/media/templates/site/cassiopeia/js"
DEBUG: oldFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/templates/cassiopeia/scss/global/gaga.scss",
newFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/media/templates/site/cassiopeia/scss"
Old file is right, new file is missing the part after the new parent folder.
Could be my mistake since I had proposed that code as far as I remember.
Update: No, it was the $oldPath = $oldFile->getPathname();
which you have added. It needs to remove that.
@dgrammatiko First the good news: Moving the custom files looks ok now. Here my debug output:
DEBUG: oldFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/templates/cassiopeia/css/user.css",
newFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/media/templates/site/cassiopeia/css/user.css"
DEBUG: oldFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/templates/cassiopeia/images/MyImages/2021-11-27_colored.png",
newFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/media/templates/site/cassiopeia/images/MyImages/2021-11-27_colored.png"
DEBUG: oldFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/templates/cassiopeia/images/MyImages/MyOtherImages/2021-11-27_monochrome.png",
newFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/media/templates/site/cassiopeia/images/MyImages/MyOtherImages/2021-11-27_monochrome.png"
DEBUG: oldFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/templates/cassiopeia/js/gaga.js",
newFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/media/templates/site/cassiopeia/js/gaga.js"
DEBUG: oldFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/templates/cassiopeia/scss/global/gaga.scss",
newFile="/home/richard/lamp/public_html/joomla-cms-4.1-dev/media/templates/site/cassiopeia/scss/global/gaga.scss"
And all files are present at the new location.
But after the update my backend looks like this:
Network analysis in browser's developer tools shows status 200 everywhere, all green. No errors in browser console.
And after this update npm ci
fails when compiling css:
Error: Can't find stylesheet to import.
╷
2 │ @import "../../../administrator/templates/atum/scss/variables";
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
╵
installation/template/scss/template.scss 2:9 @import
installation/template/scss/template-rtl.scss 1:9 root stylesheet
Update: This is expected behaviour since the update package doesn't update the installation folder. So pls. ignore this comment.
@dgrammatiko I think the CSS and probably also JS issues after an update could come from assets not being searched at the right place for some reason. Maybe with previous tests by other people this was not observed because applying the patch did not remove the old assets while the update package correctly removes the assets from the old place, and maybe with new installation all is fine?.
@richard67 is this #35920 related (should be the problem with missing media files in the update)
@richard67 is this #35920 related (should be the problem with missing media files in the update)
No, the media files are there in the update package built by drone because that's always the full update package while the issue exists only for the "small" update package not used by the live update.
Maybe the issue is that other successful testers used Windows and my server is on Linux?
@richard67 can you check that in the #__templates_styles
all the rows for cassiopeia and atum have the column parent set to 1
@dgrammatiko Ouch .. I should have known. The update did not run your update SQL script because the version in the file name is too old. Rename the scripts from "4.1.0-2021-10-22.sql" to "4.1.0-2021-11-28.sql" and it will work.
@dgrammatiko ... you know, in the 4.1-dev branch we meanwhile have update SQL with a newer date from the task scheduler, so the database schema version is already greater than the one of your update SQL so it does not run on update.
@dgrammatiko Yeah !!! I think that was it. Now I gotta make some 1 hour break due to dinner and then I will test this one and the other PR.
you know, in the 4.1-dev branch we meanwhile have update SQL with a newer date from the task scheduler, so the database schema version is already greater than the one of your update SQL so it does not run on update.
Now that you wrote it I realised that this was the problem. Finally this should be safe now
I am very optimistic now. CU later.
@dgrammatiko Loading the local Roboto fonts, i.e. fonts scheme "Roboto (local)" doesn't work. Fonts from web do work.
The network analysis in browser's developer tools shows an error loading for a resource which seems to be a version modifier without a file name, "?b3fc84e13caa5c536799085ad5741261".
@dgrammatiko The reason for the local roboto font not working is that you need to fix the path to the css file here: https://github.com/dgrammatiko/joomla-cms/blob/4.1-dev-child-templates/templates/cassiopeia/templateDetails.xml#L97
P.S.: We might need to update that path in the database in the template style settings, too. I will prepare an add-on for your update SQL scripts for that with a PR to your branch together with the fix for the XML.
@dgrammatiko See dgrammatiko#8 for the update SQL.
@richard67 thank you for the PR. Now the roboto font also should be fixed
I have tested this item
That was a long struggle but it was worth it.
I have tested this item
Tested on a 4.0 installation where an own template (derived from cassiopeia). After PR thePR template is unchanged, files of cassiopeia and atum are moved as described.
Status | Pending | ⇒ | Ready to Commit |
RTC
There is still time (GMT) for this to be merged on @dgrammatiko birthday if you're quick
@dgrammatiko Happy (belated) birthday.
So the name of the update SQL scripts is very suitable, it seems, if that’s the birthday of the author :-)
It is not possible to create overrides.
Failed to create override.
Go to System > Site Templates
Click Cassiopeia Details and Files
Click Create Overrides tab
Click mod_articles_archive
This is expected. This is the first of 2 PRs. To get the whole picture try to apply also the second pr
I had no problems creating an override, but it was for the language switcher module.
Applied the other PR and still cannot create overrides. Testing locally with WampServer on Windows 10.
No errors in Console and PHP error log.
@Quy How did you apply the patches?
If you are using patchtester, maybe that has a problem?
I had updated a clean 4.1-dev to the branch of this PR using the custom URL created by drone, and the same I did with the other PR (and then without this one here) on a clean 4.1-dev. After that I had tested both together by creating a new branch based on 4.1-dev, merging both branches of both PRs into that one and made a new installation using that branch (with composer install and nmp ci of course).
Using the prebuilt package and Joomla update to manually install the other PR.
Hmm, maybe an problem on Windows? I've tested with a Linux server.
@Quy it seems to work fine here
Also, the changes in this PR have nothing to do with the code in com_content that handles the creation of the overrides and on top of that the layout overrides didn't had any changes here (I mean the HTML directory wasn't moved). Can you try the same in a vanilla 4.1 installation? Maybe there's something else affecting permissions or there's a bug that you just discovered. My point is that this PR should not affect in any shape or form the functionality for creating layout overrides...
Tested fine with the 4.1.0 nightly build from today. https://developer.joomla.org/nightly-builds.html
No JS/PHP errors.
This installation includes the other PR. I get the same error with this PR by itself. I will try to troubleshoot later and report any findings.
The $override
parameter to createOverride
is empty in this PR but not in the 4.1 nightly build.
http://localhost/Joomla_4.1.0-dev+pr.35874-Development-Full_Package/administrator/index.php?option=com_templates&view=template&task=template.overrides&folder=Qzpcd2FtcDY0XHd3d1xKb29tbGFfNC4xLjAtZGV2K3ByLjM1ODc0LURldmVsb3BtZW50LUZ1bGxfUGFja2FnZVxtb2R1bGVzXG1vZF9hcnRpY2xlc19hcmNoaXZl&id=218&file=aG9tZQ&33c1bf54cc8ea6301b9a91a7b4e85521=1
But ONCE AGAIN: this is unrelated to what this PR is doing!!!. This PR just moves some static files, nothing to do with Overrides!!!
I don't want to waste any more of your time. Let's get this in and worry about it later if it persists. Thank you!
So, the part folder=...
is the base64 encoded part of the override that you asked the component to create
You said that the $override
(i guess in the controller) is empty. But if that is failing is due to this:
And this part wasn't touched by this PR thus my surprise here. This shouldn't fail with this PR...
The +
is the folder Joomla_4.1.0-dev+pr.35874-Development-Full_Package
caused the issue and unrelated to this PR. Sorry for the false alarm.
Wow, that's actually a very good catch!!
@HLeithner I think you need to rename the packages, it seems the +
will be problematic on Windows for some cases.
Labels |
Added:
?
Documentation Required
|
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-11-30 02:34:52 |
Closed_By | ⇒ | bembelimen |
Thx, awesome job @dgrammatiko and thanks for all the feedbacks
Thanks, @bembelimen also thanks to everyone contributing and testing here, especially @richard67 for the update patches!!
Wow, that's actually a very good catch!! @HLeithner I think you need to rename the packages, it seems the
+
will be problematic on Windows for some cases.
know your environment ;-)
@dgrammatiko Which SQL changes? I can't see any change in the PR yet for new installations, so I have no idea what to do on updates. And which folders moving? If the update packages contain what we ship with the core at the new place, that will be there after unzip, so we just have to delete the stuff at the old place post-flight with the regular files and folder deletion, or am I missing something?
Update: Or does custom stuff like user.css has to be moved, too?