- Issue created by @mariacha1
- First commit to issue fork.
- Status changed to RTBC
5 months ago 4:41pm 17 July 2024 - 🇦🇹Austria drunken monkey Vienna, Austria
drunken monkey → made their first commit to this issue’s fork.
- Merge request !156Resolve #3461936: Fix str_replace() called with NULL → (Open) created by drunken monkey
- Status changed to Needs review
5 months ago 1:00pm 20 July 2024 - 🇦🇹Austria drunken monkey Vienna, Austria
Thanks for reporting this issue, and thanks for posting a patch for fixing it.
However, since Drupal CI isn’t working anymore, please use MRs instead of patches for proposing changes. I now created an MR with the changes from your patch.However, instead of just adding
?? ''
I think we’d better explicitly check for an error condition and returnNULL
in that case.
As an alternative, we could also just use the original$text
in this case, skipping the stripping of<style>
/<script>
element contents – since the tags themselves will still be stripped, there should be no security problem, it might just lead to weird excerpts that contain highlighted Javascript or CSS – though only in the rare cases where thepreg_replace()
fails. (E.g., when a visitor searches for “function” they might get an excerpt with something like “… eslint-disable-next-line func-names, prefer-arrow-callback, no-shadow function (preloadFontPaths) { const basePath = drupalSettings.path.baseUrl; …”.)
What would you say to that suggestion? Otherwise, those items would end up with an empty excerpt, and I think avoiding those might be worth risking unhelpful/confusing excerpts in rare cases.