With the latest version of 2.x and Drupal 10.3.2, I noticed a warning and fixed a couple of best practices.

It appears that $allowed_styles might be null or not set, which causes the error when trying to access an offset. To fix this, we need to ensure that $allowed_styles is an array and contains the key before accessing it.

It probably applies to 3.x as well

Issue fork gutenberg-3471570

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

doxigo created an issue. See original summary.

bessonweb’s picture

Hi,

I tried to apply the patch but I get "Could not apply patch! Skipping. The error was: Cannot apply patch https://git.drupalcode.org/project/gutenberg/-/merge_requests/193.diff"

Gutenberg : 8.x-2.11
Drupal : 10.3.5

Thanks!

sgoodwin’s picture

I had this same error message, and was able to clear it by :

  1. Opening the edit page for the content type using Gutenberg
  2. Clicking save

No changes were made to the configuration settings, but after a config export, I see that the allowed image styles is now saved to the gutenberg.settings.yml.

interactivex’s picture

I can confirm that the solution in comment #4 is working

bessonweb’s picture

Message deleted, it was an error of ticket.

leksat’s picture

Allowed image styles were added in #3340209: Limit available image styles for image related blocks. No upgrade path was provided.

If I follow #4 solution, Gutenberg adds all styles to {content_type}_allowed_image_styles. Which I guess is the desired behavior.

If I use the current MR, the warning will be suppressed, but there will be no allowed image styles at all. Which I guess is wrong.

My assumption is that we need an upgrade path for #3340209: Limit available image styles for image related blocks. A post-update hook that will add all available image styles to all content types. I hope maintainers can clarify this.

leksat’s picture

thomjjames’s picture

Hi,

Needed to solve this for a wider deployment so resaving wasn't really an option, this hook_update seems to work & remove the warning. It doesn't respect any previous settings but i'm not so up to speed on the _allowed_image_styles config ie. is it a new setting or an old one:

use Drupal\image\Entity\ImageStyle; // at top of file

function MODULE_update_VERSION() {
  // Step 1: Load all image styles into an array.
  $image_styles = ImageStyle::loadMultiple();
  $image_styles_array = [];
  foreach ($image_styles as $style_name => $style) {
    $image_styles_array[$style_name] = $style->label();
  }
  $image_styles_array['full'] = 'Original';

  // Step 2: Load the Gutenberg settings configuration.
  $config = \Drupal::service('config.factory')->getEditable('gutenberg.settings');

  // Step 3: Load all content types.
  $content_types = \Drupal::entityTypeManager()->getStorage('node_type')->loadMultiple();

  // Step 4: Iterate through each content type and check configuration.
  foreach ($content_types as $content_type_id => $content_type) {
    $config_key = $content_type_id . '_allowed_image_styles';
    $allowed_image_styles = $config->get($config_key);
    
    $config->set($config_key, $image_styles_array);
  }

  // Finally, save the configuration.
  $config->save();
}

Hope it helps others
Tom

eiriksm’s picture

Status: Active » Closed (cannot reproduce)

Could it be that these notices are on the latest stable 2.x?

The last commit on 2.x fixes this issue. It was added as part of #3467986: Undefined variable $sizes in gutenberg_form_node_form_alter() which also made it into a new stable 3.x version. However, we did not tag a new version on 2.x, so I will do that.

eiriksm’s picture

eiriksm’s picture

Released 2.12: https://www.drupal.org/project/gutenberg/releases/8.x-2.12

Making a follow up to add test coverage for the bug, so we can avoid similar inconveniences in the future

leksat’s picture

Status: Closed (cannot reproduce) » Active

@eiriksm The warning is not the main issue here.

An existing website gets a Gutenberg update => allowed styles are empty
Admin resaves a content type (without doing any changes) => allowed styles are filled with all styles

Which behavior is correct?

If allowed styles are meant to be empty, the the content type form save should behave differently.

If allowed styles should be prefilled, then we need a post-update hook.

eiriksm’s picture

Ah, I see what you mean now, sorry I did not catch that 🤓️

Awesome, well let's solve that in this issue, and in the meantime I have opened #3476648: Create a test to make sure we don't have notices on a gutenberg enabled node add page where we can add tests to all branches to avoid the warning as well

leksat’s picture

@eiriksm Great :)

So which behavior is the desired one? Allow all images styles? Or none?

eiriksm’s picture

My personal opinion: all. That was the behaviour before the feature in #3340209: Limit available image styles for image related blocks was added.

What do you think? What do other people think?

eiriksm’s picture

Alright. Step 1. A test that

  • Tests default behavior (what happens when you hit save on a content type). We expect all image styles to be enabled.
  • Tests what happens if you don't have that setting and run an update hook (not implemented yet, so this test is expected to fail). We expect all image styles to be enabled.
  • Tests that we can actually use the functionality with disabling image styles, by disabling the "full" option and making sure it does not appear any more
eiriksm’s picture

Status: Active » Needs review

Great. So here is a failing test (proving the test is testing what it should): https://git.drupalcode.org/project/gutenberg/-/jobs/2852605

And now I pushed what should be the fix, hopefully making the CI green. Thanks to @thomjjames for the starting point of that

codebymikey’s picture

Also ran into this issue whilst updating to the latest 2.14, and just thought I'd put in my two cents regarding the current situation regarding the #3340209: Limit available image styles for image related blocks feature.

As stated in #7, I believe new functionality like this should either provide an upgrade path or maintain backwards compatibility for site builders to use.

However the current suggested upgrade path of setting the available image styles to a fixed set of values is a regression in behaviour imo and limits flexibility since if the site has multiple node bundles, and introduces new image styles which they should all have access to, the site builder would need to manually resave all those bundles to provide support for it.

I believe the best approach to address this is to maintain backwards compatibility by keeping the default behaviour of allowing all image styles unless the site builder specifically wants to restrict it to specific ones.

Ideally the work done in #3229848: Allow API to restrict which image styles are available to users would allow site builders/developers to restrict the image styles in a more dynamic fashion (e.g. custom permission checks via alter hooks), but that's currently only working for image uploads and will need to be adapted to support the drupalSettings['gutenberg']['image-sizes'] value too.

As part of my config normalization work in #3415218: Normalize the gutenberg.settings config and support image sizes filtering, I've addressed the default value issue, provided a config schema and maintained backwards compatibility without requiring a database upgrade. So feel free to test the patch on that issue, or provide feedback if the proposed solution isn't viable.

I like the tests, so we should definitely keep it in once a decision has been made regarding backwards compatibility.

eiriksm’s picture

Great! That's also what's implemented in the (not yet merged) merge request. ✌️🎉

Could you test and/or review that, since it sounds like we agree on the way for an upgrade path?

codebymikey’s picture

Status: Needs review » Needs work

I tested the merge request and I believe it works differently to what was proposed.

My proposition was to:

  1. Remove the update hook, and instead leave the config empty until the site builder specifically chooses to filter to specific image styles (keeping backwards compatibility)
  2. Stop auto-filling the entire list of image styles by default (as part of this, we should also implement and fix the "all" functionality)

I'd ideally make most of those changes in this issue, but due to the change in data schema and potential merge conflicts, most of those have been implemented within the #3415218: Normalize the gutenberg.settings config and support image sizes filtering merge request, and is currently awaiting review/testing.

Are you able to test that issue when you get the chance? And hopefully once others test and agree with the changes, merging that in should help improve the general DX.

eiriksm’s picture

Ah I see what you mean. That makes sense to me

I took a quick look, but then realized the diff was so long :o

I personally would like to merge this one first, since this way we would have tests in place for the image settings which are also touched there. Let me update the MR and you can give that a spin?

codebymikey’s picture

I took a quick look, but then realized the diff was so long :o

Haha, yeah, there's quite a lot going on in that MR, but it seemed like the easiest way to address and efficiently test all the improvements from like 3 separate issues 😅.

I don't mind having this merged in first - would we need to cherry-pick for 3.x as well? I haven't had a chance to use that branch yet.