airbnb / lottie-web

Render After Effects animations natively on Web, Android and iOS, and React Native. http://airbnb.io/lottie/
MIT License
30.54k stars 2.87k forks source link

this.elements[i].destroy() is not a function #2275

Closed WartClaes closed 4 years ago

WartClaes commented 4 years ago

Tell us about your environment It happens on all browsers and all os-es

What is the issue?

When using Lottie we encountered a CSP issue when loading animations. This happend because Lottie uses eval() to render certain effects. The easy fix then was to make the switch to Lottie light.

Once this CSP issue was resolved, we got a new error when the animation was destroyed:

https://github.com/airbnb/lottie-web/blob/bf8217770e40cedad1f20ed52b24eddea9b82b77/player/js/renderers/SVGRenderer.js#L142-L155

this.elements[i].destroy() is not a function

The error occured because the elements array was not properly constructed and some items just contained the value true and (true).destroy() triggers the error above.

When looking for the reason why the element contained true we noticed that new SVGEffects() uses the EffectManager.

https://github.com/airbnb/lottie-web/blob/a9a82511344a46906a3c17121a019b1ef860b5e6/player/js/elements/svgElements/SVGEffects.js#L30-L33

This function still exists in Lottie light, but is empty. Lottie crashed on this and catches the error so it is not shown immediately. The browser error happens when destroying and not in the initial loading of the animation.

Solution

By preventing that we add any effects in Lottie light, the animation is loaded in the same way as before, but doesn't crash when destroying.

I already created a PR (#2234) a while ago but I'm not sure if I should have made an issue first.

bodymovin commented 4 years ago

Hi, can you share an example of an animation that triggers this error? I'm trying to reproduce it with the lottie_light.js player and I can't.

WartClaes commented 4 years ago

Here is a file with the issue: https://assets5.lottiefiles.com/packages/lf20_gmgQQi.json

WartClaes commented 4 years ago

@bodymovin Any new about this one? It would be nice to have it fixed :-)

vitaliiburlaka commented 4 years ago

@bodymovin

Hi, can you share an example of an animation that triggers this error? I'm trying to reproduce it with the lottie_light.js player and I can't.

@bodymovin I was able to reproduce the issue in my project using the file from @WartClaes.

bodymovin commented 4 years ago

@WartClaes that animation has an effect appliead to a layer. lottie-light is not expecting effects, and that's why it breaks. Do you have a use case where you need animations with effects played with the light version?

WartClaes commented 4 years ago

Now we have removed them from the animations but in the end it would be nice.

But still it remains the case that it could work perfect without throwing a somewhat hidden error. I get that the effects won't be applied and I can totally live with that, but it would be nice that we can still see the rest of the animation (which is possible with the fix in the linked PR)

WartClaes commented 4 years ago

@bodymovin can you give your opinion on this?

vitaliiburlaka commented 4 years ago

@bodymovin the issue for our project was that the Lottie-web calls eval without the proper try/catch and possibly without check if there are expressions to not.

https://github.com/airbnb/lottie-web/blob/8f905baeba08342a32202ec47339f5e1dbcbf4ad/player/js/utils/expressions/ExpressionManager.js#L375

With the strict Content-Security-Policy that does not allow usage of 'unsafe-eval' it was just throwing that vague this.elements[i].destroy() is not a function error

bodymovin commented 4 years ago

@vitaliiburlaka eval should be invoked only if the there are expressions on the animation. Regarding try/catch, every frame is rendered wrapped in a try / catch statement to prevent errors propagating upwards, so if I'm not wrong, that shouldn't be the issue.

bodymovin commented 4 years ago

@WartClaes I'll see what I can do to prevent this.

WartClaes commented 3 years ago

@bodymovin is this fixed in 5.7.4? Because I just tested it and I still get the same error?

bodymovin commented 3 years ago

@WartClaes version 5.7.5 should have it fully fixed

QuocVi1994 commented 3 years ago

@WartClaes version 5.7.5 should have it fully fixed

i use the version 5.7.5 ,however i still encounter this problem, my chrome version is 87.0.4280.88, when i use firefox it prompt that "Content Security Policy: The page's settings blocked the loading of a resource at eval ("script-src"). lottie.js"

Dar0n commented 3 years ago

@bodymovin I am sorry to bring this up again, but it happens in our project as well. However, the errors point to react-lottie package. Can it be that it is using an outdated lottie-web version? Will it be updated?

razbensimon commented 2 years ago

still happening (also with react-lottie) while our app has a strict Content-Security-Policy... Is there any solution?

QuocVi1994 commented 2 years ago

你的信我已收到,我会尽快阅览并回复,谢谢你的来信!~

wadehammes commented 2 years ago

Happening with me, but only when I am trying to move from Yarn to PNPM (Latest Next.js). My yarn installation works fine, but this error happens when using PNPM.

ConardLi commented 2 years ago

May I ask if this error will affect the normal use of the program after strict Content-Security-Policy is enabled? Because it is difficult for our project to switch to Lottie light version.

oguz3 commented 1 year ago

I have opened a pull request for this. This does not play the animation, but prevents the application from throwing errors. that's enough for my scenario

Donkfather commented 1 year ago

The issue for me happened in a Chrome extension when moved to MV3. It was because of the Content-Security-Policy which didn't allow unsafe-eval. Changed to lottie-light and it worked

NilsBaumgartner1994 commented 7 months ago

Still having this issue in my Expo app