Skip to content

Commit

Permalink
Support nested elements inside <button>
Browse files Browse the repository at this point in the history
This change is necessary to address a potential issue that could arise when a button or an input of type submit contains child elements, such as spans, icons, or other HTML elements.

Currently, ActiveStorage's `didClick` event listener checks the target of the click event to determine if a submit button was clicked. The target property of the event refers to the specific HTML element that was clicked.

In cases where a submit button contains child elements, and one of these child elements is the element that actually gets clicked, the target would refer to this child element, not the button itself.

Since the `didClick` function checks if the target is a button or an input of type submit, this check would fail, and the button wouldn't be stored in `submitButtonsByForm`.

As a result, if the form is then submitted after a direct upload, the first submit button in the form could be incorrectly used to submit the form, even if a different button was originally clicked. This could cause unexpected behavior, as different submit buttons might be intended to trigger different actions on form submission.

By using the `event.currentTarget` instead, we'll get back the button or an input of type submit. This way we ensure that the correct button is stored in `submitButtonsByForm`, even if the click event was triggered by a child element of the button. This addresses the issue and ensures that the correct button is used to submit the form after a direct upload.

https://developer.mozilla.org/en-US/docs/Web/API/Event/currentTarget
  • Loading branch information
marckohlbrugge committed Nov 13, 2023
1 parent 757f489 commit a1504f4
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 9 deletions.
6 changes: 3 additions & 3 deletions activestorage/app/assets/javascripts/activestorage.esm.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions activestorage/app/assets/javascripts/activestorage.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions activestorage/app/javascript/activestorage/ujs.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ export function start() {
}

function didClick(event) {
const { target } = event
if ((target.tagName == "INPUT" || target.tagName == "BUTTON") && target.type == "submit" && target.form) {
submitButtonsByForm.set(target.form, target)
const submitButton = event.target.closest(":is(button, input)[type='submit']")
if (submitButton && submitButton.form) {
submitButtonsByForm.set(submitButton.form, submitButton)
}
}

Expand Down

0 comments on commit a1504f4

Please sign in to comment.