-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[5.4] Fix deprecated direct property access of model state #45704
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
base: 5.4-dev
Are you sure you want to change the base?
Conversation
I have tested this item ✅ successfully on cdeadcf 1 comment/inquiry (I apologize in advance if my inquiry is stupid: while the messages Direct property access will not be supported in 7.0 in Joomla\CMS\MVC\Model\State:: etc are no longer in the deprecated.php log :D, I am noticing one quite similar (I understand it is different) and out of doubt, I prefer mentioning it in case it mattered: deprecated Instead of an empty value, the default value will be returned in 7.0 in Joomla\CMS\MVC\Model\State::get::Joomla\CMS\MVC\Model\State. - [ROOT]/libraries/src/MVC/Model/State.php - Line 56 (I am seeing a LOT of those as well) Is that a side-effect of this PR, or completely irrelevant/unrelated? Sorry, again for the dumb question. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45704. |
@exlemor No worries at all - it's a valid question! In this case, it's not relevant to this PR, so no need to worry about it here. But it looks like the code does not match the registry class. It compares against null and an empty string, while the deprecation message uses PHP’s empty() function, which also returns true for 0 (int/float) and empty arrays. @laoneo, should a PR be created? joomla-cms/libraries/src/MVC/Model/State.php Lines 53 to 64 in 36516f6
https://github.com/joomla-framework/registry/blob/4452de9c66abfec59b535e27c62af3528a539311/src/Registry.php#L207-L209 |
If I 'm not wrong, then made it like that because of bc reasons. |
Hello @richard67, I had followed instruction and stupid me I assumed it was a mistake (the double line) and I had not but I JUST did a second ago and validate that the message goes away with this PR which I just redownloaded as a Download Package so all good :). |
Yes your right |
I have tested this item ✅ successfully on 9aa1341 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45704. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45704. |
Summary of Changes
Remove direct property access of model state
Change API model state also to State|Registery (missed in #39663)
Testing Instructions
Enable

Log Deprecated API
in Global ConfigurationVisit different views
Check log file
logs/deprecated.php
Actual result BEFORE applying this Pull Request
direct property access of model state
deprecated Direct property access will not be supported in 7.0 in Joomla\CMS\MVC\Model\State::__get::Joomla\CMS\MVC\Model\State. - [ROOT]\libraries\src\MVC\Model\State.php - Line 94
Expected result AFTER applying this Pull Request
no direct property access of model state
Link to documentations
Please select: