fix(select) ... breaks some selects with required attributes on firefox.

[eendeego]

The commit dc149de breaks selects with a required attribute and an disabled default empty option, but only on firefox.

Please check this jsbin on both chrome and firefox.

[Narretz]

It looks like the optional select is also broken on Chrome, because the select has A selected, whereas 'Choose a choice' should be selected

[caitp]

we filed an issue about this with Chromium, actually, because it works as expected in firefox, but differently in chrome --- mostly because that section of html is a bit underspecified.

I haven't kept up with the issues though, don't think I added myself to the CC list

[Narretz]

Oh, so it's actually correct in Firefox: because the first field is disabled, the second field is automatically selected? But @luismreis says that's broken. So maybe the expectation is that a disabled field can still be preselected (but not user selected), because you don't want to make a selection for the user ... regardless of required state.

[caitp]

the scenario which is broken is sort of hard to explain, it's easiest to just show the fiddle --- it's in one of the issues about this, can't recall OTOH though

[eendeego]

@Narretz The scenario is this: you need a select control for something that is required, but you don't want to pre-choose one of the normal options. A way to do this is by having a default value that is disabled, so, the user can choose another one, but not the initial void/empty one.

I prefer the "show disabled option" behaviour, but I understand that it feels a bit broken - maybe I should be using another widget for this.

BTW, I started to have doubts on this being pure browser or angular bejaviour and now there's a new version of the fiddle at (http://jsbin.com/wobod/4). The problem is exactly the same regardless if angular is involved or not.

[jeffbcross]

After some investigation with @ealtenho, I think the only way to satisfy these constraints for a consistent experience in all browsers is to create a new directive (e.g. ngOptionDisabled).

Constraints:

• Have the disabled option displayed and selected by default
• Style the disabled option in UA-specific way, indicating that it's not a choice
• Consider the select element invalid if the disabled element is selected, if the HTML5 required attribute is set
• Do not allow user to select disabled option (after having selected an enabled option)

The approach would involve setting the disabled property after the option has been added to the document. Setting the disabled attribute on an already-selected, already-rendered option will not de-select it, and will cause it to be styled as a legitimately-disabled option. Presuming the option has the appropriate value (empty string in Chrome, not set in Firefox, who knows in IE...), then the disabled option would still be selected, the select would have a validity.valid value of "false", the UA styling of the option would be correct, and the user could not re-select the option after an enabled option had been selected. We tested this approach in Chrome and Firefox.

Here's the script we were using to test in FF and Chrome:

var select = document.createElement('select');
select.setAttribute('required', true);
var option1 = document.createElement('option');
option1.setAttribute('selected', true);
//if (Chrome) option1.value = ''
select.appendChild(option1);
option1.text = 'One';
var option2 = document.createElement('option');
option2.setAttribute('value', '2');
option2.text = 'Two';
select.appendChild(option2);
document.body.appendChild(select);

//select.validity.valid == false
//Selected option in browser should be 'One'

option1.setAttribute('disabled', true);
//select.validity.valid == false
//Selected option in browser should be 'One'

@caitp interested in your thoughts on this approach

[ealtenho]

@jeffbcross @caitp @btford
Another option @jeffbcross and I discussed was examining dc149de which is referenced as the origin of several regressions.

Particularly, we wonder if the alteration in line 540 is a source of error.

Pursuing this strategy, I've been trying to develop a PR:
ealtenho@6adbc1e
However, this currently breaks an e2e test that creates a change based on a blur event.
I'd appreciate any advice.

[caitp]

Basically what the issue came down to was, the existing algorithm was causing a hovered property to be deselected/flicker when frequent digests were happening frequently (if I recall correctly).

I tried a few times to come up with an algorithm which would make each browser happy, but it turned out to be not very easy :( It might actually be easier to just fix the bug in Gecko, and revert the patch here which introduced the regressions.

As it stands, that part of the code is kind of maze-like and very hard to reason about, so I think it's okay to not come up with a workable solution on the first try. I'm sure there's something that could be done about it, but it might be easier to just fix it in C++ instead of JS.

[ealtenho]

Thanks @caitp ! I appreciate the advice.

[ealtenho]

Currently, our plan to deal with this issue is to

1. Create a failing test to ensure that reverting dc149de fixes this issue
2. Revert dc149de
3. Document why we have decided to revert this commit and explain that ngModelOptions could be used by developers if they have concerns about the digest issue
4. File an issue with Firefox about the select problem

[ealtenho]

@btford
Originally, this issue of a disabled option in a required select being skipped over was opened as a bug created by the fix of a Firefox error in dc149de.

The following tests shows this select box error.

it('should allow the selection of a disabled option as the first element in a required select',
        function() {
          var html = '<select';
          html += ' required>';
          html += '<option value="" disabled>Choose One</option>';
          html += '<option value="1">Option One</option>';
          html += '</select>';

          compile(html);

          document.body.innerHTML = html;

          var options = element.find('option');
          var optionToSelect = options.eq(0);
          expect(optionToSelect.text()).toBe('Choose One');
          expect(optionToSelect.prop('selected')).toBe(true);

          dealoc(element);
    });

It fails in Firefox (but passes in Chrome), documenting this issue.
However, reverting dc149de has no effect on this failing test. It does not appear that dc149de is the cause of this issue.

This appears to be a separate Firefox behavior. Hence a pertinent question now is whether Angular should alter this behavior using something like ngOptionDisabled as @jeffbcross describes.

[lgalfaso]

I would expect for b8ae73e to fix this issue. Can you please check if this issue is present at 1.3.beta15? If it is not, then I can back port this issue to the 1.2.x branch