Fix: PHP 8.1 deprecated warning strpos()#5935
Fix: PHP 8.1 deprecated warning strpos()#5935getdave wants to merge 4 commits intoWordPress:trunkfrom
Conversation
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
Nope. I'm just trying to help things along. Thank you 👍 @youknowriad Can we commit this please? |
|
|
||
| if ( false !== strpos( $processor->get_attribute( 'class' ), $inner_block_wrapper_classes ) ) { | ||
| $class_attribute = $processor->get_attribute( 'class' ); | ||
| if ( is_string( $class_attribute ) && false !== strpos( $class_attribute, $inner_block_wrapper_classes ) ) { |
There was a problem hiding this comment.
Looks like you can make use of str_contains() here.
There was a problem hiding this comment.
+1 The polyfill version is available from 5.9. See
wordpress-develop/src/wp-includes/compat.php
Lines 468 to 474 in 6daf853
There was a problem hiding this comment.
Note to self: this will now require us to backport the usage of str_contains to the Gutenberg repo.
There was a problem hiding this comment.
There was a problem hiding this comment.
thanks for the update. generally I would encourage folks to think about separating the act of bringing over already-merged and accepted code from the act of changing it.
it can make it difficult to deal with when things go wrong because we've wrapped up a styling-preference change with what should otherwise be a somewhat mechanical copy operation.
another way this could have taken place is to create a PR in Gutenberg to update the code where it came from, and then port that change over, so we don't introduce divergence between the repos without a clear audit trail.
There was a problem hiding this comment.
That sounds like a better plan. I have:
- Reverted to match the changeset in Fix: PHP 8.1 deprecated warning strpos() gutenberg#56171.
- Updated Layout block supports use
str_containsgutenberg#58251 to make thestr_containschange suggested here.
Once this one is merged I will raise a new PR to backport the change from WordPress/gutenberg#58251.
This leaves a clear audit trail and avoid any further confusion.
There was a problem hiding this comment.
you're amazing. to be clear, I would have asked to change this PR if I thought it was important enough to reject, so thanks for going above and beyond to split these apart.
mostly I wanted us all to think about these kinds of things and the headaches that can happen when mangling them. "style-only changes" often introduce behavioral bugs and that's never fun in a crisis moment.
This reverts commit abc1865.
So we should be good to go 🤞 Please can this be approved so I can tick it off the backports list. Many thanks in advance 🙇 cc @youknowriad |
5 participants
Syncs changes from
str_containsgutenberg#58251...to Core.
Fix for PHP warning in Layout Block Supports.
Trac ticket: https://core.trac.wordpress.org/ticket/60327
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.