Image Block: Replace stripos with str_contains

[mikachan]

What?

Small PR to replace stripos with str_contains, as per feedback from @spacedmonkey here: WordPress/wordpress-develop#5468.

Follow-up to #55269.

[ockham]

Thank you!

Quoting my WordPress/wordpress-develop#5468 (comment):

[...] strictly speaking, the code isn't 100% equivalent, since stripos is case-insensitive, whereas str_contains isn't. While block editor-generated code shouldn't contain non-lowercase HTML tags, we'd need to wrap $content in strtolower() to be 100% equivalent.

For all practical purposes, we probably don't need case-sensitivity here; might just be interesting to know if there was any scenario that necessitated using stripos over strpos when this was first introduced here.

[mikachan]

For all practical purposes, we probably don't need case-sensitivity here; might just be interesting to know if there was any scenario that necessitated using stripos over strpos when this was first introduced here.

Thanks @ockham! Pinging @artemiomorales @michalczaplinski to double-check this 🙇

[hellofromtonya]

Advantages of str_contains():

• more readable, thus understandable.
• less code as it does not need an explicit false check to determine if the substring does not exist.

Some cons:

• it is case-sensitive.
• it is not multibyte safe.

I advise caution in switching false === stripos() to str_contains() for these reasons:

• stripos() is case-insensitive. Thus, when it's used, it may be intentionally designed to deal with case-insensitive edge cases. That may or may not be the case here, but it should cause us to pause to go find out.
• If it usage is by design, then to switch will require a strtolower(), which IMO negates the benefits of switching to str_contains().
• Core is full stripos(), quite common.
• The current code with stripos() was tested. Switching it this late in the cycle should only be done if the change is also well tested with high confidence.

To sum up:
Is there an edge case that could require case-insensitivity? (meaning its usage is by design)

• If yes, close this PR.
• If maybe, wait until there's certainty that there are no edge cases. If there's confidence, then merge this PR; else, wait.

Also noting, this change is not required for 6.4 RC1. It could happen later without effect.

[dmsnell]

thanks for the contribution, and the intention here. I can see where it would be tempting to make this replacement, but since HTML tag names are case insensitive this was chosen to avoid falsely asserting that there are no IMG elements when there might be.

this test is only asking "is it possible that there is an IMG element?" and the only confident answer it can give is "there are definitely no IMG elements in this HTML," except with the change to str_contains() it cannot answer that question and introduces a defect.

coincidentally, the fear that someone might assume this is a code styling violation and accidentally introduce this very bug was discussed yesterday. I take fault for adding the optimization without measurement, but I plan on performing a proper analysis when I can and then if it proves valuable enough I'll add a more clear warning not to arbitrarily change the code.

[dmsnell]

Closing this because it introduces a bug without solving any problems, as it was likely a misunderstanding that led to the proposal.