Problem/Motivation
Media Library field widget unintentionally removes items when adding or removing items. Likely, the remove_button (of '#type' => 'submit') submit callback (::removeItem) is always executed with the regular submit.
Steps to reproduce
Additional references are removed when removing a single item:
- Have a reference field for media entities, using the Media Library widget
- Have a node with at least two referenced media entities
- Remove item #2 (delta = 1)
- Submit form
Expected result:
The node has only one referenced media item (delta = 0). We're directed to the media collection/canonical page.
Actual result:
The node has zero referenced media items and we're still on the edit form, the node was not saved. Items #2 (the one we wanted to remove) and #1 (which we wanted to keep) were removed.
Something similar happens when adding an item:
- Have a reference field for media entities, using the Media Library widget
- Have a node with N referenced media entities
- Add an item
- Submit form
Expected result:
The node has only one referenced media item (delta = 0). We're directed to the media collection/canonical page.
Actual result:
The node still has N referenced media items, the Nth item is missing and replaced by our addition. We're also still on the edit form, the node was not saved.
Proposed resolution
Prevent the additional call to ::removeItem.
Remaining tasks
Write tests & patch.
User interface changes
-
API changes
-
Data model changes
-
Release notes snippet
-
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 3167909-8.patch | 880 bytes | ckaotik |
Comments
Comment #2
pameeela commented@ckaotik I am having trouble reproducing this. Is there any more info you can provide?
It is working as expected for me with these steps, can you let me know if I have missed something:
Comment #3
pameeela commentedComment #4
ckaotikI just checked, and it indeed works fine on node forms. It seems the error occurs only on media edit forms, sorry I missed that detail. We have a gallery media type with a field referencing image media entities, and that's where I've observed this behavior.
Comment #5
phenaproximaThat's helpful, @ckaotik, thank you! I don't believe we have test coverage for that, so let's add some. With a failing test in hand, it should be no problem getting this fix landed.
Comment #6
phenaproximaComment #7
ckaotikDrag-sorting the media's child media entities also seems bogged. As a temporary workaround for this facet of the problem, I temporarily disabled Javascript in my browser (something going wrong on the AJAX side of things?). This is of course not helping when trying to get the entities referenced in the first place.
Comment #8
ckaotikOkay, so I did lots of debugging and found the problem. Due to the AJAX calls, the submit button's value is moved from
opintomedia_library_ajax_submit. This leads to the FormBuilder no longer finding a match for the "op" button, thus not recognizing it astriggering_element. When I remove$form['actions'][$key]['#name'] = 'media_library_ajax_submit';frommedia_library_form_media_form_alter, the correct button is detected.Said line is however commented as an intentional change, so I fear removing it might have unexpected side-effects?
I'm attaching the change as a patch I used on 9.2.2
, so maybe test bot can tell us if things break.Edit: This code was apperently introduced by a patch from #2985168: [PP-1] Allow media items to be edited in a modal when using the field widget, I'll have to check if this really also applies on vanilla core. My bad.
Comment #9
ckaotikI'm closing this because it's apparently a side-effect of #2985168.