User tests: Successful: Unsuccessful:
Pull Request for Issue #38258 and partially for #38922.
This pr replaces the few calls to utf8_encode/utf8_decode to mbstring. It uses hardcoded the latin 1 character set (ISO-8859-1).
[Update]
It also makes the mbstring polyfill an explicit dependency, so we ensure that it still works on hosts, which do not have the mbstring module loaded.
If we should support all, then 'ISO-8859-1' can be replaced with mb_list_encodings(). See comment below why not!
Login in the frontend, and go to the profile user page.
A deprecated warning is shown.
No deprecated warning is shown.
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
Category | ⇒ | Front End com_users Installation Libraries |
Status | New | ⇒ | Pending |
I really like the idea to recheck if the code still makes sense and then remove what is not really needed. For now to improve compatibility I would prefer to replace the deprecated functions with alternatives.
From a maintenance perspective I would not add two helper functions which do basically replace 5 native function calls, even when it requires to change the compatibility requirements.
@laoneo I was thinking of a one-two approach. First add a drop-in replacement marked as @internal
and @deprecated 5.0
, then go through the (few, to be fair) instances where we use that function, thus removing the need for the drop-in replacement and the drop-in replacement itself.
This would be perfectly viable with regards to semantic versioning as @internal
clearly communicates “thou shalt not use” to 3PDs and @deprecated
sets the expectation for dropping the darned thing on a very short horizon.
Why do that instead of just doing the second step? To make sure that real world use of the replacement doesn't cause any issues in use cases we have never thought of or could have expected just by looking at the code. I am a bit conservative here but I have seen… THINGS
Category | Front End com_users Installation Libraries | ⇒ | Front End com_users External Library Composer Change Installation Libraries |
Labels |
Added:
?
|
Labels |
Added:
Composer Dependency Changed
|
Title |
|
Labels |
Added:
PHP 8.x
|
I have tested this item
For test it. Login in the frontend, and go to the profile user page
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
I restored test result from @carlitorweb because it was lost for some reasons. RTC
Labels |
Added:
?
|
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-01-14 11:01:41 |
Closed_By | ⇒ | roland-d |
Thank you
No, we absolutely shouldn't!
The reason
utf8_decode
was deprecated is that it only ever supported ISO-8859-1 as per https://wiki.php.net/rfc/remove_utf8_decode_and_utf8_encode in the Introduction at the very top of the page (emphasis mine):I am unwilling to consider this PR a good solution because
mbstring
is actually not a hard requirement for Joomla 4 per the published technical requirements. I know it's nonsensical and probably a mistake on that page.Unless that page is amended to list mbstring as a hard requirement it makes far more sense to follow the approach in #38922 with a function which uses whatever is actually available — including a fallback to a pure PHP implementation. Do note that the code in #38922 is in fact following the “Throwing the kitchen sink at it” section of the PHP RFC for
utf8_decode
deprecation, i.e. the “official” way PHP core developers want us to polyfill this deprecated feature. It also usesutf8_decode
on older PHP versions to remove surprises, so there's that.As a second stage we should look at every instance of
utf8_decode
and decide if it actually makes sense to be doing that (most of that code came from early PHP 5 versions which didn't support UTF8 natively) and whether we actually meant to use ISO-8859-1 (e.g. when handling URLs that's deliberate) instead of blindly trying to use whatever encoding. I believe that in most cases we'll just end up removing the decoding altogether.