fix(ngOptions): select disabled option when set from model

[Narretz]

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
bug fix

What is the current behavior? (You can also link to an open issue here)
See #12756

What is the new behavior (if this is a feature change)?

Does this PR introduce a breaking change?
Probably not, when we understand it as a bug fix.

Please check if the PR fulfills these requirements

• The commit message follows our guidelines: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit-message-format
• Tests for the changes have been added (for bug fixes / features)

Other information:

When a disabled option is set as the ngModel, ngOptions will now select the option
in the select element, which will set select.val() to the option hash value and visually
show the option value / label as selected in the select box. Previously, disabled
options forced the unknown value.
The previous behavior is inconsistent with both default HTML behavior and select with
ngModel but without ngOptions. Both allow disabled values to be selected programmatically.

Common use cases for this behavior include options that where previously valid, but have
been disabled, and cannot be selected again.

Fixes #12756

[gkalpak]

What is the duplicate test ?

[Narretz]

This one :) It's a duplicate of https://github.com/angular/angular.js/pull/14233/files#diff-2cda0d7a6d8ec47da3560fbe59a634e3R850

[gkalpak]

Are you sure we don't need to also ignore .disabled in selectCtrl.readValue() ?

Also, I can't decide if this is a BC or not ? Sure the new behavior seems more consistent and the old one was not documented, but still...

[Narretz]

Hmm ... readValue is called when the select changes, so on user input. At this point, the selected option shouldn't be disabled anyay, becasue the browser prevents selecting a disabeld option. So it's an additional check that would only come into play when someone selectes a disabled option programmtically and then triggers change. I think the check is okay.

I'm fine with putting it into 1.6, but I also think that this is the expected behavior. I also don't think that anyone's application could break by showing the disabled value. The model is the same in both cases.

[Narretz]

@petebacondarwin wdyt, 1.5 / 1.4 or 1.6 with Breaking Change notice?

[gkalpak]

Hmm ... readValue is called when the select changes, so on user input.

AFAICT, readValue() is read inside updateOptions() which is invoked whenever the collection returnd by ngOptions.getWatchables() changes. I suspect that under certain circumstances, a change could be incurred programmatically to the collection returned by ngOptions.getWatchables(), but I don't have evidence (and not because I didn't try 😛).

So, this is just a hunch. And even if I proved correct, it would only affect some very cornery corner cases, so feel free to ignore.

On the other hand, readValue reads the current value of the <select> element and tries to match it to an option item. I don't think we should care much, how the value ended up into the <select> (be it through user-interaction or other means (e.g. rogrammatically)) - although most browsers don't allow selecting a disabled option.
This would also be more consistent with select without ngOptions (which just returns the value of the <select> element, not caring how it came to be.

Theoretically thinking about it, the disable when construct's purpose should be to set the disabled property of the respective <option> element. How the underlying implementation chooses to handle this (be it a native <select> or even a custom one) is up to them.

That's all too theoretical and minor though. I don't feel strongly either way.
(Besides I might be missing some other side effects of returning disabled options from readValue().)

[petebacondarwin]

Given that this is really only a visual change let's put it in 1.5.x

[petebacondarwin]

Please merge @Narretz