- Issue created by @doxigo
- Merge request !193Issue #3471570: Warning: Trying to access array offset on value of type null... β (Open) created by doxigo
- π«π·France bessonweb
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"
Thanks!
I had this same error message, and was able to clear it by :
- Opening the edit page for the content type using Gutenberg
- 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.
- π³π±Netherlands interactivex
I can confirm that the solution in comment #4 π Warning: Trying to access array offset on value of type null in gutenberg_form_node_form_alter() (line 578 of gutenberg.module). Active is working
- π«π·France bessonweb
Sorry but for me this solution has no results.
The error messages still here and my media field can't open the media library.
Allowed image styles were added in β¨ Limit available image styles for image related blocks Fixed . 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 β¨ Limit available image styles for image related blocks Fixed . A post-update hook that will add all available image styles to all content types. I hope maintainers can clarify this.
- πΈπͺSweden thomjjames Sweden
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 - π³π΄Norway eiriksm Norway
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 π Undefined variable $sizes in gutenberg_form_node_form_alter() Fixed 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.
- π³π΄Norway eiriksm Norway
Sorry I was mistaken. It was added as part of β¨ Limit available image styles for image related blocks Fixed and never tagged: https://git.drupalcode.org/project/gutenberg/-/commit/a57338fa5f79f0fb9a...
- π³π΄Norway eiriksm Norway
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
@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 stylesWhich 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.
- π³π΄Norway eiriksm Norway
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 π Create a test to make sure we don't have notices on a gutenberg enabled node add page Active where we can add tests to all branches to avoid the warning as well
@eiriksm Great :)
So which behavior is the desired one? Allow all images styles? Or none?
- π³π΄Norway eiriksm Norway
My personal opinion: all. That was the behaviour before the feature in β¨ Limit available image styles for image related blocks Fixed was added.
What do you think? What do other people think?
- π³π΄Norway eiriksm Norway
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
- π³π΄Norway eiriksm Norway
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
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 β¨ Limit available image styles for image related blocks Fixed 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 β¨ Allow API to restrict which image styles are available to users Active 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 π Clean up gutenberg.settings config Active , 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.
- π³π΄Norway eiriksm Norway
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?
I tested the merge request and I believe it works differently to what was proposed.
My proposition was to:
- 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)
- 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 π Clean up gutenberg.settings config Active 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.
- π³π΄Norway eiriksm Norway
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?
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.