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
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
Comment #3
bessonweb commentedHi,
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!
Comment #4
sgoodwin commentedI had this same error message, and was able to clear it by :
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.
Comment #5
interactivex commentedI can confirm that the solution in comment #4 is working
Comment #6
bessonweb commentedMessage deleted, it was an error of ticket.
Comment #7
leksat commentedAllowed 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.
Comment #8
leksat commentedComment #9
thomjjames commentedHi,
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:
Hope it helps others
Tom
Comment #10
eiriksmCould 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.
Comment #11
eiriksmSorry I was mistaken. It was added as part of #3340209: Limit available image styles for image related blocks and never tagged: https://git.drupalcode.org/project/gutenberg/-/commit/a57338fa5f79f0fb9a...
Comment #12
eiriksmReleased 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
Comment #13
leksat commented@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.
Comment #14
eiriksmAh, 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
Comment #15
leksat commented@eiriksm Great :)
So which behavior is the desired one? Allow all images styles? Or none?
Comment #16
eiriksmMy 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?
Comment #18
eiriksmAlright. Step 1. A test that
Comment #19
eiriksmGreat. 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
Comment #20
codebymikey commentedAlso 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.
Comment #21
eiriksmGreat! 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?
Comment #22
codebymikey commentedI tested the merge request and I believe it works differently to what was proposed.
My proposition was to:
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.
Comment #23
eiriksmAh 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?
Comment #24
codebymikey commentedHaha, 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.