Skip to content

Fix background-image styled tag visitor's handling of parsing style without background-image #1288

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.

Sign up for GitHub

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

Jump to bottom
Merged
westonruter merged 3 commits into from
Jun 7, 2024
Merged

Fix background-image styled tag visitor's handling of parsing style without background-image #1288

westonruter merged 3 commits into from
Jun 7, 2024

Conversation

@westonruter
Copy link
Member

@westonruter westonruter commented Jun 7, 2024

In a support forum topic, a user reported Image Prioritizer was causing a warning and fatal error in the Image_Prioritizer_Background_Image_Styled_Tag_Visitor class.

PHP Warning: Undefined array key “background_image” in .../image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php on line 44

PHP Fatal error: Uncaught TypeError: Image_Prioritizer_Tag_Visitor::is_data_url(): Argument #1 ($url) must be of type string, null given, called in .../image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php on line 46 and defined in .../image-prioritizer/class-image-prioritizer-tag-visitor.php:61 
Stack trace:
#0 /srv/data/web/vhosts/[domain removed]/htdocs/wp-content/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php(46): Image_Prioritizer_Tag_Visitor->is_data_url()
#1 /srv/data/web/vhosts/[domain removed]/htdocs/wp-content/plugins/optimization-detective/optimization.php(183): Image_Prioritizer_Background_Image_Styled_Tag_Visitor->__invoke()
#2 /srv/data/web/vhosts/[domain removed]/htdocs/wp-includes/class-wp-hook.php(324): od_optimize_template_output_buffer()
#3 /srv/data/web/vhosts/[domain removed]/htdocs/wp-includes/plugin.php(205): WP_Hook->apply_filters()
#4 /srv/data/web/vhosts/[domain removed]/htdocs/wp-content/plugins/optimization-detective/optimization.php(44): apply_filters()
#5 [internal function]: {closure}()
#6 /srv/data/web/vhosts/[domain removed]/htdocs/wp-includes/functions.php(5420): ob_end_flush()
#7 /srv/data/web/vhosts/[domain removed]/htdocs/wp-includes/class-wp-hook.php(324): wp_ob_end_flush_all()
#8 /srv/data/web/vhosts/[domain removed]/htdocs/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters()
#9 /srv/data/web/vhosts/[domain removed]/htdocs/wp-includes/plugin.php(517): WP_Hook->do_action()
#10 /srv/data/web/vhosts/[domain removed]/htdocs/wp-includes/load.php(1270): do_action()
#11 [internal function]: shutdown_action_hook()
#12 {main} thrown in /srv/data/web/vhosts/[domain removed]/htdocs/wp-content/plugins/image-prioritizer/class-image-prioritizer-tag-visitor.php on line 61

The issue is faulty logic here:

performance/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php

Line 42 in 5a2d306

false !== preg_match( '/background(-image)?\s*:[^;]*?url\(\s*[\'"]?\s*(?<background_image>.+?)\s*[\'"]?\s*\)/', $style, $matches )

Namely, preg_match() returns false only if there is a pattern compilation error. It returns zero (0) when no matches are made. So this should have rather been:

0 < (int) preg_match( '/background(-image)?\s*:[^;]*?url\(\s*[\'"]?\s*(?<background_image>.+?)\s*[\'"]?\s*\)/', $style, $matches )

Which handles both the error case and the non-match case.

@westonruter westonruter added [Type] Bug An existing feature is broken [Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) labels Jun 7, 2024
@westonruter westonruter added this to the image-prioritizer n.e.x.t milestone Jun 7, 2024
@westonruter westonruter marked this pull request as ready for review June 7, 2024 18:22
@westonruter westonruter requested a review from felixarntz as a code owner June 7, 2024 18:22
@github-actions
Copy link

github-actions bot commented Jun 7, 2024

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: westonruter <[email protected]>
Co-authored-by: thelovekesh <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions github-actions bot temporarily deployed to wp.org plugin: image-prioritizer June 7, 2024 18:27 Destroyed
@westonruter westonruter merged commit ec9762b into trunk Jun 7, 2024
@westonruter westonruter deleted the fix/bg-image-condtion branch June 7, 2024 18:31
@westonruter
Copy link
Member Author

westonruter commented Jun 7, 2024

Released 0.1.1 to WordPress.org.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thelovekesh thelovekesh thelovekesh approved these changes

@felixarntz felixarntz Awaiting requested review from felixarntz felixarntz is a code owner

Assignees

No one assigned

Labels

[Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) [Type] Bug An existing feature is broken

Projects

None yet

Milestone

image-prioritizer 0.1.1

Development

Successfully merging this pull request may close these issues.

3 participants

@westonruter @thelovekesh