cancel and formnovalidate attributes do not work for buttons with inner elements #1490
Conversation
…r elements A common scenario <button type="submit" class="cancel"><span class="maybesymbolfont">x</span><button> will not work as expected, validation does not get cancelled. Proposed fix searches for the appropriate target when the inner element is clicked
|
Oops, forgot to add in the original comment. I only was able to reproduce this behavior in the Chrome browser. Only Chrome has in the event.target property set to the inner button element when listened for the click on form. Other tested browsers (Firefox, IE11) have the event.target set to the button itself and the problem does not show there. |
sorry, editing over the gighub web interface only
|
jsbin demonstrating the bug: http://output.jsbin.com/pagutolovo/10 The second button should not trigger the validation, however if you click the blue text on the button (make sure you hit the glyphs, not the button face), the validation will run. Chrome only (from what I have tested) |
|
Sorry guys, I found too late this is a duplicate to #1370, just a slightly different solution of the problem |
| if ( validator.settings.submitHandler ) { | ||
| validator.submitButton = event.target; | ||
| } | ||
|
|
||
| // locate the actual "action" element | ||
| var actionTarget = $(event.target); |
There was a problem hiding this comment.
couldn't we just check wheter event.target is of type input, and if not, find for the input starting from event.target?
There was a problem hiding this comment.
I think this is exactly what the code does. With the extension in must not limit the test to input only and needs to work for <button type=”submit”> also. Basically exactly what :submit selector test does. This solution (=use the :submit selector) seems cleaner and more readable to me. Moreover at this point there is no need to worry about the performance.
As for the performance I did not measure the impact of the validateDelegate compound selector, that one I would suspect first.
Just my opinion, if I’m missing anything, please, let me know.
There was a problem hiding this comment.
Right, but I want to achieve it in a different way as implemented currently. Instead of binding to :submit * we should do a find only when event.target doesnt work (means it is not a input).
This would help because only chrome needs to pay for this edge case, not all browsers
There was a problem hiding this comment.
Hmmm after re-reading it looks like everything is in place.
Please just add the unit test
There was a problem hiding this comment.
OK, understand, that makes sense but it implies the other solution described in the issue-1370 … getting rid of the validateDelegate, which now filters out the click events coming from the inner elements. That solution would be more efficient however (and a bigger change in the current code)
There was a problem hiding this comment.
oops, soory, I have missed your previous reply ... ignore the post above ....
|
I like this PR more then the alternative solution in issue-1370 because it is less obtrusive. please integrate the unit test of https://github.com/jzaefferer/jquery-validation/pull/1392/files to make sure you are actually fixing the issue and we wont regress. |
|
Hmm, sorry, I’m lost now. The PR-1392 seems to me to primary focus a quite different topic (clean removing of the validator from DOM) and it “by the way” replaces the |
|
Lets hold on a bit and see what jzaeffer responds in the alternative PR. Thanks for your contribution until now. |
|
Due to recent changes, switching from custom delegation to using |
A common scenario x will not work as expected, validation does not get cancelled. Proposed fix searches for the appropriate target when the inner element is clicked