Problem/Motivation

entity_browser_form_alter() identifies forms it wants to alter using a route ID, however this is not reliable if more than one form is built on that callback - which can happen if you have media-library in an entity edit form, in a modal (yes all the modals)

Proposed resolution

Be super sure before altering

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 3130500.patch974 byteslarowlan

Comments

larowlan created an issue. See original summary.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Active » Needs review
StatusFileSize
new 3130500.patch974 bytes
sim_1’s picture

sim_1
she/her
commented
Status: Needs review » Reviewed & tested by the community

I have spent so much time trying to figure out how to solve this issue. Thank you!!!! Marking RTBC because it allows the media library to be used in an editor. I'm sure there are other use cases too.

JvE’s picture

JvE commented
Related issues: +#3220322: entity_browser_form_alter error on non-entity forms
JvE’s picture

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

I don't think all entity forms on that route should be altered either. Just the initial one.
If anything else builds an entity form during the same request we don't want that to be altered do we?

ammaletu’s picture

ammaletu commented

I have the same problem: I am editing a custom content in a modal, which contains a media library element. There is an "add media" button, rendered by the media library widget. Clicking on it should open a second modal with the media library, but instead leads to:

TypeError: Drupal\Core\Render\Element::children(): Argument #1 ($elements) must be of type array, null given, called in /var/www/html/web/modules/contrib/entity_browser/entity_browser.module on line 75 in Drupal\Core\Render\Element::children() (line 72 of /var/www/html/web/core/lib/Drupal/Core/Render/Element.php)

The patch from #2 might not be the perfect solution, but it solves this problem for all cases with completely different forms (tested on Drupal 9.3.14). Why not merge this and create a new ticket that describes the now smaller problem of multiple entity browser forms? What is needed to get this ready?

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Priority: Normal » Major
Status: Needs work » Reviewed & tested by the community

I'm here with the exact same problem described in #6. The patch in #2 fixes my problem. Thanks for the patch, @larowlan.

I am very much of the opinion that this should get committed.

It's definitely an improvement even if it's not perfect. It's possible that comment #5 is correct, but there aren't any clear comments or steps to reproduce on this or the duplicate issue that demonstrate any failings of the patch here. Further, committing this patch would not introduce any new problems: the comment in #5 claims (probably correctly) this patch is not restrictive enough, not that it is too restrictive. I'm taking this back to RTBC.

I'm going to up the severity to Major since it's a fatal error that is easily triggered through the UI under normal configuration.

  • Berdir committed 4a7b06c on 8.x-2.x authored by larowlan 
    Issue #3130500 by larowlan: Entity browser assumes all forms on the...
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Reviewed & tested by the community » Fixed

Committed.

Status: Fixed » Closed (fixed)

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