Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix #12584: don't ignore disabled property of select-one options after form reset #932

Closed
wants to merge 1 commit into from

3 participants

Richard Gibson Mike Sherov Dave Methvin
Richard Gibson
Collaborator

Sizes - compared to master @ 6ad4a0e

    265231      (-108)  dist/jquery.js                                         
     93498       (-33)  dist/jquery.min.js                                     
     33455        (-3)  dist/jquery.min.js.gz
Dave Methvin

The logic changes above preserve the IE6 bug fix, but I'm concerned we'll not realize that nothing that convoluted is required once we drop oldIE support. Really, if we didn't need this bug fix we could just drop this block of code and #12584 would be fixed as a result. So we'll want to end up back at that in jQuery 2.0. Thoughts on how we could do that?

Collaborator

That describes a general issue (already present in much of our source), and at the simplest level it's addressed by mentioning oldIE in the comments to highlight a block that can be improved in 2.0. @mikesherov's concept of explicitly and formally tracking environment-specific logic would be a godsend here. Code coverage analysis would be even more rigorous, and hopefully we'll get that. Regardless, I expect that much of the 2.x trajectory will be removing code as we discover it to be unnecessary for supported environments.

Mike Sherov
Collaborator

@gibson042, yes, I just had a ticket that I closed as wontfix as it was just too much to do under the scope of one ticket, but I assume that'll be the MAIN exercise for the work we need to do for 2.0: identify oldIE code paths!

Dave Methvin dmethvin closed this in 425d17d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 21, 2012
  1. Richard Gibson
This page is out of date. Refresh to see the latest.
Showing with 20 additions and 20 deletions.
  1. +13 −19 src/attributes.js
  2. +7 −1 test/unit/attributes.js
32 src/attributes.js
View
@@ -225,26 +225,25 @@ jQuery.extend({
},
select: {
get: function( elem ) {
- var value, i, max, option,
- index = elem.selectedIndex,
- values = [],
+ var value, option,
options = elem.options,
- one = elem.type === "select-one";
-
- // Nothing was selected
- if ( index < 0 ) {
- return null;
- }
+ index = elem.selectedIndex,
+ one = elem.type === "select-one" || index < 0,
+ values = one ? null : [],
+ max = one ? index + 1 : options.length,
+ i = index < 0 ?
+ max :
+ one ? index : 0;
// Loop through all the selected options
- i = one ? index : 0;
- max = one ? index + 1 : options.length;
for ( ; i < max; i++ ) {
option = options[ i ];
- // Don't return options that are disabled or in a disabled optgroup
- if ( option.selected && (jQuery.support.optDisabled ? !option.disabled : option.getAttribute("disabled") === null) &&
- (!option.parentNode.disabled || !jQuery.nodeName( option.parentNode, "optgroup" )) ) {
+ // oldIE doesn't update selected after form reset (#2551)
+ if ( ( option.selected || i === index ) &&
+ // Don't return options that are disabled or in a disabled optgroup
+ ( jQuery.support.optDisabled ? !option.disabled : option.getAttribute("disabled") === null ) &&
+ ( !option.parentNode.disabled || !jQuery.nodeName( option.parentNode, "optgroup" ) ) ) {
// Get the specific value for the option
value = jQuery( option ).val();
@@ -259,11 +258,6 @@ jQuery.extend({
}
}
- // Fixes Bug #2551 -- select.val() broken in IE after form.reset()
- if ( one && !values.length && options.length ) {
- return jQuery( options[ index ] ).val();
- }
-
return values;
},
8 test/unit/attributes.js
View
@@ -713,7 +713,7 @@ test("removeProp(String)", function() {
});
test("val()", function() {
- expect( 20 + ( jQuery.fn.serialize ? 6 : 0 ) );
+ expect( 21 + ( jQuery.fn.serialize ? 6 : 0 ) );
document.getElementById("text1").value = "bla";
equal( jQuery("#text1").val(), "bla", "Check for modified value of input element" );
@@ -756,6 +756,12 @@ test("val()", function() {
jQuery("#select5").val(3);
equal( jQuery("#select5").val(), "3", "Check value on ambiguous select." );
+ strictEqual(
+ jQuery("<select name='select12584' id='select12584'><option value='1' disabled='disabled'>1</option></select>").val(),
+ null,
+ "Select-one with only option disabled (#12584)"
+ );
+
if ( jQuery.fn.serialize ) {
var checks = jQuery("<input type='checkbox' name='test' value='1'/><input type='checkbox' name='test' value='2'/><input type='checkbox' name='test' value=''/><input type='checkbox' name='test'/>").appendTo("#form");
Something went wrong with that request. Please try again.