Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getAnimationStyle causes exceptions in older browsers #24094

Closed
janousek opened this issue May 24, 2018 · 15 comments
Closed

getAnimationStyle causes exceptions in older browsers #24094

janousek opened this issue May 24, 2018 · 15 comments
Assignees
Labels
area: animations freq3: high P2 The issue is important to a large percentage of users, with a workaround regression Indicates than the issue relates to something that worked in a previous version state: has PR type: bug/fix
Milestone

Comments

@janousek
Copy link

janousek commented May 24, 2018

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[X] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

Errors are thrown during collection of animation styles (in older browsers).
The problem is on this line:


Most of browsers will return empty string if the style property is not set. But some browsers (like Chrome on Android 4.4) returns null instead of empty string.
Several string functions are applied on the output of the method "getAnimationStyle" - like trim, split, indexOf, etc... But if the output of the"getAnimationStyle" is null, exception is thrown.

Simple fix of this bug:

  return element.style[ANIMATION_PROP + name] || '';

Expected behavior

Animation should work without errors.

Minimal reproduction of the problem with instructions

What is the motivation / use case for changing the behavior?

Environment


Angular version: 6.0.3


Browser:
- [ ] Chrome (desktop) version XX
- [X] Chrome (Android) version 34
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
 
For Tooling issues:
- Node version: XX  
- Platform:  

Others:

@ngbot ngbot bot added this to the needsTriage milestone May 25, 2018
@matsko matsko added type: bug/fix freq3: high regression Indicates than the issue relates to something that worked in a previous version labels Jul 17, 2018
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Jul 17, 2018
@omieliekh
Copy link

Any progress on this issue? It now causes issues with PhantomJS tests

@janousek
Copy link
Author

janousek commented Aug 9, 2018

It seems that no one cares about this issue. My temporary solution/workaround to prevent errors, looks like this:

@Component({
  ....
})
class AppComponent {

	@HostBinding('@.disabled')
	public disableAnimations = document.createElement('div').style['animation'] == null;
  
  .....
}

This will disable animations in problematic browsers.

@omieliekh
Copy link

@janousek I've solved issue on my side with a polyfill, added this code to src/polyfills.ts

if (document.body.style.animation === undefined && CSSStyleDeclaration) {
  CSSStyleDeclaration.prototype.animation = '';
}
if (document.body.style['animation-name'] === undefined && CSSStyleDeclaration) {
  CSSStyleDeclaration.prototype['animation-name'] = '';
  CSSStyleDeclaration.prototype['animationName'] = '';
}
if (document.body.style['animation-duration'] === undefined && CSSStyleDeclaration) {
  CSSStyleDeclaration.prototype['animation-duration'] = '';
  CSSStyleDeclaration.prototype['animationDuration'] = '';
}
if (document.body.style['animation-play-state'] === undefined && CSSStyleDeclaration) {
  CSSStyleDeclaration.prototype['animation-play-state'] = '';
  CSSStyleDeclaration.prototype['animationPlayState'] = '';
}

Only the last one (with animationPlayState) is fixing my issue, but I've added few more to be more future-proof

@ghernandez81
Copy link

@janousek i fixed my issue with using NoopAnimationsModule in my unit-test

@pedep
Copy link

pedep commented Sep 4, 2018

I think you can get pretty far by adding some vendor prefix checks at these 2 points

maybe simply something like this?

function vendorPrefix() {
  return ['', '-webkit-', '-moz-', '-o-'].find((prefix) =>
    document.body.style[prefix+'animation'] !== undefined || ''
  )
}

// [...]
let keyframeStr = `@${vendorPrefix()}keyframes ${name} {\n`; 
// [...]
const ANIMATION_PROP = vendorPrefix()+'animation'; 

This approach requires calls to setAnimationStyle to use the lowercase-dashes version of the css properties

setAnimationStyle(this._element, 'Delay', `-${this._position}ms`, index);

setAnimationStyle(element, 'PlayState', status, index);

Thoughts?

@Brandinga
Copy link

Brandinga commented Oct 2, 2018

have the same problem:

  • TypeError: Cannot call method 'split' of undefined
  • TypeError: Cannot call method 'trim' of undefined

Errors on the following User-Agents:

  1. Mozilla/5.0 (Linux; Android 4.4.2; Doro Liberto 820 Build/KOT49H) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/30.0.0.0 Mobile Safari/537.36
  2. Mozilla/5.0 (Linux; Android 4.4.2; de-de; SAMSUNG GT-I9301I Build/KOT49H) AppleWebKit/537.36 (KHTML, like Gecko) Version/1.5 Chrome/28.0.1500.94 Mobile Safari/537.36
  3. Mozilla/5.0 (Linux; U; Android 4.4.4; de-at; GT-I9060I Build/KTU84P) AppleWebKit/534.30 (KHTML, like Gecko) Version/4.0 Mobile Safari/534.30
  4. MT6582_TD/V1 Linux/3.4.67 Android/4.4.2 Release/ Browser/AppleWebKit537.36 Chrome/30.0.0.0 Mobile Safari/537.36 System/Android 4.4.2

FYI duplicate issue #25563

@RostyslavVolytskyi
Copy link

@omieliekh your solution will break angular material dropdowns in Safari.

@dottodot
Copy link

dottodot commented Jun 2, 2019

Is there still no fix for this?

@BojanKogoj
Copy link

There is a pull request opened (#29709), but nothing has been done about it.

@matsko any chance this will be merged?

@pascaliske
Copy link

This error also seems to occur in Chrome 70 on Windows 10 because I just got the same error message in my client's Angular app through Sentry: https://sentry.io/share/issue/8c7da85210424e63b96c8f7a10bc9435/

@vlaco
Copy link

vlaco commented Mar 24, 2020

Is there any hope for this being solved soon?

@danwhitston
Copy link

So, this has been blowing up our Sentry logging, and it's an absolutely tiny fix with a PR that has no chance of causing issues, and has been sitting in the queue forever. Seems unfortunate that it's taken over 2 years and several conversations to fail to change a single reference.

@petebacondarwin
Copy link
Member

@danwhitston - sorry that this has not yet been solved. Now that @matsko has left the team, such issues are not getting as much attention as they deserve. Are you able to test the fix given in #29709 for your scenario? If it works for you, I would be willing to try to get it merged.

@petebacondarwin petebacondarwin self-assigned this Aug 20, 2020
@jelbourn jelbourn added the P2 The issue is important to a large percentage of users, with a workaround label Oct 1, 2020
@AndrewKushnir
Copy link
Contributor

Closing this issue since corresponding PR #29709 was merged and should be available in the next Angular release (v11.0.3). Thank you.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: animations freq3: high P2 The issue is important to a large percentage of users, with a workaround regression Indicates than the issue relates to something that worked in a previous version state: has PR type: bug/fix
Projects
None yet
Development

No branches or pull requests