Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
If you click on add to shortcut multiple times(before the yellow start appears), will add multiple shortcuts of the same item
Steps to reproduce
Click on the Add to shortcut multiple times, will create multiple shortcut(provided, click should be made continuously)
Comment | File | Size | Author |
---|---|---|---|
#13 | 2370115-13.patch | 605 bytes | Upchuk |
#4 | 2370115-4.jpg | 70.48 KB | sidharthap |
shortcut multiple.jpg | 56 KB | nafsinvk |
Comments
Comment #1
Upchuk CreditAttribution: Upchuk commentedComment #2
Upchuk CreditAttribution: Upchuk commentedComment #3
tstoecklerWow, nice find!
Comment #4
sidharthapIn 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
Comment #9
jibranThe 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.Comment #11
borisson_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.
Comment #13
Upchuk CreditAttribution: Upchuk as a volunteer commentedSo 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.
Comment #14
jibranNice! let's add some tests.
Comment #15
Upchuk CreditAttribution: Upchuk as a volunteer commented@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?
Comment #16
jibran@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. :)
Comment #17
Upchuk CreditAttribution: Upchuk as a volunteer commented@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?
Comment #18
jibranI asked @larowlan for second opinion on #17, let's create a follow-up.
Comment #20
jibranFail seems unrelated so back to RTBC.
Comment #21
Upchuk CreditAttribution: Upchuk as a volunteer commentedFollowup created, see #3015264: Tests for double click prevention on form submission.
Comment #22
alexpottWhy 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...
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.
Comment #23
Upchuk CreditAttribution: Upchuk as a volunteer commented@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
Comment #24
catchCommitted and pushed edfd5bbeae to 8.7.x and 30fcc3ca95 to 8.6.x. Thanks!
Comment #28
eleleka CreditAttribution: eleleka at Skilld commented