No double click prevention on content entity form

Couldn't this be fixed by simply attaching the core/drupal.form library for everyone? Curtently is the library attached only for anonymous users with the specific Anonymous commenting settings value. See the proposed patch.

Applied patch #4 on 9.5.x.The multiple submissions of comment form is not happening after applying this patch.

Attaching screen recordings for reference.

Patch #4 applied on 9.4.8 and doesn't appear to have introduced any issues.

Just as important, I've not seen any duplicate comments since!

…I would love to see this committed, as duplicate comments has been a longstanding issue on my site. Not sure why. Maybe particularly inpatient users. Or maybe it's related to the fact that the majority of my site users use some form of assistive technology to access the web (mostly screen readers), and possibly this increases the likelihood of the submit button being clicked multiple times.

Many thanks to those working on resolving this issue for us all.

Rerolled the patch #4 against 10.1.x

@Anchal_gupta You mentioned reroll, but your patch contains a new MainContentViewSubscriberTest test class. Can you explain please if it was added intentionally or by mistake?

yes, Poker10 I checked again my patch then I found it was added by mistake. sorry for that

I have applied patch #10 and it is working as intended, when clicked on the 'Save' button multiple times, the comment is only posted once. Below are the screenshot of the issue before the patch and screen-recording of the issue after the patch.

The solution in #4 seems to be the right approach. The drupal.form library has repeat submission prevention built in. My concern is that its inclusion comes with a great deal of other functionality that could potentially disrupt functionality of an existing site. Were this being introduced as part of new functionality, it would be 100% fine, but there may be existing sites that run into issues if this broader inclusion of drupal.form adds jQuery to the page, broadcasts the formUpdated event, etc.

I can think of two different ways to address:

1. Create a temporary 10.x library that duplicates Drupal.behaviors.formSingleSubmit without the additional functionality of the form library. In 11.x drupal.form can be included instead of the temporary library, and provide a change record to document the potential changes that come with including the full library
2. Audit the additional functionality introduced by the addition of core/drupal.form and confirm that nothing added would potentially disrupt a site

It's an elegant solution overall, I'd just like to be sure it doesn't adversely impact an site that implicitly expects core/drupal.form functionality to not be present.

*********

The reroll in #8 was not needed - using "add test/retest" in #4 I was able to confirm it applies to 10.1.x without issue. In addition to it being unnecessary, it introduced code that is out of scope in the issue yet #11 RTBCd it. That adds unnecessary noise to issues! So... @Anchal_gupta be sure to check if a reroll is actually needed (a version change does not automatically === reroll needed) @fadilraj please read fully through the issue before reviewing so you aren't reviewing a patch that was confirmed as having mistakes.

Hi everyone,

Thank you very much for raising this issue and for all the code contributions, in particular the patches, it's greatly appreciated.

We encountered the same issue with custom content entities in general ContentEntityType and not only with the comment form, that's why the title was updated accordingly.
Since Comment is also a content entity (ContentEntityType), we would like to suggest that perhaps a more generic approach or solution is taken as an attempt to provide a solution for all content entities, if possible.

Therefore, please find attached to this comment a corresponding patch on the 11.x branch:
drupal-core-double-click-prevention-on-content-entity-form-3220709-16.patch

The idea of the patch is to attach core library core/drupal.form (which prevents double click submissions) to the forms of any entity extending class ContentEntityBase
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

(Which is the case for Comment entities, see: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/comme...)

Which should not only fix the problem for Comment forms, but also any custom content entity type, in general.

We would greatly appreciate if you could please try testing the patch and give us your feedback.

Feel free to let us know if you have any questions or concerns on any aspects of the patch or the ticket in general, we would surely be very happy to help.
Thanks in advance !

Thanks for working on this and a good suggestion @DYdave.

However I think that the points from #13 are still valid. We need to create a separate library with only Drupal.behaviors.formSingleSubmit and include this library instead of core/drupal.form. Thanks!

Hi Juraj (@poker10),

Thanks a lot for your prompt follow-up on this and helpful reply, it's greatly appreciated.

Thanks a lot for re-centering the ticket and bringing to our attention the points from #13:

Let's try to go through the different points again :

1 - Help transitioning D10 sites towards D11 change :

We need to create a separate library with only Drupal.behaviors.formSingleSubmit and include this library instead of core/drupal.form.

Create a temporary 10.x library that duplicates Drupal.behaviors.formSingleSubmit without the additional functionality of the form library. In 11.x drupal.form can be included instead of the temporary library, and provide a change record to document the potential changes that come with including the full library

a. D11:
Sounds like the patch from #16 should cover the second part of the requirements, in particular for D11, which seems to be the current version of this ticket.
Do you think the current patch and approach could work if we consider D11 + change record ?
 

b. D10:
Otherwise, for the first part of the requirements for D10, we've sort of got the idea of the changes that could be suggested:

We'd have to extract (or duplicate ?) the code of Drupal.behaviors.formSingleSubmit:
https://git.drupalcode.org/project/drupal/-/blob/0a0cb1b40f8425be728fc71...
to a separate file and add it as a core library, for example core/drupal.form-single-submit.

If the JS code was extracted and not duplicated from form.js, add the library as a depency of core/drupal.form.
Then attach the file to content entity forms, by using the added library core/drupal.form-single-submit instead of core/drupal.form in the patch from #16.
 

2 - Audit core/drupal.form

Audit the additional functionality introduced by the addition of core/drupal.form and confirm that nothing added would potentially disrupt a site

Sounds good ! That could certainly be done separately, along with the changes suggested above.
 

What do you think of this summary of the changes suggested so far ?
Overall, we'd still have to audit the JS file and fix the changes for D10.
Otherwise, changes for D11 should be addressed in patch #16.

Of course, I suppose we might have to write tests (for example, ensuring the scripts are correctly loaded on comment edit form), change record (for D11), probably some doc for D10 and perhaps more tasks.
But at this point, we'd like to know whether the approach is correct mostly and if the points listed in this comment give a clearer vision of the remaining work in this ticket.
 

We have started using the patch on development environments and couldn't see any side effects so far, from any of the additional JS in form.js, but we're going to keep testing and will surely be happy to report back.

Therefore, we're probably not going to take development work any further, with the first part for D10 (1.b temporary 10.x library), since the patch works for us "as-is".

At least #13 would seem to confirm we are on the right track with this patch for D11.
 

In any case, we would greatly appreciate if you could please try testing the patch, review this comment and give us your feeback.

Switching status back to Needs review so we can get more feedback on patch for D11 (current version of ticket).

Feel free to let us know if you have any questions, concerns or suggestions on any points in this comment, the patch from #16 or the ticket in general, we would surely be very happpy to help.
Thanks in advance for your great help and contributions.

There's an issue with this patch it dose solve the problem of multiple commenting but when user comment for 1 time , user cant able to comment again

Thanks a lot Kalash for the feedback, which gives a good start to the audit.

We might have to re-consider the patch and implement the change suggested above to extract only the piece of JS needed from form.js.

I'll give this a round of testing on a fresh D10 site and report back !

Thanks !

Tagging for tests to show this issue.

@DYdave @smustgrave
Can you please explain what is pending as a part of this issue?