-
Notifications
You must be signed in to change notification settings - Fork 276
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
PHP 8.1 ltrim()
Compatibility Issue
#7809
Comments
I'll get a PR opened for a fix for this. I've tested it and a simple validation before the call is all that is needed. |
* Fixes google#7809 * Fixes an `ltrim()` deprecation warning in the Tag Manager module by adding a validation check.
Thank you for raising the issue @timnolte ! Can you provide a step to reproduce this issue? Cheers! |
I'm getting the error:
Environment: WordPress 6.4.1 on PHP 8.1.25. The error appears both in the dashboard and on the front-end. Here's a full stacktrace with identifying stuff redacted:
I'll note that this doesn't seem to be exactly the same as @timnolte's fix, so it's possible this error is in a few different places. Here is my Site Kit info from Tools > Site Health with identifying stuff removed
|
@mrwweb Thanks for your comment. We don't provide support via GitHub but we'd be delighted to assist you with this in the Site Kit support forum. Please can you open a new support topic and share the details you have done so here. Thanks and we look forward to helping you with this! |
@adamdunnage this isn't in need of some additional support forum request. This is a code issue that needs to be fixed, which I already provided a PR to fix this. The output that @mrwweb provided are the details to traceback this depreciation notice. This is a PHP 8.1 compatibility issue not an end user issue. |
@mrwweb just noticed that your stack trace does appear to call out the |
@timnolte Haven't done anything else. I saw the error, found this issue, and saw the request for a stacktrace and dumped it here. Hope it's useful! |
This issue still exists in Site Kit. I get the deprecation warning in both PHP 8.1 and 8.2 on every page load with stack trace results like originally reported. |
@kevinlisota Yes, this issue remains open at the moment, but it's on our list to fix in a future release. Feel free to follow along here for updates. Thanks! |
site-kit-wp/includes/Modules/Tag_Manager.php
Line 213 in b5b3f4a
Since the
trim()
call prior to theltrim()
call can returnnull
there is no verification check to ensure thatnull
isn't being passed toltrim()
and thus a deprecation notice is thrown.Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
ltrim
usage.Implementation Brief
Test Coverage
QA Brief
Changelog entry
The text was updated successfully, but these errors were encountered: