E_DEPRECATED: htmlentities(): Passing null to parameter #1 ($string) of type string is deprecated

Yoast / wordpress-seo

Yoast SEO for WordPress

https://yoast.com/wordpress/plugins/seo/

Other

1.78k stars 895 forks source link

E_DEPRECATED: htmlentities(): Passing null to parameter #1 ($string) of type string is deprecated #21737

Open kkmuffme opened 1 month ago

kkmuffme commented 1 month ago

E_DEPRECATED: htmlentities(): Passing null to parameter #1 ($string) of type string is deprecated

PHP 8.3 wordpress-seo 23.7 => this issue wasn't there in 23.6

in wordpress-seo\src\images\Application\image-content-extractor.php

Maybe related to https://github.com/Yoast/wordpress-seo/issues/21723 but since there's no description I'm not sure.

josevarghese commented 1 month ago

Hi @kkmuffme

Thanks for creating the issue and for providing the PR. Our development team will review it and will provide you with more details.

Thanks in advance.

leonidasmi commented 1 month ago

Hi there @kkmuffme a question so that we can replicate as well:

How come you get a NULL for the $processor->get_attribute( 'src' ) operation?
If I'm not mistaken, the only way for $processor->get_attribute( 'src' ) to return NULL is to have an <img> element without an src attribute
And that's not valid HTML

So I was wondering if you have some pointers for us to be able to replicate

kkmuffme commented 1 month ago

Exactly, that's how it happens. I've run across 3 scenarios how that happens:

documentation uses an tag in markdown as example where there is no "src" tag there on purpose (= it's valid, since this <img isn't actually HTML but text documentation showcasing an example)
people mis-spelling the src tag (e.g. <img stc="...")
people messing up img tags, e.g. when refactoring text and the src,... is removed, but the img is left-over

Since I can't control what people do :-) and PHP deprecated notices are not a useful way to reach the target audience (= IT, but the HTML is from a user or a marketing dept) who created the invalid HTML, this should be handled to not throw a deprecated notice.

leonidasmi commented 1 month ago

Good point, I'm moving this forward then 👍

I've created a separate PR that is based on your PR, just so that I facilitate reviewing and potentially merging this (along with some props for you).