-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[4.3] Fix invalid json in params of com_templates after update to 4.3.0 or 4.3.1 caused by PR #38823 #40535
[4.3] Fix invalid json in params of com_templates after update to 4.3.0 or 4.3.1 caused by PR #38823 #40535
Conversation
I have tested this item ✅ successfully on e10d7d7 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40535. |
I have tested this item ✅ successfully on e10d7d7 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40535. |
@degobbis @TheWhykiki Sorry, my mistake. It should not be an empty JSON. I will provide a fix in a minute. |
@degobbis @TheWhykiki Could you test again with the latest changes? I've updated the link to the prebuilt packages in the description. The params should not be an empty JSON but the values used for a new installation. I've updated the expected result accordingly. Thanks in advance, and thanks for testing before. |
I have tested this item 🔴 unsuccessfully on 1c85bb0 Testet with Updateserver URL and both packages. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40535. |
I have tested this item 🔴 unsuccessfully on 1c85bb0 from 3.10.11 to this -> params are entered, all OK Testet with Updateserver URL and both packages. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40535. |
@degobbis @TheWhykiki There is nothing I can change on that. When people have the invalid JSON after an update from 3.10, the update component will not work anymore. They have to apply the SQL change manually to fix their broken installation. It might need an FAQ document for that. This PR here can only fix make it not happen anymore when updating from 3.10. |
I have tested this item ✅ successfully on 1c85bb0 from 3.10.11 to this -> params are entered, all OK PS: it would be enough to simply save the options in the configuration for templates again This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40535. |
@degobbis Not really, because when you have the problem with the invalid JSON you can't go to the template manager anymore to save the options. |
I have tested this item ✅ successfully on 1c85bb0 from 3.10.11 to this -> params are entered, all OK This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40535. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40535. |
Thank you for the fix Richard @richard67. And thank you for the extended explanations, it really helped understand the issue at hand. Also, thank for the comments in the code, it will help keep a trace of the issue. |
Thanks all. |
Pull Request for https://forum.joomla.org/viewtopic.php?f=808&p=3690614 (also reported on social media and at other places)
Summary of Changes
When updating from 3.10 or 4.x to 4.3.0 or later, and before the update the value of the
params
column ofcom_templates
is an empty JSON{]
in the#__extensions
table in database, you get an error "error decoding JSON data: Syntax error" when you try to go to the template manager in backend.This is caused by the update SQL script which was introduced with pull request (PR) #38823 , which produces invalid JSON for that special case.
Note that when you have an empty
params
forcom_templates
, the template manager is broken when using Joomla 4 with PHP 8.1. Details and fix see PR #40544 . But when you are coming from 3.10 or from 4.0 with PHP 7.4 you won't have noticed any problems with the template manager.The issue fixed here is for a small part my fault because I have not noticed the problem when having an empty JSON params when having quickly reviewed that PR .
This PR here fixes this by 2 means:
In addition it fixes a typo here, which also was introduced with PR #38823: https://github.com/joomla/joomla-cms/blob/4.3-dev/administrator/components/com_templates/tmpl/template/default.php#L152 . It should be
'SideBySide'
.The reason for fix number 1 is that we should modify JSON parameters only when there is the need to fix broken functionality, like it is e.g. here: https://github.com/joomla/joomla-cms/blob/4.3-dev/administrator/components/com_admin/sql/updates/mysql/4.1.0-2021-11-28.sql#L7 . This example is safe and does not do anything when having an empty JSON params.
But in case of PR #38823 it was not necessary, it was only nice to have the side-by-side view switched on after an update.
It's a different thing if we don't modify but completely replace the params value, like e.g. here: https://github.com/joomla/joomla-cms/blob/4.3-dev/administrator/components/com_admin/sql/updates/mysql/4.0.0-2021-05-30.sql#L3 .
But as said, with string replacement for JSON parts only we have to be careful and avoid that were we can.
The reason for fix number 2 is because when you have the problem, your template manager params were an empty JSON before that update which broke it.
So we could safely set the params to the values for new installations as obviously the params have never been saved in backend.
It would have been easier and maybe better to revert to an empty params value.
But as said before, the template manager is broken in Joomla 4 if you set the params of com_templates to an empty string or an empty json in the extensions table as we can do it safely with other components, see PR #40544 .
For the same reason and because the template manager is not really usable when there are not any allowed file name extensions, the SQL update script introduced with this PR sets the params not only for the broken JSON but also for an empty JSON and an empty string.
Testing Instructions
Test 1: Reproduce the issue
On a 3.10.11 or current 3.10-dev branch, make a new installation.
Set the
params
value to an empty JSON in database, e.g. with phpMyAdmin:Result: See section "Actual result BEFORE applying this Pull Request" below.
Test 2: Test the fix for the issue
Proceed as described in the previous test "Test 1", but this time update to the patched package created by Drone for this PR.
You can find the update package or the custom update URL here:
https://ci.joomla.org/artifacts/joomla/joomla-cms/4.3-dev/40535/downloads/65417/
Result: See section "Expected result AFTER applying this Pull Request" below.
Test 3: Review the other change
Verify that the change here is correct: https://github.com/joomla/joomla-cms/pull/40535/files#diff-190cfe05a7ca67886b08e825520e05421b44ccd8d0bf048b55fc7165f187ea59R152
I.e. compare with the casing of the default value used here:
https://github.com/joomla/joomla-cms/blob/4.3-dev/administrator/components/com_templates/config.xml#L77
And here:
https://github.com/joomla/joomla-cms/blob/4.3-dev/installation/sql/mysql/base.sql#L174
Actual result BEFORE applying this Pull Request
Error "error decoding JSON data: Syntax error".
Further symptoms see https://forum.joomla.org/viewtopic.php?f=808&p=3690614 .
Expected result AFTER applying this Pull Request
No error "error decoding JSON data: Syntax error".
The
params
value ofcom_templates
in the#__extensions
table is equal to the value used for new installations e.g. here: https://github.com/joomla/joomla-cms/blob/4.3-dev/installation/sql/mysql/base.sql#L174 ,Link to documentations
Please select:
No documentation changes for docs.joomla.org needed
No documentation changes for manual.joomla.org needed