Added support for button elements with descendants #1901
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Non-Firefox browsers dispatch "click" on descendants of <button>, which breaks the submitHandler logic that adds a hidden field with the name and value of the submit button (since it is not included in the submission if it did not initiate the submission, which might be the case when using form.submit in submitHandler and returning false). submitButton is instead a descendant of <button> and the hidden field does not compensate for the lack of an actual submit button. This would fix crbug.com/668524 and make submitButton an actual button.
For testing that clicking on descendants of a button element still keeps the button element as the submit button.
For testing that clicking on descendants of a button element still keeps the button element as the submit button.
Accidental copy-paste.
Forgot to wrap the parenthesized value with spaces.
Forgot to wrap a parenthesized value with spaces.
Forgot to add a blank line above a comment.
Replaced a complex event initiation with a basic click().
Removed extraneous trailing spaces.
Removed two unused variables.
Instead of returning a DOM element, a jQuery element was returned. Changed to return the original element in the target is already a button case.
|
An alternative approach is to use |
Instead of looking for the parent or ancestor of the target, just used the currentTarget property.
|
The |
b1zzu
suggested changes
Jan 26, 2017
|
@davbizz - I did already. See c257fc0. |
Arkni
requested changes
Jan 27, 2017
| button = $( "#testForm27 :submit" )[ 0 ]; | ||
| $( button ).find( "span" ).click(); | ||
| } ); | ||
|
|
There was a problem hiding this comment.
You can extend your tests by testing the property submitButton of the validator. So, your tests will look like this:
var button = $( "#testForm27 :submit" )[ 0 ];
var v = $( "#testForm27" ).validate( {
debug: true,
submitHandler: function( form ) {
// Compare the button with the `submitButton` property
assert.deepEqual(
v.submitButton, button, "The submitButton property should be the same as button"
);
var hidden = $( form ).find( "input:hidden" )[ 0 ];
assert.deepEqual( hidden.value, button.value );
assert.deepEqual( hidden.name, button.name );
return false;
}
} );
$( "#testForm27 [name=\"year\"]" ).val( "2016" );
$( button ).find( "span" ).click();There was a problem hiding this comment.
I added your test as well.
Devised by @Arkni.
|
Thx for your fix. I will merge this now but have mixed feelings about it. Be warned that we could revert this again in case it breaks something... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JSFiddle -
https://jsfiddle.net/0b3we0gm/9/ (with the fix)
https://jsfiddle.net/0c8nvnob/7/ (without the fix)
Non-Firefox browsers dispatch
clickon descendants of<button>, which breaksthe submitHandler logic that adds a hidden field with the name and value of the
submit button (since it is not included in the submission if it did not
initiate the submission, which might be the case when using
form.submitinsubmitHandlerand returningfalse).submitButtonis instead a descendant of<button>and the hidden field does not compensate for the lack of an actualsubmit button.
This would fix https://crbug.com/668524 and make
submitButtonan actual button.The issue is reproducible in Chrome.
Firefox does not dispatch events to the descendants of
<button>(but the specification requires that it will, see https://bugzilla.mozilla.org/show_bug.cgi?id=1089326 for details). The rest of the gang dispatch events to the descendants, so I imagine the problem exists in all of them and not just in Chrome.