trentrichardson / jQuery-Impromptu

An extention to help provide a more pleasant way to spontaneously prompt a user for input.
http://trentrichardson.com/Impromptu/
MIT License
327 stars 144 forks source link

jQuery-Impromptu & Content Security Policy (CSP) #58

Closed martineeez closed 9 years ago

martineeez commented 9 years ago

Hello, since Impromptu applies inline event handlers (considered as "bad" practice) it will cause trouble in combination with a strict CSP. http://www.html5rocks.com/en/tutorials/security/content-security-policy/ Might consider to use only event listeners at future versions.

Another thing I noticed: Nothing prevents multiple form submissions. Kind regards, Martinez

trentrichardson commented 9 years ago

@martineeez I only saw one inline event declared, which was onsubmit="return false;" and I removed it. I'm thinking on what approach to take in regards to multiple form submissions without breaking older code. I may provide enable/disable api methods, disable on the first submit, and to reenable you must call $.prompt.enableButtons() (or something along those lines)

martineeez commented 9 years ago

Another approach would be to set a timeout after submission. In this case users merely need to set the timeout duration. :-)

And sorry Trent, I forgot to mention that inline CSS can cause trouble too.

martineeez commented 9 years ago

Thank you for the super fast fixes.

I tried the buttonTimeout option and noticed that it's still possible to submit a form twice. A second submission can go through while a prompt is closing or changing it's state.

Regards, Martinez

trentrichardson commented 9 years ago

@martineeez Were you testing from the dev branch? It was working for me, although I may have missed something or a specific scenario..

martineeez commented 9 years ago

Yes I was using the dev branch. I looked into it and found out that it happens when I send an AJAX request after submission. I'm parsing the response in order to decide which state follows, thus opening a short time frame that allows further submissions.

trentrichardson commented 9 years ago

Ok, so what might work in that scenario is to turn off buttonTimeout (pass -1) then in the submit callback/event call $.prompt.disableStateButtons(). Since you never know how long the ajax request will be buttonTimeout may not work and this way you manually tell it to disable and re-enable when you're done with the ajax?

Working through this to figure out what will work best for every situation, thanks for your input!

martineeez commented 9 years ago

I've done so and it seems to be working now. Thank you, too :-)

martineeez commented 9 years ago

Unfortunately it still happens. I couldn't reproduce it first.

I guess the buttonTimeout is cleared when a prompt is closing or changing its state as well? If so the timeout might needs to be cleared a little later, not until the fadeOut function has completed?

trentrichardson commented 9 years ago

Currently the code doesn't ever clear the timeout, it waits for the timeout to expire (which by default is half a second).

Another approach could be this: When a button is clicked disable the buttons. Buttons will not be re-enabled until a state change or re-enabled manually.

This essentially removes the use of the setTimeout, however may break some existing code people have written that doesn't expect the buttons to ever be disabled.

martineeez commented 9 years ago

I'll make a demo and reopen the issue when I'm done.