fix(animations): process shorthand `margin` and `padding` styles correctly (#35701)

Prior to this patch, the `margin` and `padding` properties were not
detected properly by Firefox due to them being shorthand properties.
This patch ensures that both `margin` and `padding` are converted
read as `top right bottom left` in the event that the shorthand
property detection fails for auto-styling in Angular animations.

Fix #35463 (FW-1886)

PR Close #35701
This commit is contained in:
Matias Niemelä 2020-02-26 14:54:46 -08:00 committed by atscott
parent d7efc45c04
commit 35c9f0dc2f
9 changed files with 100 additions and 16 deletions

View File

@ -12,7 +12,7 @@
"master": { "master": {
"uncompressed": { "uncompressed": {
"runtime-es2015": 2987, "runtime-es2015": 2987,
"main-es2015": 451469, "main-es2015": 451552,
"polyfills-es2015": 52195 "polyfills-es2015": 52195
} }
} }
@ -21,7 +21,7 @@
"master": { "master": {
"uncompressed": { "uncompressed": {
"runtime-es2015": 3097, "runtime-es2015": 3097,
"main-es2015": 429230, "main-es2015": 429313,
"polyfills-es2015": 52195 "polyfills-es2015": 52195
} }
} }

View File

@ -7,9 +7,9 @@
*/ */
import {AnimationPlayer, ɵStyleData} from '@angular/animations'; import {AnimationPlayer, ɵStyleData} from '@angular/animations';
import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes, computeStyle} from '../../util'; import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes} from '../../util';
import {AnimationDriver} from '../animation_driver'; import {AnimationDriver} from '../animation_driver';
import {containsElement, hypenatePropsObject, invokeQuery, matchesElement, validateStyleProperty} from '../shared'; import {computeStyle, containsElement, hypenatePropsObject, invokeQuery, matchesElement, validateStyleProperty} from '../shared';
import {packageNonAnimatableStyles} from '../special_cased_styles'; import {packageNonAnimatableStyles} from '../special_cased_styles';
import {CssKeyframesPlayer} from './css_keyframes_player'; import {CssKeyframesPlayer} from './css_keyframes_player';
@ -36,7 +36,7 @@ export class CssKeyframesDriver implements AnimationDriver {
} }
computeStyle(element: any, prop: string, defaultValue?: string): string { computeStyle(element: any, prop: string, defaultValue?: string): string {
return (window.getComputedStyle(element) as any)[prop] as string; return computeStyle(element, prop);
} }
buildKeyframeElement(element: any, name: string, keyframes: {[key: string]: any}[]): any { buildKeyframeElement(element: any, name: string, keyframes: {[key: string]: any}[]): any {

View File

@ -7,7 +7,7 @@
*/ */
import {AnimationPlayer} from '@angular/animations'; import {AnimationPlayer} from '@angular/animations';
import {computeStyle} from '../../util'; import {computeStyle} from '../shared';
import {SpecialCasedStyles} from '../special_cased_styles'; import {SpecialCasedStyles} from '../special_cased_styles';
import {ElementAnimationStyleHandler} from './element_animation_style_handler'; import {ElementAnimationStyleHandler} from './element_animation_style_handler';

View File

@ -237,3 +237,37 @@ export function hypenatePropsObject(object: {[key: string]: any}): {[key: string
}); });
return newObj; return newObj;
} }
/**
* Returns the computed style for the provided property on the provided element.
*
* This function uses `window.getComputedStyle` internally to determine the
* style value for the element. Firefox doesn't support reading the shorthand
* forms of margin/padding and for this reason this function needs to account
* for that.
*/
export function computeStyle(element: HTMLElement, prop: string): string {
const gcs = window.getComputedStyle(element);
// this is casted to any because the `CSSStyleDeclaration` type is a fixed
// set of properties and `prop` is a dynamic reference to a property within
// the `CSSStyleDeclaration` list.
let value = gcs[prop as any];
// Firefox returns empty string values for `margin` and `padding` properties
// when extracted using getComputedStyle (see similar issue here:
// https://github.com/jquery/jquery/issues/3383). In this situation
// we want to emulate the value that is returned by creating the top,
// right, bottom and left properties as individual style lookups.
if (value.length === 0 && (prop === 'margin' || prop === 'padding')) {
// reconstruct the padding/margin value as `top right bottom left`
const propTop = (prop + 'Top') as 'marginTop' | 'paddingTop';
const propRight = (prop + 'Right') as 'marginRight' | 'paddingRight';
const propBottom = (prop + 'Bottom') as 'marginBottom' | 'paddingBottom';
const propLeft = (prop + 'Left') as 'marginLeft' | 'paddingLeft';
value = `${gcs[propTop]} ${gcs[propRight]} ${gcs[propBottom]} ${gcs[propLeft]}`;
}
return value;
}

View File

@ -10,7 +10,7 @@ import {AnimationPlayer, ɵStyleData} from '@angular/animations';
import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes, copyStyles} from '../../util'; import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes, copyStyles} from '../../util';
import {AnimationDriver} from '../animation_driver'; import {AnimationDriver} from '../animation_driver';
import {CssKeyframesDriver} from '../css_keyframes/css_keyframes_driver'; import {CssKeyframesDriver} from '../css_keyframes/css_keyframes_driver';
import {containsElement, invokeQuery, isBrowser, matchesElement, validateStyleProperty} from '../shared'; import {computeStyle, containsElement, invokeQuery, isBrowser, matchesElement, validateStyleProperty} from '../shared';
import {packageNonAnimatableStyles} from '../special_cased_styles'; import {packageNonAnimatableStyles} from '../special_cased_styles';
import {WebAnimationsPlayer} from './web_animations_player'; import {WebAnimationsPlayer} from './web_animations_player';
@ -32,7 +32,7 @@ export class WebAnimationsDriver implements AnimationDriver {
} }
computeStyle(element: any, prop: string, defaultValue?: string): string { computeStyle(element: any, prop: string, defaultValue?: string): string {
return (window.getComputedStyle(element) as any)[prop] as string; return computeStyle(element, prop);
} }
overrideWebAnimationsSupport(supported: boolean) { this._isNativeImpl = supported; } overrideWebAnimationsSupport(supported: boolean) { this._isNativeImpl = supported; }

View File

@ -7,7 +7,7 @@
*/ */
import {AnimationPlayer} from '@angular/animations'; import {AnimationPlayer} from '@angular/animations';
import {computeStyle} from '../../util'; import {computeStyle} from '../shared';
import {SpecialCasedStyles} from '../special_cased_styles'; import {SpecialCasedStyles} from '../special_cased_styles';
import {DOMAnimation} from './dom_animation'; import {DOMAnimation} from './dom_animation';

View File

@ -6,9 +6,10 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {AnimateTimings, AnimationMetadata, AnimationMetadataType, AnimationOptions, sequence, ɵStyleData} from '@angular/animations'; import {AnimateTimings, AnimationMetadata, AnimationMetadataType, AnimationOptions, sequence, ɵStyleData} from '@angular/animations';
import {Ast as AnimationAst, AstVisitor as AnimationAstVisitor} from './dsl/animation_ast'; import {Ast as AnimationAst, AstVisitor as AnimationAstVisitor} from './dsl/animation_ast';
import {AnimationDslVisitor} from './dsl/animation_dsl_visitor'; import {AnimationDslVisitor} from './dsl/animation_dsl_visitor';
import {isNode} from './render/shared'; import {computeStyle, isNode} from './render/shared';
export const ONE_SECOND = 1000; export const ONE_SECOND = 1000;
@ -340,7 +341,3 @@ export function visitDslNode(visitor: any, node: any, context: any): any {
throw new Error(`Unable to resolve animation metadata node #${node.type}`); throw new Error(`Unable to resolve animation metadata node #${node.type}`);
} }
} }
export function computeStyle(element: any, prop: string): string {
return (<any>window.getComputedStyle(element))[prop];
}

View File

@ -6,8 +6,6 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {ElementAnimationStyleHandler} from '../../../src/render/css_keyframes/element_animation_style_handler'; import {ElementAnimationStyleHandler} from '../../../src/render/css_keyframes/element_animation_style_handler';
import {computeStyle} from '../../../src/util';
import {assertStyle, createElement, makeAnimationEvent, supportsAnimationEventCreation} from './shared'; import {assertStyle, createElement, makeAnimationEvent, supportsAnimationEventCreation} from './shared';
const EMPTY_FN = () => {}; const EMPTY_FN = () => {};

View File

@ -0,0 +1,55 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {computeStyle} from '../../src/render/shared';
describe('shared animations code', () => {
if (isNode) return;
describe('computeStyle', () => {
it('should compute the margin style into the form top,right,bottom,left', () => {
const div = buildActualElement();
div.style.setProperty('margin', '1px 2px 3px 4px');
expect(computeStyle(div, 'margin')).toEqual('1px 2px 3px 4px');
div.style.setProperty('margin', '0px');
div.style.setProperty('margin-top', '10px');
div.style.setProperty('margin-right', '20px');
div.style.setProperty('margin-bottom', '30px');
div.style.setProperty('margin-left', '40px');
expect(computeStyle(div, 'margin')).toEqual('10px 20px 30px 40px');
});
it('should compute the padding style into the form top,right,bottom,left', () => {
const div = buildActualElement();
div.style.setProperty('padding', '1px 2px 3px 4px');
expect(computeStyle(div, 'padding')).toEqual('1px 2px 3px 4px');
div.style.setProperty('padding', '0px');
div.style.setProperty('padding-top', '10px');
div.style.setProperty('padding-right', '20px');
div.style.setProperty('padding-bottom', '30px');
div.style.setProperty('padding-left', '40px');
expect(computeStyle(div, 'padding')).toEqual('10px 20px 30px 40px');
});
});
});
/**
* Returns a div element that's attached to the body.
*
* The reason why this function exists is because in order to
* compute style values on an element is must be attached within
* the body of a webpage.
*/
function buildActualElement() {
const div = document.createElement('div');
document.body.appendChild(div);
return div;
}