TypeError: explode(): Argument #2 ($string) must be of type string, array given in explode() (line 65 of modules/contrib/media_library_form_element/src/Element/MediaLibrary.php).

Hi, thanks for the bug report!

If I'm not mistaken, instead of using a null-coalesce operator there, it should actually be a short-hand ternary.

Instead of

$entity_ids = array_filter(explode(',', $default_value ?? ''));

this might work better:

$entity_ids = array_filter(explode(',', $default_value ?: ''));

Updated patch #1 by addressing #2, please review it.

Thanks!

re: #4, I don't think we need the new if check, but rather just changing ?? to ?:.

Check out https://www.php.net/manual/en/language.operators.comparison.php#example-113 for more information on how this php language feature should work.

Expression expr1 ?: expr3 evaluates to the result of expr1 if expr1 evaluates to true, and expr3 otherwise. expr1 is only evaluated once in this case.

For a concrete example, print_r([] ?: 'Quack!'); will output 'Quack!' because [] evaluates to false.

if (!is_array($default_value)) {
  $entity_ids = array_filter(explode(',', $default_value ?? ''));
}

This is fast workaround

Created issue fork and MR to update the null-coalesce operator to a short-hand ternary as suggested by Luke.Leber. PRetty simple change that I can confirm resolved the issue on my end.

Thanks, @shaunole. That change looks good, but I have no idea why your MR didn't trigger a test run.

I'm digging into the testing config for this project to see if I can come up with anything.

No problem @luke.leber. I was reviewing my commit in GitLab and initiated the Merge Request there instead of in this issue. Perhaps it was not triggered due to the steps I took to create it.

The project only had test configurations set up to run bi-weekly. I added a new configuration to run against php8.2 | drupal 10 development on commit and for issues.

I even kicked the MR to get the gears spinning :-)

Seems like the error still exists after patching via composer. I've confirmed that the patch correctly changed ?? to ?:. The tests passed, so it appears more testing may be necessary.

It could be that null coalescing operators are misused in more than one spot. Fatal errors often mask other fatal errors with language level breaking changes.

Either that or I've finally gone bonkers, which is equally possible.

It would be really nice to get a test to fail with this. Can we isolate the reproduction steps?

@Luke.Leber have you tried to run print_r([] ?: 'Quack!'); with non-empty array?
Let's keep is_array or is_string check!

Here is our example of "offencive" module that brings this problem too.
$default_value:

^ array:3 [
  "media_library_open_button" => Drupal\Core\StringTranslation\TranslatableMarkup {#2712
    #string: "Add media"
    #arguments: []
    #translatedMarkup: null
    #options: []
    #stringTranslation: null
  }
  "media_library_selection" => ""
  "media_library_update_widget" => Drupal\Core\StringTranslation\TranslatableMarkup {#2713
    #string: "Update widget"
    #arguments: []
    #translatedMarkup: null
    #options: []
    #stringTranslation: null
  }
]

Gives:
TypeError: explode(): Argument #2 ($string) must be of type string, array given in explode() (line 68 of /var/www/html/docroot/modules/contrib/media_library_form_element/src/Element/MediaLibrary.php).

with such $element

"#description" => Drupal\Core\StringTranslation\TranslatableMarkup {#3171
    #string: "Background image"
    #arguments: []
    #translatedMarkup: null
    #options: []
    #stringTranslation: Drupal\Core\StringTranslation\TranslationManager {#303}
  }
  "#allowed_bundles" => array:1 [
    0 => "image"
  ]
  "#default_value" => array:3 [
    "media_library_open_button" => Drupal\Core\StringTranslation\TranslatableMarkup {#2712
      #string: "Add media"
      #arguments: []
      #translatedMarkup: null
      #options: []
      #stringTranslation: null
    }
    "media_library_selection" => ""
    "media_library_update_widget" => Drupal\Core\StringTranslation\TranslatableMarkup {#2713
      #string: "Update widget"
      #arguments: []
      #translatedMarkup: null
      #options: []
      #stringTranslation: null
    }
  ]
  "#states" => array:1 [
    "visible" => array:1 [
      ":input.bs_background--type" => array:1 [
        "value" => "image"
      ]
    ]
  ]
  "#input" => true
  "#tree" => true
  "#cardinality" => 1
  "#process" => array:4 [
    0 => array:2 [
      0 => "Drupal\ui_patterns_settings\Plugin\PatternSettingTypeBase"
      1 => "formGroupProcess"
    ]
    1 => array:2 [
      0 => "Drupal\media_library_form_element\Element\MediaLibrary"
      1 => "processAjaxForm"
    ]
    2 => array:2 [
      0 => "Drupal\media_library_form_element\Element\MediaLibrary"
      1 => "processMediaLibrary"
    ]
    3 => array:2 [
      0 => "Drupal\media_library_form_element\Element\MediaLibrary"
      1 => "processGroup"
    ]
  ]
  "#pre_render" => array:1 [
    0 => array:2 [
      0 => "Drupal\media_library_form_element\Element\MediaLibrary"
      1 => "preRenderGroup"
    ]
  ]
  "#element_validate" => array:1 [
    0 => array:2 [
      0 => "Drupal\media_library_form_element\Element\MediaLibrary"
      1 => "elementValidateMediaLibrary"
    ]
  ]
  "#theme" => "media_library_element"
  "#value_callback" => array:2 [
    0 => "Drupal\media_library_form_element\Element\MediaLibrary"
    1 => "valueCallback"
  ]
  "#defaults_loaded" => true
  "#parents" => array:6 [
    0 => "layout_settings"
    1 => "ui"
    2 => "tab_content"
    3 => "appearance"
    4 => "background"
    5 => "background_image"
  ]
  "#array_parents" => array:6 [
    0 => "layout_settings"
    1 => "ui"
    2 => "tab_content"
    3 => "appearance"
    4 => "background"
    5 => "background_image"
  ]
  "#weight" => 0.002
  "#processed" => false
  "#required" => false
  "#attributes" => array:2 [
    "data-drupal-selector" => "edit-layout-settings-ui-tab-content-appearance-background-background-image"
    "aria-describedby" => "edit-layout-settings-ui-tab-content-appearance-background-background-image--txzj0Plh64s--description"
  ]
  "#title_display" => "before"
  "#description_display" => "after"
  "#errors" => null
  "#id" => "edit-layout-settings-ui-tab-content-appearance-background-background-image--txzj0Plh64s"
  "#name" => "layout_settings[ui][tab_content][appearance][background][background_image]"
  "#value" => array:3 [
    "media_library_open_button" => Drupal\Core\StringTranslation\TranslatableMarkup {#2712}
    "media_library_selection" => ""
    "media_library_update_widget" => Drupal\Core\StringTranslation\TranslatableMarkup {#2713}
  ]
  "#ajax_processed" => false

Hey @vlad.dancer - you're completely right about the ternary. I was also able to reproduce this (albeit not in an expected way!)

Here's my form element that reproduces the condition:

    $form['test'] = [
      '#type' => 'media_library',
      '#title' => $this->t('Test'),
      '#allowed_bundles' => ['image'],
      '#default_value' => ['1', '2', '3'],  <-- passing an array in for #default_value
      '#cardinality' => -1,
      '#required' => FALSE,
    ];

When consulting the docs for the element:

 * Usage can include the following components:
 *
 *   $element['image'] = [
 *     '#type' => 'media_library',
 *     '#allowed_bundles' => ['image'],
 *     '#title' => t('Upload your image'),
 *     '#default_value' => NULL|'1'|'2,3,1',  <-- an array is not allowed here
 *     '#description' => t('Upload or select your profile image.'),
 *     '#cardinality' => -1|1,
 *   ];

Were you able to trigger this condition while passing one of the allowed default value patterns into the element? I'm really curious where this array value is coming from. We've been using this module for years and have never seen this before.

I just want to be sure that by adding an is_string check, we aren't masking a logical problem elsewhere.

@Luke.Leber I think this one https://git.drupalcode.org/project/bootstrap_styles/-/blob/2.0.x/src/Plugin/BootstrBackgroundMedia.php#L238

From my experience there are lots of errors with null, empty, other types in vars when dealing with render array (and arrays in general).
Render elements should be classes (like in Symphony) rather than arrays.
So, I always add more checks when work with render arrays. It is better to place one more check than expect that tons of contrib modules following rules in runtime.

The last submitted patch, 16: check-default-value-not-array-15044301-16.patch, failed testing. View results →

Patch from @vlad.dancer #16 is working for me.

as per #20

The last submitted patch, 4: check-default-value-not-array-4.patch, failed testing. View results →

The last submitted patch, check-default-value-not-array.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.