Problem/Motivation
This is very niche, but a showstopper for us. If there happens to be two different entity forms, (not subforms) with the same field name (the field that is using the media library widget), then the field_widget_id that is used to attach the selected media entities will be identical.
In MediaLibraryFieldWidgetOpener.php
$response
->addCommand(new InvokeCommand("[data-media-library-widget-value=\"$widget_id\"]", 'val', [$ids]))
->addCommand(new InvokeCommand("[data-media-library-widget-update=\"$widget_id\"]", 'trigger', ['mousedown']));
This will simply be, for example, "field_image".
I understand that this is not technically a core issue as the only way to currently have these embedded forms is with some sort of contrib module. I do believe it's worth talking about, for a few reasons:
- There is no way to fix this in a contrib module, they cannot alter the id that's used here
- There are a number of feature requests that will eventually have this functionality in core, in a number of different ways. I imagine it will come up at some point. #1728816: Ensure multiple entity forms can be embedded into one form
- Even if this should be addressed in contrib, it would be nice to have a more centralized reference.
Steps to reproduce
The simplest way to re-create is with the contact_block module, (very high usage). Install Drupal demo_umami (the easiest way) with contact_block enabled.
Add a media reference field to the "Website feedback" contact form. "field_image", using the media library widget.
Add an instance of the website feedback contact block to the content area.
Add another instance of the website feedback contact block to the content area. (two separate blocks)
The forms themselves have proper unique IDs contact-message-feedback-form--2, contact-message-feedback-form--1, etc.
Adding media to the second block, however, will always add an image to the first block.
Proposed resolution
One potential solution is the way the id is formed. In MediaLibraryWidget::formElement
the $id_suffix could include more criteria. I attached a patch with the entity uuid, but this isn't meant to be a real solution, this is just to fix it for now and use in composer.
Another potential, (and maybe better) solution is the InvokeCommand used in MediaLibraryFieldWidgetOpener. Perhaps this selector should have a wrapper element ID.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#6 | 3345064-nr-bot.txt | 1.15 KB | needs-review-queue-bot |
#3 | 3345064.patch | 1.29 KB | asherry |
media-library-upload-demo.gif | 2.06 MB | asherry |
Issue fork drupal-3345064
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 #2
asherry CreditAttribution: asherry at Aten Design Group commentedComment #3
asherry CreditAttribution: asherry at Aten Design Group commentedComment #4
eglawSaved my day !!!
Ya da man!
Comment #5
droath CreditAttribution: droath at Aten Design Group commentedThis fixed an issue I was having in layout paragraphs where the same media entity was getting added into multiple fields that had the same machine name.
Comment #6
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #7
hudriA media library
field_image
on both, a node and a paragraph, seems a quite natural and common use-case and not that niche to me. Bumping to major.Comment #10
super_romeo CreditAttribution: super_romeo as a volunteer commentedComment #12
jnettikI can also confirm the patches here fix the issue. It also doesn't feel trivial to me as well.
Comment #17
ant1Comment #20
smustgrave CreditAttribution: smustgrave at Mobomo commentedThanks, as a bug will need a test case or simple assertion added.
Comment #21
J-LeeHowever, it is theoretically possible for $this->fieldDefinition->getUniqueIdentifier() to return an empty string.
Comment #22
J-LeeThe tests use \Drupal\Core\Field\BaseFieldDefinition, which returns a non-unique identifier for the same field name.
Did someone have a suggestion for the best way to test this?