entity_browser_entity_form_field_widget_third_party_settings_form hooks can no longer return NULL

Merge request created.

This could probably use a test.

Could an additional assertion be added somewhere?

A test would require another hook that returns nothing. I'm not sure that's really worth it.

I'm also not sure we actually want to support this. we could also explicitly deprecate a non-array return value.

either way, leaving at needs work to update the second place

Updated the second place. I think it would be nice to get this in before 11.2 and avoid fatal errors. I can fix entity_browser but others might be affected too.

If we don't add tests could we add steps for manual testing to trigger this error?

Added STR, I don't remember 100% if it already happens when you visit the form display page or if you need to click the gear icon of a given field, but one or the other should reproduce the error.

Just visiting the Manage form display caused the fatal error. "TypeError: Unsupported operand types: array + null in Drupal\field_ui\Form\EntityFormDisplayEditForm->Drupal\field_ui\Form\{closure}() (line 131 of core/modules/field_ui/src/Form/EntityFormDisplayEditForm.php)."

MR did solve the problem

Wasn't even using entity_browser just installed it.

catch → credited quietone → .

I think we need a follow-up to either add a test hook implementation that triggers the bug somewhere (probably an existing test would do fine - just enable the module so it explodes without the fix), or to properly deprecate a void return value from the hook and require an array.

This is a clear example where the new test guidelines from https://www.drupal.org/about/core/policies/core-change-policies/core-gat... → apply for me - easy to reproduce, easy to fix, doesn't add any new untested code paths because that ternary executes all the time anyway. Maybe we should add a line like 'Fixes a recent regression which for any reason is not suitable to rollback in the target branch' too.

Committed/pushed to 11.x and cherry-picked to 11.2.x, thanks!

Once the follow-up is open would be good to link from here.

catch → closed merge request !11551

Just so the followup doesn't get lost. Normally we would ask for that before commit but this was about hotfixing a regression in the minor before said minor was out. :)