fix($$RAFProvider): check for webkitCancelRequestAnimationFrame.

[Traxmaxx]

Request Type: bug

How to reproduce: Use AngularJS 1.2.14 together with ngAnimate in Cordova/PhoneGap WebView on Android 4.3

Component(s): ngAnimate

Impact: small

Complexity: small

This issue is related to:

Detailed Description:

RAFProvider is checking for cancelAnimationFrame and webkitCancelAnimationFrame ( see

angular.js/src/ng/raf.js

Line 8 in 04d7317

var cancelAnimationFrame = $window.cancelAnimationFrame ||

)
For Android 4.3 we need to check for webkitCancelRequestAnimationFrame too, otherwise the animations will fail.

Other Comments:

No breaking changes, closes #6526

[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 (#6542)

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!

[tbosch]

Hi,
thanks for the PR. Could you also create a unit test for this?

[Traxmaxx]

Heya,
I assumed it's already covered by https://github.com/angular/angular.js/blob/master/test/ng/rafSpec.js and took a closer look today. Tests for the different rAF APIs is indeed missing.

I'm currently facing problems on how to test the different rAF implementations. I started with adding a spy like that:

beforeEach(module(function($provide) {
  $provide.value('$window', {
    requestAnimationFrame: jasmine.createSpy('$window.requestAnimationFrame')
  });
}));

but expect($window.requestAnimationFrame).toHaveBeenCalled; is always passing, even when it should fail. Do you have a idea what I'm missing?

[quantizor]

Is there an Android browser included in the E2E tests? Otherwise the situation where it is needed would not arise. I don't see one listed: https://github.com/angular/angular.js/blob/master/karma-shared.conf.js

[caitp]

We don't, but it looks like SL will let us test on a couple of more recent android platforms, and some iOS platforms, so it should be easy to hook those in. Want to analyze the costs and send a PR with the configuration changes and that info? Or maybe it would be better if someone internal looked at doing that (or maybe for 1.x we just don't care, but we'll probably want this for 2.x)

[quantizor]

Sure thing!

[quantizor]

PR opened #6578

[tbosch]

@Traxmaxx The problem is that angular-mocks contains a mock implementation of $$rAF.

For $httpBackend we have the same problem (there is a mock implementation and a real one). To test the real one, src/ng/httpBackend.js defines a top level function createHttpBackend that is used in the tests to create the real instance.

Applying this to rAF: Add a top level function createRAF($window) in src/ng/raf.js and make the provider use that function as well. Then call createRAFin your unit test to create the real rAF service and pass in your fake window.

E.g. change src/ng/raf.js like this:
function $$RAFProvider(){ //rAF this.$get = ['$window', createRAF]; } function createRAF($window) { var requestAnimationFrame = $window.requestAnimationFrame || $window.webkitRequestAnimationFrame; ... }

Note that this is working as during the unit tests of Angular we don't add the surrounding function closure of Angular, so unit tests are able to access all the details of Angular.

[Traxmaxx]

@tbosch thanks for the detailed explanation : )
I made the changes and added a unit test for the Android fallback.

[matsko]

@Traxmaxx could you copy the test spec from here? This way you don't have to rewrite raf.js and expose a function name. Look at the test code, it creates a custom injector to work around the ngMock defaults: matsko@3439ca6

[Traxmaxx]

Of course! I'll take care of it.

[matsko]

@Traxmaxx any progress?

[Traxmaxx]

@matsko fixed and pushed : )

[matsko]

MERGED

[matsko]

Landed as c839f78 and e84da22