Remove reference to msTransition for IE10

[themikelee]

This fixes an issue with transitions in IE10. For example dialog with transitions on never removes the elements in that browser.

In IE10 msTransition exists but msTransitionEnd never fires. IE10 does however support standard transitionend. Since IE<10 does not support transitions this line can be removed entirely.

This has already been done in Twitter Bootstrap. Reference here: twbs/bootstrap#4166

[petebacondarwin]

Is there any way to create a unit test for this?

[themikelee]

Well I'm fairly new (about 2 weeks 😄) to both angular and unit testing. However, I think the strange situation where the functionality exists in IE but is never actually called in real usage is messing with the test. I see a test that checks for transitionend correctly calling the handler. After much wrangling* I was able to run the tests and confirm that on the existing code they pass in IE10. I think something about the way the test runs triggers the event which doesn't actually fire in real world. So the existing test should cover the new code but wasn't sufficient to catch the error. I don't know how or if it's possible to get the test to cover this odd case.

• On a side note, I have not been able to get grunt test to run in Windows. It fails almost immediately with no real explanation. I managed to get testacular running manually but almost 2/3 of the tests fail. I may be missing something with my setup. Luckily the set of tests for transition doesn't seem to be among them and I'm able to confirm that my pull request passes in IE10 (though the existing code also passes so not sure how much that's worth).

[pkozlowski-opensource]

@themikelee how do you run tests that you see them failing?

Are you using grunt test? If so you need to run the html2js task at least once (this tasks converts directive's templates to JS files to be but in $templateCache so unit tests don't need to load partials).

Try simply running grunt to run the whole build.

[themikelee]

@pkozlowski-opensource Thanks that makes some significant progress for me. I was running the tests with the command testacular.cmd start testacular.conf. I've modified the browsers in the conf to have the path to the browser since testacular can't detect them by name in Windows. Running the full build, grunt test at least launches the browser now. However it still fails with this message.

Running "test" task
ERROR [launcher]: Cannot start C:\Program Files (x86)\Internet Explorer\iexplore
.exe

ERROR [launcher]: Cannot start C:\Program Files (x86)\Internet Explorer\iexplore
.exe

ERROR [launcher]: Cannot start C:\Program Files (x86)\Internet Explorer\iexplore
.exe

Warning: Task "test" failed. Use --force to continue.

Aborted due to warnings.

Three IE windows are actually launched in quick succession during this process which then fail to connect because grunt has already quit. Running it with chrome also fails in the same manner but leaves out the 3 error messages. Something about running through grunt is causing testacular to immediately register a failed connection without waiting. Then grunt quits. Running testacular manually also launches three windows or tabs but does stick around long enough to connect all three and run the tests. This means it's annoyingly running the tests 3 times but at least it's running. Thanks to having run the build there are now only 18 failed out of 252 tests in IE10 which is a significant improvement over what I was getting before. Chrome passes all tests.

[themikelee]

Well after some more experimentation I've discovered how to get Testacular running properly in Windows. The reason grunt didn't work but running testacular.cmd did was because I wasn't passing the --single-run option. This caused the testacular server to stick around and all the browsers that it didn't detect or wait for were then able to connect. Next I found that setting the _BIN env variables actually causes the browser detection to work. Why this is different from setting a custom browser via the browsers array in the conf is beyond me but it is. Setting the env for chrome and using browsers = ['Chrome', 'IE'] works perfectly. grunt now works it runs through the test and then stops because of the IE failures. Running it while only testing chrome makes its way through the entire build.

Thanks, and sorry for turning this into a "get testacular working in windows" thread. I've been searching all over and haven't been able to find this information anywhere. Also doesn't help that testacular has been renamed to karma and the google group moved so none of the links pointing to google groups discussions work anymore.

[pkozlowski-opensource]

@petebacondarwin honestly I've got no idea how this one could be tested :-) WDYT?

[redaemn]

@pkozlowski-opensource does anyone know how did they test it at https://github.com/twitter/bootstrap?!?

[pkozlowski-opensource]

@redaemn It is simple, they didn't test it at all :-)
twbs/bootstrap@802ced9

[redaemn]

@pkozlowski-opensource in the original commit twbs/bootstrap@2a0cf0f taken from the pull request twbs/bootstrap#4166 I read:

Current unit test requires no attention in light of this change.

What do they mean by this?!? Maybe someone could look into this a little bit

[themikelee]

@pkozlowski-opensource @redaemn Looks like all they do is make sure that if transition has been set as supported then transitionend has also been set as supported. https://github.com/twitter/bootstrap/blob/master/js/tests/unit/bootstrap-transition.js

[pkozlowski-opensource]

@themikelee cool, but I don't get they test :-)

I would tend to trust you here and merge this PR. @petebacondarwin ?

[pkozlowski-opensource]

Landed as 55437b1, thank you!

[themikelee]

Cheers! 🍻