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:

  1. Have a reference field for media entities, using the Media Library widget
  2. Have a node with at least two referenced media entities
  3. Remove item #2 (delta = 1)
  4. 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:

  1. Have a reference field for media entities, using the Media Library widget
  2. Have a node with N referenced media entities
  3. Add an item
  4. 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

-

CommentFileSizeAuthor
#8 3167909-8.patch880 bytesckaotik

Comments

ckaotik created an issue. See original summary.

pameeela’s picture

@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:

  1. Install Drupal
  2. Enable Media library
  3. Add a media image reference field with unlimited cardinality to the article content type
  4. Add some media images
  5. Create an article and select some images
  6. Save
  7. Edit the article
  8. Remove the second image
  9. Save
  10. As expected, the second image is removed and the first image remains
pameeela’s picture

Status: Active » Postponed (maintainer needs more info)
Issue tags: +Bug Smash Initiative
ckaotik’s picture

I 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.

phenaproxima’s picture

Status: Postponed (maintainer needs more info) » Needs work
Issue tags: +Needs tests

That'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.

phenaproxima’s picture

Title: Media Library widget unintentionally removes items » Media Library widget unintentionally removes items when used from a media item's edit form
Priority: Normal » Major
Issue tags: +Media Initiative, +Triaged Media Initiative issue
ckaotik’s picture

Drag-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.

ckaotik’s picture

Status: Needs work » Needs review
StatusFileSize
new880 bytes

Okay, so I did lots of debugging and found the problem. Due to the AJAX calls, the submit button's value is moved from op into media_library_ajax_submit. This leads to the FormBuilder no longer finding a match for the "op" button, thus not recognizing it as triggering_element. When I remove $form['actions'][$key]['#name'] = 'media_library_ajax_submit'; from media_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?

The default name is 'op', but we change it on purpose so that it is easier to detect our modified AJAX call above.

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.

ckaotik’s picture

Status: Needs review » Closed (works as designed)
Related issues: +#2985168: [PP-1] Allow media items to be edited in a modal when using the field widget

I'm closing this because it's apparently a side-effect of #2985168.