Do not attempt to register string resources for Smarty3+ #29104
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
It turns out this is not required for Smarty3 & it breaks for Smarty4.
Further, it is the only thing that breaks for Smarty4
Before
If I go to the smarty3 package and upgrade it to Smarty4 the page fails to load with
After
With this change Smarty4 works - as @demeritcowboy points out the code can also go for Smarty 3 "We should probably remove that line for smarty3 too? In smarty3 string resource is "native", in smarty2 it was something civi added that just happened to then be the same as what smarty added."
Technical Details
Comments
It turned out the step from Smarty3 to Smarty4 is actually very small indeed - this is the only thing I have hit
https://smarty-php.github.io/smarty/5.x/upgrading/#upgrading-from-v3-to-v4
We did make the define
'CIVICRM_SMARTY3_AUTOLOAD_PATH'
- which suggests perhaps we should add Smarty4 to it's own folder in packages & add a v4 version & when we are ready put that rather than 3 in composer.It's also possible that the leap to 5 will be sane...it's not stable yet though