The getFormIconField function in Drupal\material_icons\Plugin\Field\FieldWidget\MaterialIcons assumes the icon field to be in a location within the form array that isn't always the case in the line:
return (!is_null($parents)) ? $form[$parents[3]][$parents[2]][$parents[1]]['icon'] : NULL;

It returning null results in the error
ResponseText: TypeError: Drupal\Core\Render\MainContent\AjaxRenderer::renderResponse(): Argument #1 ($main_content) must be of type array, null given, called in /app/web/core/lib/Drupal/Core/Form/FormAjaxResponseBuilder.php on line 89 in Drupal\Core\Render\MainContent\AjaxRenderer->renderResponse() (line 49 of /app/web/core/lib/Drupal/Core/Render/MainContent/AjaxRenderer.php).
at paths like
/layout_builder/update/block/overrides/node.9316/0/blb_region_col_1/3c9a37a5-f66c-45ba-80f9-a804ea7108af?destination=/node/9316/layout&_wrapper_format=drupal_ajax&ajax_form=1

The $parents array where I saw this issue looks like this:
issue screenshot

It's not clear if this is a problem with the $parents array pointing to the wrong location or other assumptions about the location that are incorrect.

It appears this issue may be caused by nesting the field type this module provides within a paragraph.

Steps to Reproduce

  1. Add a paragraph type with a material icons field
  2. Add a paragraph field to an entity referencing new paragraph type
  3. Add a paragraph and material icon value
  4. Change the "Icon Style" for the material icon field value
CommentFileSizeAuthor
Screenshot 2024-06-27 at 1.47.36 PM.png50.21 KBdamondt

Issue fork material_icons-3457701

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:

About issue forks

Comments

damondt created an issue. See original summary.

damondt’s picture

damondt commented
Issue summary: View changes
damondt’s picture

damondt commented
Title: MaterialIcons::getFormIconField assumes depth of icon field » MaterialIcons::getFormIconField returns null causing ajax error
Issue summary: View changes
darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented
Version: 2.0.2 » 2.0.x-dev

Can confirm this is happening to me too, trying to track down the cause, don't hold your breath.

darvanen opened merge request !8

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented
Status: Active » Needs review

Well I haven't found out *why* the array keys are not in the positions this module expects them to be on a particular request, but I have found a way to prevent the error, which doesn't affect the widget's behaviour as far as I can tell.

It seems with the !is_null( there are times when the $parents variable is expected to be unavailable, so I think it's reasonable to assume that maybe in some situations it will be available but not in exactly the expected format. So I've made an MR to use a null coalesce operator instead of only checking the main variable.

The same may well be applied at line 195 but I wanted to keep the initial MR to the scope defined within the issue.

damondt’s picture

damondt commented
Status: Needs review » Needs work

In the case I'm looking at the issue is that the icon field is nested within a paragraph field. There may be other issues with the logic in this function that the provided patch would work for, but it does not solve for every case.

damondt’s picture

damondt commented
Issue summary: View changes
darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented

You're absolutely right, for one thing we need to retain the null check on the parent variable. I've added a commit to that effect.

This does work for me for a field nested within paragraphs, are you saying it didn't work for you?

damondt’s picture

damondt commented

It doesn't solve the issue I'm seeing, the paragraph is on a block content entity where I'm investigating which could be related

damondt’s picture

damondt commented

Pushed progress on fixing this, problems identified:
- Assumes the nested depth of form field in form, this is not true if in paragraphs.
- Tries to return a field value as an ajax response, must be render array for field or ajax command.

I haven't fully solved the second issue yet, so leaving as "needs work" for now. It no longer errors on style value change, but the icon value is yet not updated successfully.

damondt’s picture

damondt commented
Status: Needs work » Needs review

Fixed ajax issue, marking as needs review.

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented

I tested out a patch from the latest diff of the MR, in my situation there was still an error: 

An AJAX HTTP error occurred.
HTTP Result Code: 200
Debugging information follows.
Path: /node/8321/edit?destination=/admin/content&ajax_form=1&_wrapper_format=drupal_ajax
StatusText: parsererror
ResponseText: 
Warning:  Trying to access array offset on value of type null in /var/www/html/docroot/modules/contrib/material_icons/src/Plugin/Field/FieldWidget/MaterialIcons.php on line 211

A difference might be that https://www.drupal.org/project/paragraphs_browser is in use on this site.

damondt’s picture

damondt commented

Sorry about that, just pushed a fix.

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented

All good man, no apologies please - we're all in this together :)

I've tried out the latest code and at least on that project, it passes manual testing. I don't think that's enough to call it reviewed, but it's a start.

vladimiraus changed the visibility of the branch 2.0.x to hidden.

vladimiraus’s picture

vladimiraus
Primary language English
Location Brisbane 🇦🇺
commented
Status: Needs review » Needs work

exit stops the program. To exit the loop, use break.

prasanth_n made their first commit to this issue’s fork.

vladimiraus changed the visibility of the branch 3457701-getfontfamily-ajax-error to hidden.

vladimiraus changed the visibility of the branch 3457701-getfontfamily-ajax-error to active.

prasanth_n’s picture

prasanth_n commented
Status: Needs work » Needs review

Fixed PHPCS violations and replaced exit with break.

vladimiraus’s picture

vladimiraus
Primary language English
Location Brisbane 🇦🇺
commented
Status: Needs review » Reviewed & tested by the community

Looks good. 👍

b_sharpe made their first commit to this issue’s fork.

b_sharpe’s picture

b_sharpe commented

I'm having a hard time recreating this. I've tried inside a paragraph on a block inside Layout builder and not seeing any errors. Can you provide a step by step?

b_sharpe’s picture

b_sharpe commented
Status: Reviewed & tested by the community » Needs work

Nevermind, found it, but I think this is wayyyy over architecting. Let me try something new here.

b_sharpe opened merge request !12

b_sharpe changed the visibility of the branch 3457701-getfontfamily-ajax-error to hidden.

b_sharpe’s picture

b_sharpe commented
Status: Needs work » Needs review

Much cleaner using NestedArray::getValue().

Please review.

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented

Code looks good - I'm afraid I don't have access to the projects I was using this on any more so I can't try it out.

  • b_sharpe committed 56bac364 on 2.0.x 
    Issue #3457701: MaterialIcons::getFormIconField returns null causing...
b_sharpe’s picture

b_sharpe commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.