fix(select): ensure the label attribute is updated in Internet Explorer #10042
Conversation
petebacondarwin
force-pushed
the
select-label-fix
branch
from
9fe02fa to
7852b3f
Compare
| @@ -1300,6 +1320,25 @@ describe('select', function() { | |||
| expect(scope.selected).toBe(scope.values[0]); | |||
| }); | |||
|
|
|||
| // bug fix #9621 | |||
| if (msie) { | |||
| it('should update the label attribute if running in MS IE', function() { | |||
There was a problem hiding this comment.
does this test not work outside of IE? or is the behaviour different in IE? I feel like we should probably run the test for both IE and not
|
I am trying to reproduce the issue that this fixes on IE10 on SL, but I'm not able to (using basically a reproduction of the unit test case). Is the unit test testing the actual issue? |
|
@caitp - You are right. The unit test does not actually test the issue. It tests the fix. I will try to make a more realistic test. |
|
Unfortunately, as this is a rendering issue, I cannot reproduce in a unit test. The programmatic interface is accurate, but it just hasn't triggered the correct re-rendering of the currently selected label. |
|
OK, so we don't need to update the |
|
That means that we don't need the first commit, the second becomes simpler, we can apply the fix to all browsers and the test can be applied to all browsers too. It is also simple enough that it should be able to go into 1.2.x |
petebacondarwin
force-pushed
the
select-label-fix
branch
from
7852b3f to
d3e407d
Compare
…tion` matcher By using a new matcher our tests become less brittle with respect to unimportant extra attributes.
Only changing the `<option>` text value is not enough to trigger a render change in IE. We need to explicit update the `label` property too. Closes angular#9621
petebacondarwin
force-pushed
the
select-label-fix
branch
from
d3e407d to
640cd1b
Compare
|
@caitp - it is not possible to actually test for the bug (without taking a snapshot of the screen and comparing images) since it is not exposed by the programmatic interface. The test checks that the fix is in place. I changed the code to update the |
|
hmm, and I guess reftesting it isn't really possible either... alright, lgtm |
|
I don't know what a reftest is. Is it this: http://adobe.github.io/web-platform/presentations/testtwf-how-to-write-a-reftest/ |
|
yeah basically --- it's a testing strategy used in (some) browsers, but there are some tools that offer this as well. |
Fixes #9621