Don't unset the #submit callback [#3246687]

Problem/Motivation

When using other modules, such as media entity file replace, which add their own custom submit handler, this module prevents those from working. It does this by completely clearing out the value of $form['actions']['submit']['#submit'] and then applying an ajax submit handler instead.

Steps to reproduce

Install the media entity file replace module and add it to the form for at least one media type
Enable this module for any given media entity reference field
Edit an existing media entity using this module that is also using media entity file replace, and try uploading a replacement file
Save the media entity, then go to Content -> Files. You will see the file uploaded to replace the existing one, but still marked as temporary and used in 0 places
Go back to editing the media entity for which you tried to replace the file, and check the file that is attached - open that file to view it, refresh a few times to make sure you are not viewing a cached image. It will be the same image as before, and not replaced as expected

Proposed resolution

See this core issue. It is aimed to resolve the same problem, and it may end up coming to Drupal core once they have worked through the various concerns identified with it.

The patch provided on that issue works, so if this module is not working for you then use that patch instead.

This module could be updated based on that latest patch in order to allow other submit handlers, such as the one in media entity file replace, to work properly.

Comment	File	Size	Author
#18	3246687-media-edit-modal-20250306.patch	1.64 KB	hargurpreet

Issue fork media_library_edit-3246687

Show commands

Start within a Git clone of the project using the version control instructions.

Add & fetch this issue fork’s repository

Or, if you do not have SSH keys set up on git.drupalcode.org:

Add & fetch this issue fork’s repository

3246687-prevents-other-submit-3.0.x changes, plain diff MR !26
Check out this branch for the first time

Check out existing branch, if you already have it locally
3246687-prevents-other-submit changes, plain diff MR !11
Check out this branch for the first time

Check out existing branch, if you already have it locally

1 hidden branch

3.0.x

compare

Check out this branch for the first time

Check out existing branch, if you already have it locally

About issue forks

Comments

Comment #1

29 October 2021 at 17:57

teknocat created an issue. See original summary.

Comment #2

ahebrank commented 29 October 2021 at 17:58

Is this a duplicate of https://www.drupal.org/project/media_library_edit/issues/3231869 ?

Comment #3

teknocat commented 1 November 2021 at 15:22

It seems to be a bit different. The version I am filing this issue against already has the fix from that other issue - it's no longer completely unsetting the #submit array. However, when this module runs its hook after any other modules that may have set other submit handlers, it strips out those handlers that still need to run because it sets the submit handlers to an empty array, without accounting for any that may be set already that need to run. I found this happens when using it in conjunction with Media Entity File Replace, for example. It strips out the submit handler set by that module.

The core patch I linked in the issue description above provides an edit button without causing any issues with submit handlers.

Comment #4

ahebrank commented 17 November 2021 at 13:27

Version:	8.x-2.0	» 8.x-2.x-dev
Status:	Active	» Postponed

Unfortunately I think this is a wont-fix given that we're getting into a pretty complex interaction between contrib modules that are both working against core to some degree. The 1.x version of this module is much simpler and doesn't have many of these alter-related problems, or there's the core patch. Both of these are discussed on https://www.drupal.org/project/media_library_edit

I'm going to leave open for visibility but mark postponed.

Comment #5

3 January 2023 at 17:33

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

Comment #6

prudloff commented 3 January 2023 at 17:43

We encountered a similar problem using media_library_edit with media_name.
media_name adds a custom #submit callback (after the media_library_edit_form_alter() the array, so it is kept).
But since the ::submitForm callback has been removed, the entity is not filled correctly with the new values when reaching the _media_name_form_submit callback (and it comes before _media_library_edit_media_edit_save() calls ::submitForm).

What is the reason to remove the #submit callbacks?
I tried to keep them (see MR) and everything seems to work correctly. (The #submit callbacks are executed correctly in order and then the #ajax callback closes the modal.)

Comment #7

3 January 2023 at 17:46

prudloff opened merge request !11

Comment #8

ahebrank commented 3 January 2023 at 17:53

Title:	Prevents other submit handlers from working, such as media entity file replace	» Don't unset the #submit callback
Status:	Postponed	» Needs review

Comment #9

prudloff commented 22 March 2023 at 10:35

Comment #10

teknocat commented 24 March 2023 at 02:43

Yes I agree there is no reason for this module to wipe out all the submit handlers just to be able to do it's thing. @prudloff's solution looks great. I don't think I would have thought of that.

Comment #11

ahebrank commented 24 March 2023 at 11:42

@prudloff can you change the name of _media_library_edit_media_edit_save() since it no longer saves?

Looking for an RTBC and then can merge.

Comment #12

ahebrank commented 31 January 2024 at 18:46

Version:	8.x-2.x-dev	» 3.0.x-dev
Status:	Needs review	» Needs work

Comment #13

dhruv.mittal commented 16 January 2025 at 12:31

Assigned:

Unassigned

» dhruv.mittal

Comment #14

dhruv.mittal commented 19 January 2025 at 16:15

Assigned:	dhruv.mittal	» Unassigned
Status:	Needs work	» Needs review

Comment #15

divyansh.gupta commented 27 February 2025 at 04:29

Status:

Needs review

» Needs work

Since there is a unresolved thread moving it to Needs Work, Please resolve the thread.

Comment #16

dhruv.mittal commented 27 February 2025 at 04:45

Status:

Needs work

» Needs review

Please review

Comment #17

divyansh.gupta commented 27 February 2025 at 06:57

Status:

Needs review

» Reviewed & tested by the community

I have reviewed and tested the latest patch in !11.

✅ The #submit callback is no longer being unset, allowing other submit handlers (such as from Media Entity File Replace) to function correctly.
✅ The function name update issue has been resolved, and $form['actions']['submit']['#ajax']['callback'] now correctly references the updated function.
✅ The form submission process works as expected, and the modal closes properly after submission.

Everything looks good, and I don't see any regressions or issues. Moving this to RTBC.

Comment #18

hargurpreet commented 6 March 2025 at 15:57

Issue summary:

View changes

Status	File	Size
new	3246687-media-edit-modal-20250306.patch	1.64 KB

I also encountered an issue while replacing a file in the modal, but this patch resolved it.

Comment #19

6 March 2025 at 16:00

ahebrank committed 98479951 on 8.x-2.x authored by prudloff

Issue #3246687 by dhruv.mittal, prudloff: Don't unset the #submit...

Comment #20

ahebrank commented 6 March 2025 at 16:00

Status:

Reviewed & tested by the community

» Fixed

Comment #21

ahebrank commented 6 March 2025 at 16:03

Status:

Fixed

» Needs work

That MR was targeted at 8.x-2.x so that's where it landed. So we'll need another one for 3.x, if it's intended to apply there.

Comment #22

hargurpreet commented 6 March 2025 at 16:25

Issue summary:

View changes

Restored the issue detail

Comment #23

15 April 2025 at 08:43

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

Comment #24

15 April 2025 at 08:47

marcoliver changed the visibility of the branch 3.0.x to hidden.

Comment #25

15 April 2025 at 08:48

marcoliver opened merge request !26

Comment #26

marcoliver

he/him

German

Neuss, NRW, Germany

commented 15 April 2025 at 08:49

Status:

Needs work

» Needs review

Hey everybody, I ported the changes from 8.x-2.x to the 3.0.x branch and opened a new MR.

Comment #27

mlncn commented 28 May 2025 at 11:40

Priority:	Normal	» Major
Status:	Needs review	» Reviewed & tested by the community

marcoliver's merge requent !26 works beautifully for 3.0.x; good time to get it in now before further development on that branch!

Comment #28

28 May 2025 at 11:44

ahebrank committed 3f944f08 on 3.0.x authored by marcoliver

Issue #3246687 by prudloff, dhruv.mittal, marcoliver: Don't unset the #...

Comment #29

mlncn commented 28 May 2025 at 11:44

(Oh and respectfully upgraded to major because the way it manifests with Media Entity File Replace #3377463: Incompatibility with media_library_edit results in the file not being replaced and it is extremely difficult to figure out exactly what is happening… if you even notice that people are being served the old file because the new one was never even uploaded (silently failing, no error or warning, seeming successful)!