Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Select multiple will return null when no values returned #2562

Closed
DataTables opened this issue Aug 31, 2015 · 18 comments
Closed

Select multiple will return null when no values returned #2562

DataTables opened this issue Aug 31, 2015 · 18 comments

Comments

@DataTables
Copy link

I've just been looking into an issue whereby if you have a select element with the multiple option set and no options selected $().val() will return null rather than an empty array. If one more more items are selected, then it returns an array. I would have expected that an empty array would have been returned in this case.

This fiddle shows the issue. IT appears all the way back to jQuery 1.2 where multiple value support was added.

The reason an empty array is not returned is the check for elem.selectedIndex being < 0 here. While I understand that for a single select, I don't while get why that would be done for a multi-select.

Removing || index < 0 would allow the empty array to be returned.

Having said that, I'm not really expecting this change to be made as I guess a number of sites will depend upon this behaviour. I'm more wondering if someone might be willing to say why it is done this way? I've searched this issue tracker and the bugs.jquery.com site, but couldn't find any mention of it.

@NeelBhatt
Copy link

It is mentioned in official site of jQuery:

http://api.jquery.com/val/

In the case of select elements, it returns null when no option is selected and an array containing the value of each selected option when there is at least one and it is possible to select more because the multiple attribute is present.

@DataTables
Copy link
Author

I read that page and completely missed that part. Sorry!

However, it doesn't explain why this is the case? I don't really understand why you would want null rather than an array since you would expect an array for all other cases (when n items are selected and n>0).

@NeelBhatt
Copy link

I am not sure why exatly but i guess they told to use like this:

$('select').val() || [];

Which is nothing but a short circuit evaluation which is used to set default values!

Means you will not have null in that case!

Fiddle: http://jsfiddle.net/jzyyhosx/2/

@dmethvin
Copy link
Member

If it's documented and it's been that way since jQuery 1.2, we'd most likely be breaking a lot of code by changing its behavior. I don't recall why it's done that way but as @NeelBhatt says it's easy to get the result you want.

@gibson042
Copy link
Member

Ugh. We actually have one test that $( selectMultiple_allSelectedOptionsDisabled ).val() is an empty array, and another (more recent) test that $( selectMultiple_noSelectedOptions ).val() is null. Even though returning null is longstanding, we might want to change it just for consistency.

@mgol
Copy link
Member

mgol commented Aug 31, 2015

@gibson042 I agree, this is too inconsistent to keep it like that. I'll reopen so that others can voice their opinions.

@mgol mgol reopened this Aug 31, 2015
@dmethvin
Copy link
Member

We should be careful that changing .val() for this case doesn't change the successful-ness of the control. I agree that we're pretty inconsistent but people may be depending on the current values and behavior. See http://jsbin.com/webugutedi/1/edit?html,js,console

@gibson042
Copy link
Member

We're all set there because .val() is called inside a .map callback, where returning null has the same effect as returning [] (i.e., no contribution from the context element).

@NeelBhatt
Copy link

Can not it be done by putting empty string instead of null here:

return val == null ?
                "" :
                jQuery.isArray( val ) ?
                    jQuery.map( val, function( val ) {
                        return { name: elem.name, value: val.replace( rCRLF, "\r\n" ) };
                    }) :
                    { name: elem.name, value: val.replace( rCRLF, "\r\n" ) }; 

So even though people currently using || [] will not get affected by ORing with empty string.

See Demo: http://jsfiddle.net/jzyyhosx/3/

@gibson042
Copy link
Member

That block of code doesn't even need to change because null and empty-array val yield identical results. But returning empty string there would definitely be incorrect.

@gibson042
Copy link
Member

This was accepted in today's meeting.

@timmywil
Copy link
Member

timmywil commented Nov 3, 2015

I forgot what we decided. Do we want to change the $( selectMultiple_allSelectedOptionsDisabled ).val() case to return null or the $( selectMultiple_noSelectedOptions ).val() case to return empty array?

@mgol
Copy link
Member

mgol commented Nov 3, 2015

Both should return an empty array. The idea is that if we ask for a collection (i.e. "select multiple") then we should get a collection, although it might be empty.

@timmywil
Copy link
Member

timmywil commented Nov 3, 2015

right, thanks.

@aried3r
Copy link

aried3r commented Jun 15, 2016

We are in the process of migrating to jQuery 3 in one of our projects and ran into this today. We couldn't find this as a breaking change in the upgrade guide nor found the documentation to be exact (emphasis mine).

When the first element in the collection is a select-multiple (i.e., a select element with the multiple attribute set), it returns an array containing the value of each selected option, or null if no options are selected.

Did we miss something or is the upgrade guide missing this?

@dmethvin
Copy link
Member

It's mentioned here:
https://jquery.com/upgrade-guide/3.0/#breaking-change-select-multiple-with-nothing-selected-returns-an-empty-array

And seems to be working that way in 3.0:
http://jsbin.com/cuhenofazi/edit?html,js,console,output

There's a docs bug here, it just got missed for the 3.0 api update:
jquery/api.jquery.com#828

@aried3r
Copy link

aried3r commented Jun 15, 2016

Thanks! Seems we missed this, we were looking for .val() and did not find it in the upgrade guide. Thank you for clearing this up. Since this concerns .val(), too, could it be mentioned in the upgrade guide as well?

And is there a way I could help to get this doc update going? :)

@dmethvin
Copy link
Member

Sure, you can file PRs for jquery/api.jquery.com#828 and one for the the upgrade guide at https://github.com/jquery/jquery.com/, that would be greatly appreciated!

aried3r added a commit to aried3r/api.jquery.com that referenced this issue Jun 16, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jun 18, 2018
rany2 added a commit to rany2/jqGrid that referenced this issue Mar 2, 2023
rany2 added a commit to rany2/jqGrid that referenced this issue Mar 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

7 participants
@dmethvin @timmywil @gibson042 @aried3r @mgol @NeelBhatt and others