fix(select): normalize options without value attrs

[cironunes]

In order to fix #6351 I'm submitting this changes based on @caitp comments.
Note that the commented expectation is returning [["? undefined:undefined ?"],"--select--","x","y"] is it expected?

[mary-poppins]

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

• Uses the issue template (#6519)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

[lgalfaso]

I am not sure if this is a valid issue, but even if (the empty string) should be the default value when there is no value attribute, the PR does not handle the case where there are other attributes other than value

[cironunes]

@lgalfaso thank you for the feedback! What do you recommend? Just handle the case of other attributes or something else?

[cironunes]

I just updated that to handle other HTML attributes.

[lgalfaso]

@cironunes I think it still needs a small refinement, please check the comments I posted. BTW, thanks for the patch!

[cironunes]

@lgalfaso thank you for the comments. I tried to implement your check but looks like that not solve the issue.

BTW even returning undefined it fix the #6351

[lgalfaso]

@cironunes it took me some extra time to understand what is going on, but I think I have all the moving parts now

• When a <option> does not have a value attribute, it takes the content of the element as the value
• When a <select> has an ng-options attribute, the <select> elements are ignored with the exception of the <option> element that has a value attribute with the value ""

This implies, the correct check should still be || nodeName_(children[i]) === 'OPTION' && isUndefined(children.eq(i).attr('value'))

The test from the patch are ok, but there should be tests that check that when there is an hg-options then the element with no attribute named value is picked as the /empty/ element

[cironunes]

@lgalfaso thank you for that.

I totally forgot about jqlite to check the attr.

[cironunes]

Need to take a look deeper to understand why in IE9 it's returning a string instead of an array.

[petebacondarwin]

@cironunes this patch is very old. There was a major refactoring of ng-options in 1.4, which has fixed these issues: see eb19368 and 4baf25b.

I am going to close this issue as we really want people to upgrade to 1.4. Please open another issue if this is not acceptable.

[cironunes]

👍