explode(): Passing null to parameter #2 ($string) of type string is deprecated

Can we open this as a merge request?

https://www.gregboggs.com/drupal-merge-requests/

I am uploading a patch for this issue

It looks like there are now two very different patches for this issue and the first patch looks a lot more involved. Could we open a merge request with what you think is the best solution?

https://www.gregboggs.com/drupal-merge-requests/

I'm using PHP 8.1.7 and am not able to reproduce this issue.

Reviewed patches #3 and #8.
About #3 - I don't see the reason to implement so many changes in this case.
About #8 - in general, the patch looks good, but we don't need to change lines with $normalized_text. $normalized_text can't be NULL in this function. We just need

$ignored_words = explode(' ', $ignored_words ?? '');

How can we replicate the issue? Only when EasyBreadcrumbConstants::CAPITALIZATOR_IGNORED_WORDS (capitalizator_ignored_words) option of the config isn't exist or NULL.
This means that under normal circumstances the problem should not occur. Therefore, I am not sure that we need to fix it.
But I have prepared a simpler patch if we do need to fix it.

P.S. It would be nice if someone else confirmed/denied my words.

@HitchShock. Your logic does make sense, but I would feel safer having the null coalescing operator for any use of `explode()`.

I would defer to the wisdom of the module maintainer.

In the meantime, I created the MR as they requested in #4

@philltran I don't see the reason to worry :)
$normalized_text couldn't be NULL because it prepares from $raw_text and $raw_text could not be empty because of

if (empty($raw_text)) {
  return '';
}

I agree with #16 on this one. Maybe right now we don't need to worry about $normalized_text , but it is always good to ensure that the code cannot break for any reason.
I reviewd the MR and everything seems to be looking good. I'm moving this to RTBC so the maintainer can look at the issue and provide a final decision.

Thanks for all the work on this one. It looks good. Has anyone tested this one on a Drupal site with PHP 8.1 yet? I am not certain we have.

I am testing this patch on 2.x-dev#92ec1706 with PHP 8.1 and D9.4.6. So far so good.

1. You have to update the config/sync/easy_breadcrumb.settings.yml and import it:

+alternative_title_field: ''
...
-capitalizator_ignored_words: ''
-capitalizator_forced_words: ''
+capitalizator_ignored_words: {  }
+capitalizator_forced_words: {  }
...
+follow_redirects: false
+limit_segment_display: false
+segment_display_limit: null
+truncator_mode: false
+truncator_length: null
+truncator_dots: false

2. Or you have to edit the config / settings page of the module easy_breadcrumb and save it. (then you can export the configurations) And, all the PHP 8.1 related errors will not exist anymore.

I've also tested on a site that showed this error before the patch. It looks good now.

With PHP 7.4 being EOL, I think it's time to commit this.

Unfortunately, this one isn't mergeable yet because the MR needs to be updated.

"Merge blocked: the source branch must be rebased onto the target branch."

If you're testing the patches, you're testing the wrong code. To test this one, we must test the fork from the merge request because that's what will end up in the module.

Thanks yall!

Reiterating #21, after applying the updated code, you need to click Save on /admin/config/user-interface/easy-breadcrumb to update the settings and then re-export (drush cex) your config files. In my case this resulted in the config following changes (not having changed any settings, just clicking Save to update):

+alternative_title_field: ''
+menu_title_preferred_menu: ''
-capitalizator_ignored_words: null
+capitalizator_ignored_words: { }
+limit_segment_display: false
+segment_display_limit: null
+truncator_mode: false
+truncator_length: null
+truncator_dots: false
+remove_repeated_segments_text_only: 0
+home_segment_validation_skip: 0

Perhaps there should be an update script that makes these changes automatically?

Definately sounds like we need an update hook to save the form

Was this ever actually committed/merged? I don't see the MR changes reflected in the latest 2.x branch.

I don't think this was ever committed or released.

Here's the merge request: https://git.drupalcode.org/project/easy_breadcrumb/-/merge_requests/48/d...

It doesn't look like it is on the 2.x branch: https://git.drupalcode.org/project/easy_breadcrumb/-/blob/2.x/src/EasyBr...

And not in the 2.0.5 tag: https://git.drupalcode.org/project/easy_breadcrumb/-/blob/2.0.5/src/Easy...

There should be a comment automatically posted in this issue whether a new commit with a #-reference to this issue in the commit summary, or the MR is merged with the project. I do not see such comment.

Now the MR is Ready to merge by members who can write to the target branch.

Awesome, thank you @apaderno !

Thanks for catching this. Sometimes the merge fails and I have to repress the button, I must have missed it on this one.