Clicking 'Add to shortcuts' adds shortcut multiple times

Wow, nice find!

In my case, The message shows once but when I navigate to shortcut set it seems multiple times added.
It should validate the path prior to add.

Thanks

The quick fix would be to add JS to the 'Add to shortcuts' link and disable it once it's clicked. A complete fix would be to not allow adding the same link twice to the shortcut set. I think we should add JS in this issue and UniqueShotcutValidator to the Shortcut entity in a follow-up.

The quick fix would be to add JS to the 'Add to shortcuts' link and disable it once it's clicked. A complete fix would be to not allow adding the same link twice to the shortcut set. I think we should add JS in this issue and UniqueShotcutValidator to the Shortcut entity in a follow-up.

I think that's a good start. This will be really hard to test though. So I don't think a test will be needed.

Removing the shortcut issue tag.

So the problem with this is actually pretty simple.

The drupal.form library provides already the functionality for preventing multiple submits in one go. And all core entity forms attach their own JS libraries which in turn depend on this. But shortcut does not. So this is why it's not happening on other entity types.

So the patch is really simple. But yeah, not sure how to test this, other than just checking the library is on the page. Not seeing how this logic is tested elsewhere.

Attached the patch that fixes it.

Nice! let's add some tests.

@jibran, like it was discussed, testing is very difficult for this. Moreover, the original logic that applies to all other forms was committed without any testing. See #1705618: Double click prevention on form submission

Any suggestions?

@Upchuk when that issue was committed 5 years ago in #1705618-42: Double click prevention on form submission we didn't have JS tests in core. Now we can write two types of JS test, one written in PHP and other in JS.
This is a behavioral bug so the way the user is interacting with the site is wrong. I think we can mimic multiple clicks using JS tests or at least try. :)

@jibran ok, but then clearly testing this does not belong in the shortcut module. The test for this is a core test that deals with the generic form logic that prevents multiple submissions. So it is outside the scope of this issue I think. No?

I asked @larowlan for second opinion on #17, let's create a follow-up.

Fail seems unrelated so back to RTBC.

Followup created, see #3015264: Tests for double click prevention on form submission.

Why don't all forms automatically have this added. I guess it's to have no javascript pages since this would add quite a bit of JS...

drupal.form:
  version: VERSION
  js:
    misc/form.js: {}
  dependencies:
    - core/jquery
    - core/drupal
    - core/drupal.debounce
    - core/jquery.cookie
    - core/jquery.once

The weird thing is that by default I'd want Drupal to do the multiple submission protection and have a way for forms to opt out of this.

@alexpott Yeah, I agree. Maybe the js can be moved to another library that only contains that small bit and then added across all the forms. But let's talk about this in the followup ticket I opened where we should also test this: #3015264: Tests for double click prevention on form submission

Committed and pushed edfd5bbeae to 8.7.x and 30fcc3ca95 to 8.6.x. Thanks!