Conversation
…processor and add tests
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| // Find the ID of each image by the class. | ||
| // TODO: It would be preferable to use the $processor->class_list() method but there seems to be some typing issues with PHPStan. | ||
| $class_name = $processor->get_attribute( 'class' ); | ||
| if ( ! is_string( $class_name ) || ! preg_match( '/(?:^|\s)wp-image-([1-9]\d*)(?:\s|$)/i', $class_name, $matches ) ) { |
There was a problem hiding this comment.
This is more robust than before since it constraints looking for the class name inside the class attribute. Previously the string could be anywhere in the img tag.
| // Make sure we use the last item on the list of matches. | ||
| $attachment_id = (int) $class_name[1]; | ||
|
|
||
| if ( ! $attachment_id ) { |
There was a problem hiding this comment.
This will never be zero because the regex is now [1-9]\d*.
| } | ||
|
|
||
| // This content does not have any tag on it, move forward. | ||
| // TODO: Eventually this should use the HTML API to parse out the image tags and then update them. |
There was a problem hiding this comment.
or, switch to using the wp_content_img_tag filter? #1259
|
Looks good! some test failing? |
|
@adamsilverstein I re-ran the failed jobs. They're passing now. Seems like a wp-env hiccup. |
| is_string( $style ) | ||
| && | ||
| 0 < (int) preg_match( '/background(-image)?\s*:[^;]*?url\(\s*[\'"]?\s*(?<background_image>.+?)\s*[\'"]?\s*\)/', $style, $matches ) | ||
| 1 === preg_match( '/background(?:-image)?\s*:[^;]*?url\(\s*[\'"]?\s*(?<background_image>.+?)\s*[\'"]?\s*\)/', $style, $matches ) // Not compatible with strict rules. See <https://github.com/phpstan/phpstan/issues/11262>. |
There was a problem hiding this comment.
Bah. I should have removed the comment as it is now irrelevant after 75cd653 per phpstan/phpstan#11262 (comment). I'll do so as part of another PR.
This upgrades PHPStan to 1.11.6 which introduced two new issues.
This is a case where PHPStan is not aware that at this point, the array keys are actually not nullable at this point. This is a bug in PHPStan which I've filed as phpstan/phpstan#11262. The issue seems to be that if the return value of
preg_match()is casted to aboolorintthen the array shape doesn't come through. So it's now checking to see if the return value is exactly1instead. Additionally, another check for'' !== $matches['background_image']will be able to be removed once phpstan/phpstan#11222 is implemented, since we'll know that the array shape here has anon-empty-string.There was addressing the second
ifstatement in the following code:performance/plugins/webp-uploads/hooks.php
Lines 542 to 549 in b71d0a0
It is a useless condition because since
preg_match()is known to have succeeded, the$class_namewill never be empty.This fixes the issue by removing that condition, but also makes the code more robust by leveraging the HTML Tag Processor. It also adds missing test assertions for
webp_uploads_update_image_references().