Fix wrong target for submit buttons in js s3file.js

[anneFly]

Hey @codingjoe 👋
I believe I found a small bug inside the js part of this library.

Description:

Inside the s3file.js there is an event handler for the click event on all submit buttons:

django-s3file/s3file/static/s3file/js/s3file.js

Lines 152 to 155 in 94ee141

	var submitButtons = form.querySelectorAll('input[type=submit], button[type=submit]')
	Array.from(submitButtons).forEach(function (submitButton) {
	submitButton.addEventListener('click', clickSubmit)
	})

When handling this event, the code is trying to copy the name and value form the clicked submit button to a new element. However, the code is using event.target:

django-s3file/s3file/static/s3file/js/s3file.js

Line 108 in 94ee141

var submitButton = e.target

This is a problem because the clicked submit button might have child elements like spans or images. If the user's mouse pointer is on a child element of the button, then event.target will point to that child element. And since this child most likely does not have a name attribute, the new input element will have the name undefined and the data that gets posted to the server will contain undefined=1 instead of the name of the actual submit button.

How to reproduce:

On the main branch you can reproduce it with a submit button that has a child and then you need to click on that child.
E.g. here is a button with a 10 pixels image.

<button type="submit" name="asdf"> <img src="data:image/jpeg;base64,/9j/4AAQSkZJRgABAQEBLAEsAAD//gATQ3JlYXRlZCB3aXRoIEdJTVD/4gKwSUNDX1BST0ZJTEUAAQEAAAKgbGNtcwQwAABtbnRyUkdCIFhZWiAH5gAIAB4ACQAgAAFhY3NwQVBQTAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA9tYAAQAAAADTLWxjbXMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA1kZXNjAAABIAAAAEBjcHJ0AAABYAAAADZ3dHB0AAABmAAAABRjaGFkAAABrAAAACxyWFlaAAAB2AAAABRiWFlaAAAB7AAAABRnWFlaAAACAAAAABRyVFJDAAACFAAAACBnVFJDAAACFAAAACBiVFJDAAACFAAAACBjaHJtAAACNAAAACRkbW5kAAACWAAAACRkbWRkAAACfAAAACRtbHVjAAAAAAAAAAEAAAAMZW5VUwAAACQAAAAcAEcASQBNAFAAIABiAHUAaQBsAHQALQBpAG4AIABzAFIARwBCbWx1YwAAAAAAAAABAAAADGVuVVMAAAAaAAAAHABQAHUAYgBsAGkAYwAgAEQAbwBtAGEAaQBuAABYWVogAAAAAAAA9tYAAQAAAADTLXNmMzIAAAAAAAEMQgAABd7///MlAAAHkwAA/ZD///uh///9ogAAA9wAAMBuWFlaIAAAAAAAAG+gAAA49QAAA5BYWVogAAAAAAAAJJ8AAA+EAAC2xFhZWiAAAAAAAABilwAAt4cAABjZcGFyYQAAAAAAAwAAAAJmZgAA8qcAAA1ZAAAT0AAACltjaHJtAAAAAAADAAAAAKPXAABUfAAATM0AAJmaAAAmZwAAD1xtbHVjAAAAAAAAAAEAAAAMZW5VUwAAAAgAAAAcAEcASQBNAFBtbHVjAAAAAAAAAAEAAAAMZW5VUwAAAAgAAAAcAHMAUgBHAEL/2wBDAAMCAgMCAgMDAwMEAwMEBQgFBQQEBQoHBwYIDAoMDAsKCwsNDhIQDQ4RDgsLEBYQERMUFRUVDA8XGBYUGBIUFRT/2wBDAQMEBAUEBQkFBQkUDQsNFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBT/wgARCAAKAAoDAREAAhEBAxEB/8QAFQABAQAAAAAAAAAAAAAAAAAAAAj/xAAUAQEAAAAAAAAAAAAAAAAAAAAA/9oADAMBAAIQAxAAAAGVAAf/xAAUEAEAAAAAAAAAAAAAAAAAAAAg/9oACAEBAAEFAh//xAAUEQEAAAAAAAAAAAAAAAAAAAAg/9oACAEDAQE/AR//xAAUEQEAAAAAAAAAAAAAAAAAAAAg/9oACAECAQE/AR//xAAUEAEAAAAAAAAAAAAAAAAAAAAg/9oACAEBAAY/Ah//xAAUEAEAAAAAAAAAAAAAAAAAAAAg/9oACAEBAAE/IR//2gAMAwEAAgADAAAAEJJP/8QAFBEBAAAAAAAAAAAAAAAAAAAAIP/aAAgBAwEBPxAf/8QAFBEBAAAAAAAAAAAAAAAAAAAAIP/aAAgBAgEBPxAf/8QAFBABAAAAAAAAAAAAAAAAAAAAIP/aAAgBAQABPxAf/9k=" /> </button> 

If you click directly on that button, you'll see that the data that gets posted to the server will contain undefined=1

Proposed solution in this PR:

I've changed event.target to event.currentTarget. That should always give you the button and not one of its children.
If you try to reproduce the bug on this branch, you should see the expected result asdf=1 in the request data instead of undefined=1

[codecov]

Codecov Report

Merging #228 (07fd7d0) into main (94ee141) will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@ ## main #228 +/- ## ======================================= Coverage 98.02% 98.02% ======================================= Files 8 8 Lines 203 203 ======================================= Hits 199 199 Misses 4 4 

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[codingjoe]

Hi @anneFly,

Long time no see. I hope you are well. Thanks for spotting the error and the fix. That reminds me that this package is overdue for some JS tests.

@SebastianKapunkt would you be interested in setting up jest + coverage here?

Thanks again, I'll release the patch right away.

Best Johannes!