Array to string conversion for Media Image

This code has been in Entity Embed since 2016. I'm surprised this is only coming up now.

Also, probably I'm missing something, but I don't see how the patch changes anything related to array access? I don't see how this could fix this notice? Could you enlighten me? :)

I've had a similar error, but during rendering. The problem is that data-entity-embed-display-settings might be an array and this can't be rendered in twig, the string should be jsonencoded again before passing on into #attributes again.

see line https://git.drupalcode.org/project/entity_embed/blob/8.x-1.x/src/EntityE...
commenting out that line fixes the notice during rendering

We have the same issue:

Looks like a problem that the "data-entity-embed-display-settings" property inside the FormState::values() does not get properly unset if there are no properties left. MediaImageDecorator::submitConfigurationForm() unsets the "title" or "alt" but not the containing "data-entity-embed-display-settings" afterwards if there are no properies left inisde the array. So it will be used in the code that the patch mentions.

I tried to figure out why this happened but could not find what changed.

*update* after looking in the logs, the problem persists for quite some time now. We only spotted it since we updated our development configuration to output devel messages more verbose on notices/errors

There seems to be a difference of opinion as to what is the cause of this. Can people share their entity embed button configs, so I that the bug is reproducable?

Sure

uuid: ...
langcode: en
status: true
dependencies:
  config:
    - core.entity_view_mode.media.editor_embed
    - entity_browser.browser.media_entity_ckeditor_browser
  module:
    - entity_embed
    - media
label: 'Media Entity'
id: media_entity
type_id: entity
type_settings:
  entity_type: media
  bundles: {  }
  display_plugins:
    - 'view_mode:media.editor_embed'
  entity_browser: media_entity_ckeditor_browser
  entity_browser_settings:
    display_review: true

Might be related https://www.drupal.org/node/2917132 and https://www.drupal.org/project/entity_embed/issues/3021505.

Below is the data structure of `data-entity-embed-display-settings` that causes the 'Notice', `AttributeArray::__toSting()` expects an indexed array, but below is an associative array.

May be just simply json_encode it?

Hi!

I had the same issue, and after some debugging I found, than some components of $values['attributes'] can be arrays. In this case current logic of filtering for empty attributes fails with error and upload provess fails also with ajax error (this error is available in the browser's console only, and not available in the database log.

So, I created a patch, which allows to filter empty attributes, with checking of sub-arrays (during debugging I did not find more than one child sub-arrays), and replaced original array_filter() logic with iteration, because with array_filter() we are unable to return the changed values in subarrays: please see https://www.php.net/manual/en/function.array-filter.php

The iteration works little slower rather than array_filter() but it allows us to filter possible combinations of attributes and not fails with error.

So, please check the patch in attach.

Fixed path in patch structure

I think this can be simplyfied

+          } else {
+            $values['attributes'][$key] = $value;
+          }

is not necessary, since its the default from the iteration anyways

(bool) mb_strlen((string) $item)

the cast to bool should not be necessary since it returns a number so the condition should evaluate properly

(I've got this warning message, too.)

"Location" of the message, in case it helps, is:
/entity-embed/dialog/filtered_html/image_embed?_wrapper_format=drupal_ajax&_wrapper_format=drupal_modal&ajax_form=1

In response to #7, here's my embed button:

uuid: 83507355-9067-4221-aae5-50bbf6eb23c0
langcode: en
status: true
dependencies:
  config:
    - core.entity_view_mode.media.full
    - core.entity_view_mode.media.thumb_smallsquare
    - core.entity_view_mode.media.thumbnail
    - core.entity_view_mode.media.views_image_thumbnail
    - entity_browser.browser.image_embed_eb
    - media.type.gallery_image
    - media.type.image
  module:
    - entity_embed
    - media
label: 'Image embed'
id: image_embed
type_id: entity
type_settings:
  entity_type: media
  bundles:
    - gallery_image
    - image
  display_plugins:
    - 'view_mode:media.full'
    - 'view_mode:media.thumb_smallsquare'
    - 'view_mode:media.thumbnail'
    - 'view_mode:media.views_image_thumbnail'
  entity_browser: image_embed_eb
  entity_browser_settings:
    display_review: true
icon_uuid: null

........."Just in case" it's helpful, here's the text format in which I'm using an embed button:

uuid: 3486daa5-80de-4ac6-a553-ef1e64702529
langcode: en
status: true
dependencies:
  module:
    - editor
    - entity_embed
    - linkit
name: 'Filtered HTML'
format: filtered_html
weight: -10
filters:
  editor_file_reference:
    id: editor_file_reference
    provider: editor
    status: true
    weight: -46
    settings: {  }
  filter_caption:
    id: filter_caption
    provider: filter
    status: true
    weight: -48
    settings: {  }
  filter_html_image_secure:
    id: filter_html_image_secure
    provider: filter
    status: false
    weight: -40
    settings: {  }
  filter_html_escape:
    id: filter_html_escape
    provider: filter
    status: false
    weight: -42
    settings: {  }
  filter_htmlcorrector:
    id: filter_htmlcorrector
    provider: filter
    status: true
    weight: -44
    settings: {  }
  filter_autop:
    id: filter_autop
    provider: filter
    status: false
    weight: -41
    settings: {  }
  filter_align:
    id: filter_align
    provider: filter
    status: true
    weight: -49
    settings: {  }
  filter_url:
    id: filter_url
    provider: filter
    status: false
    weight: -43
    settings:
      filter_url_length: 72
  filter_html:
    id: filter_html
    provider: filter
    status: true
    weight: -50
    settings:
      allowed_html: '<a href hreflang title target rel class id data-entity-* aria-label> <article class id aria-label role> <aside class aria-label role> <blockquote cite class> <br> <caption class> <cite class> <code> <dd> <div class id aria-label role> <dl class id> <drupal-entity data-* data-entity-type data-entity-uuid data-entity-embed-display data-entity-embed-display-settings data-align data-caption data-embed-button alt title> <dt> <em> <figure class aria-label role> <footer class aria-label role> <h2 id class> <h3 id class> <h4 id class> <h5 id class> <h6 id class> <hr class> <img src alt height width data-entity-type data-entity-uuid data-align data-caption class> <li class id> <ol start type class id> <p class id> <pre> <section class id aria-label role> <span class> <strong> <sub> <sup> <table class id> <tbody class> <td class colspan rowspan> <tfoot class> <th class colspan rowspan scope> <thead class> <tr class> <ul type class id>'
      filter_html_help: true
      filter_html_nofollow: false
  linkit:
    id: linkit
    provider: linkit
    status: true
    weight: -45
    settings:
      title: true
  entity_embed:
    id: entity_embed
    provider: entity_embed
    status: true
    weight: -47
    settings: {  }

I wasn't able to recreate the warning displaying in the logs or to the screen or causing ajax errors, but I was able to see that the code here fails with an empty array;

if (!empty($values['attributes']['data-entity-embed-display-settings'])) {
  // Serialize entity embed settings to JSON string.
 $values['attributes']['data-entity-embed-display-settings'] = Json::encode($values['attributes']['data-entity-embed-display-settings']);
}

This would lead to the array to string conversion here:

return (bool) mb_strlen((string) $value);

I came up with my own solution to fix this:

diff --git a/src/Form/EntityEmbedDialog.php b/src/Form/EntityEmbedDialog.php
index 160aa7a..4cb1251 100644
--- a/src/Form/EntityEmbedDialog.php
+++ b/src/Form/EntityEmbedDialog.php
@@ -818,8 +818,11 @@ class EntityEmbedDialog extends FormBase {
       $response->addCommand(new HtmlCommand('#entity-embed-dialog-form', $form));
     }
     else {
-      // Serialize entity embed settings to JSON string.
-      if (!empty($values['attributes']['data-entity-embed-display-settings'])) {
+      if (isset($values['attributes']['data-entity-embed-display-settings']) && $values['attributes']['data-entity-embed-display-settings'] === []) {
+        unset($values['attributes']['data-entity-embed-display-settings']);
+      }
+      elseif (!empty($values['attributes']['data-entity-embed-display-settings'])) {
+        // Serialize entity embed settings to JSON string.
         $values['attributes']['data-entity-embed-display-settings'] = Json::encode($values['attributes']['data-entity-embed-display-settings']);
       }

I tried out the other solutions, but I had issues with each:

Regarding #2, this results in $values["attributes"]["data-entity-embed-display-settings"] = '[]';

{"data-entity-type":"media","data-entity-uuid":"69fdd577-0732-4a07-ab62-d8a8f7c88283","data-langcode":"en","data-entity-embed-display":"view_mode:media.embed","data-entity-embed-display-settings":"[]","data-embed-button":"media_entity_embed","data-align":"center","data-caption":"","alt":""}

This doesn't work because it won't get filtered out by return (bool) mb_strlen((string) $value);

Regarding #9, I put a breakpoint there, and just before this code below in EntityEmbedDialog, and it hit the code in EntityEmbedDialog first with it still an array, so I don't think this fixes the problem in the dialog.

      // Filter out empty attributes.
      $values['attributes'] = array_filter($values['attributes'], function ($value) {
        return (bool) mb_strlen((string) $value);
      });

Regarding #14, the only value that could be an array is data-entity-embed-display-settings, so this solution seems overly complicated.

Patch in #17 worked for me.

Uploading a simplified patch for review.

My suggested approach is that we catch $values['attributes']['data-entity-embed-display-settings'] in the array_filter, making the code more generic so any future empty arrays will be filtered out.

Tested #19 and working for me. Thanks for the patch.

The patch in #19 works great, thanks!

Patch #10 or a similar solution is needed.
#10 works (no more "array to string" warnings) but I'm not sure if it's useful for anything

data-entity-embed-display-settings="{"image_style":"","image_link":"","svg_render_as_image":1,"svg_attributes":{"width":"","height":""}}"

This patch has no tests...is there any way we could add some?

EDIT: Since the bug is related to how the dialog is handling different values, I think a kernel or unit test might be sufficient here.

#19 works and as Phenaproxima said a kernel test should be enough.

#19 worked for me, thank you!

One more confirmation of #19 eliminates the issue, thanks for sharing @jorgegc!

• Drupal core: 9.1.3
• Entity Embed: 1.1.0
• PHP: 7.4.12 (memory limit: 512M)

Another confirmation of #19 eliminates the issue, thanks for sharing @jorgegc and @promes for implementing.

Drupal core: 9.1.8
Entity Embed: 1.1.0
PHP: 7.2.34 (memory limit: 128M)

Interdiff patch 19 - 30:

       // Filter out empty attributes.
       $values['attributes'] = array_filter($values['attributes'], function ($value) {
-        return (bool) is_array($value) ? !empty($value) : mb_strlen((string) $value);
+        return is_array($value) ? !empty($value) : mb_strlen((string) $value);
       });

       // Allow other modules to alter the values before getting submitted to the

The patch contains also the fix from #3249967: Skip test ConfigurationUiTest::testValidationWhenAdding for Drupal core 9.2 >=.

Confirmation #19 works. Thanks @jorgegc!

Drupal 9.2.7
Entity Embed: 8.x-1.2
PHP: 7.4.12

The patch in #30 does not apply with Entity Embed V1.3 because the extra fix in it for #3249967: Skip test ConfigurationUiTest::testValidationWhenAdding for Drupal core 9.2 >= is no longer applicable.

Attached is an updated version for 1.3 / dev. The same except there are no changes to the ConfigurationUiTest.php code and the line numbers are updated.

Manual testing seems to indicate that this issue is gone with entity_embed 1.3.
I'm guessing that #3106808: Array to string conversion notice when embedding a Media file fixed some underlying issue. Which would make this one "Closed (outdated)". Or "Closed( duplicate)".
Can others confirm?

Even if it's gone...look at that magnificent test coverage in this patch! We should add that anyway.

Even if it's gone...look at that magnificent test coverage in this patch! We should add that anyway.

Wait. There are tests? Excellent.
Found them!
Attached is a patch which should contain just the tests from #32. Very curious about the current state of affairs without the actual fix from #32.
Leaving issue title as is for now.

I think this makes sense. Ran into this today with 1.3 and the patch in #32 fixed it. RTBC.

EDIT: Redacted.

1. +++ b/src/Form/EntityEmbedDialog.php
   @@ -824,7 +824,7 @@ class EntityEmbedDialog extends FormBase {
   +        return is_array($value) ? !empty($value) : mb_strlen((string) $value);
   

   Should we maybe cast the result of mb_strlen() to a bool, just for clarity's sake?

2. +++ b/tests/src/Kernel/EntityEmbedDialogTest.php
   @@ -0,0 +1,93 @@
   +    $this->container->get('module_installer')->install(
   +      ['entity_embed_test']
   +    );
   

   Strange formatting here. Can we just have ->install(['entity_embed_test'])?

3. +++ b/tests/src/Kernel/EntityEmbedDialogTest.php
   @@ -0,0 +1,93 @@
   +    $EntityEmbedDialog = EntityEmbedDialog::create($this->container);
   +
   +    $form_state->setValue('attributes', $attributes);
   +    $response = $EntityEmbedDialog->submitEmbedStep($form, $form_state);
   

   Inconsistent variable naming. $EntityEmbedDialog should probably just be $entity_embed_dialog.

4. +++ b/tests/src/Kernel/EntityEmbedDialogTest.php
   @@ -0,0 +1,93 @@
   +    foreach ($commands as $command) {
   +      if ($command['command'] === 'editorDialogSave') {
   +        $this->assertSame($expected, $command['values']['attributes']);
   +      }
   +    }
   

   This construction could give us a false positive (i.e., it passes when it should fail) if there is no editorDialogSave command in $commands. We should probably add some kind of check for that.

5. +++ b/tests/src/Kernel/EntityEmbedDialogTest.php
   @@ -0,0 +1,93 @@
   +   * @see \Drupal\Tests\entity_embed\Kernel\EntityEmbedDialogTest::testRemovalOfEmptyAttributes
   

   We don't need this @see.

6. +++ b/tests/src/Kernel/EntityEmbedDialogTest.php
   @@ -0,0 +1,93 @@
   +    # 测试 / Тестирование -> Test
   

   Do we need this line?

Here's a re-roll of #32 for Drupal 9.5.2. and Entity Embed 8.x-1.3.

Rolled #32 into an MR and addressed most of the feedback in #39

Only outstanding issue is 39.4 which I'm not too sure how to handle.

Works for me. Back to RTBC?

This should go into the 2.0.x branch first, and then potentially backported.

Is something holding-up getting this committed?

Once someone patches this for themselves (Me, Myself and I, Us, We, Our group, Our build), they, themselves, them, those people, others quickly forget about it ever having been a problem and often do not follow up.

I guess it had just fallen off the radar. Merged into 2.0.x. I can also commit this to 8.x-1.x if someone can open a separate MR (in this issue) to backport it.

Previous minor is passing testing, however 6 months has passed and a few new minors , now failing.

The two branches are identical. You can just do a fast-forward merge.

I doubt the test failures are because of this issue. A new issue should be opened to fix them.

(These tags can probably be deleted: previous/3287235-automated-drupal-10/2022-10-20, previous/3287235-automated-drupal-10/2022-10-17.)

The fix for the failures are in an RTBC issue

There's a bunch of other RTBC issues at the top of the issue queue.

The fix for the failures are in an RTBC issue

Huh? How can that be true? All PHPUnit test jobs in that issue are failing...

Ah yes they are failing, my mistake. Likely more than one change introduced in the latest minor 10.5 and 11.2 at play here.

I was able to cleanly cherry-pick this commit to 8.x-1.x, so marking as closed.