From 35482babd9eb0970edbc99f223e04594a9d09364 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Go=C5=82e=CC=A8biowski?= Date: Mon, 17 Oct 2016 23:08:41 +0200 Subject: [PATCH] refactor($sniffer): remove $sniffer.vendorPrefix Previously, Angular tried to detect the CSS prefix the browser supports and then use the saved one. This strategy is not ideal as currently some browsers are supporting more than one vendor prefix. The best example is Microsoft Edge that added -webkit- prefixes to be more Web-compatible; Firefox is doing a similar thing on mobile. Some of the -webkit--prefixed things are now even getting into specs to sanction that they're now required for Web compatibility. In some cases Edge even supports only the -webkit--prefixed property; one example is -webkit-appearance. $sniffer.vendorPrefix is no longer used in Angular core outside of $sniffer itself; taking that and the above problems into account, it's better to just remove it. The only remaining use case was an internal use in detection of support for transitions/animations but we can directly check the webkit prefix there manually; no other prefix matters for them anyway. $sniffer is undocumented API so this removal is not a breaking change. However, if you've previously been using it in your code, just paste the following to get the same function: var vendorPrefix = (function() { var prefix, prop, match; var vendorRegex = /^(Moz|webkit|ms)(?=[A-Z])/; for (prop in document.createElement('div').style) { if ((match = vendorRegex.exec(prop))) { prefix = match[0]; break; } } return prefix; })(); The vendorPrefix variable will contain what $sniffer.vendorPrefix used to. Note that we advise to not check for vendor prefixes this way; if you have to do it, it's better to check it separately for each CSS property used for the reasons described at the beginning. If you use jQuery, you don't have to do anything; it automatically adds vendor prefixes to CSS prefixes for you in the .css() method. Fixes #13690 Closes #15287 --- src/ng/sniffer.js | 29 ++++----------------- test/helpers/privateMocks.js | 16 +++++++----- test/ng/snifferSpec.js | 43 +++----------------------------- test/ngAnimate/animateCssSpec.js | 15 ++++++----- 4 files changed, 26 insertions(+), 77 deletions(-) diff --git a/src/ng/sniffer.js b/src/ng/sniffer.js index 60ed39d81318..217f3ba9d0a7 100644 --- a/src/ng/sniffer.js +++ b/src/ng/sniffer.js @@ -34,33 +34,15 @@ function $SnifferProvider() { toInt((/android (\d+)/.exec(lowercase(($window.navigator || {}).userAgent)) || [])[1]), boxee = /Boxee/i.test(($window.navigator || {}).userAgent), document = $document[0] || {}, - vendorPrefix, - vendorRegex = /^(Moz|webkit|ms)(?=[A-Z])/, bodyStyle = document.body && document.body.style, transitions = false, - animations = false, - match; + animations = false; if (bodyStyle) { - for (var prop in bodyStyle) { - if ((match = vendorRegex.exec(prop))) { - vendorPrefix = match[0]; - vendorPrefix = vendorPrefix[0].toUpperCase() + vendorPrefix.substr(1); - break; - } - } - - if (!vendorPrefix) { - vendorPrefix = ('WebkitOpacity' in bodyStyle) && 'webkit'; - } - - transitions = !!(('transition' in bodyStyle) || (vendorPrefix + 'Transition' in bodyStyle)); - animations = !!(('animation' in bodyStyle) || (vendorPrefix + 'Animation' in bodyStyle)); - - if (android && (!transitions || !animations)) { - transitions = isString(bodyStyle.webkitTransition); - animations = isString(bodyStyle.webkitAnimation); - } + // Support: Android <5, Blackberry Browser 10, default Chrome in Android 4.4.x + // Mentioned browsers need a -webkit- prefix for transitions & animations. + transitions = !!('transition' in bodyStyle || 'webkitTransition' in bodyStyle); + animations = !!('animation' in bodyStyle || 'webkitAnimation' in bodyStyle); } @@ -90,7 +72,6 @@ function $SnifferProvider() { return eventSupport[event]; }, csp: csp(), - vendorPrefix: vendorPrefix, transitions: transitions, animations: animations, android: android diff --git a/test/helpers/privateMocks.js b/test/helpers/privateMocks.js index 1a1580b2c118..897e2a290e5d 100644 --- a/test/helpers/privateMocks.js +++ b/test/helpers/privateMocks.js @@ -36,7 +36,7 @@ function browserSupportsCssAnimations() { return !(window.document.documentMode < 10); } -function createMockStyleSheet(doc, prefix) { +function createMockStyleSheet(doc) { doc = doc ? doc[0] : window.document; var node = doc.createElement('style'); @@ -57,13 +57,17 @@ function createMockStyleSheet(doc, prefix) { }, addPossiblyPrefixedRule: function(selector, styles) { - if (prefix) { - var prefixedStyles = styles.split(/\s*;\s*/g).map(function(style) { - return !style ? '' : prefix + style; + // Support: Android <5, Blackberry Browser 10, default Chrome in Android 4.4.x + // Mentioned browsers need a -webkit- prefix for transitions & animations. + var prefixedStyles = styles.split(/\s*;\s*/g) + .filter(function(style) { + return style && /^(?:transition|animation)\b/.test(style); + }) + .map(function(style) { + return '-webkit-' + style; }).join('; '); - this.addRule(selector, prefixedStyles); - } + this.addRule(selector, prefixedStyles); this.addRule(selector, styles); }, diff --git a/test/ng/snifferSpec.js b/test/ng/snifferSpec.js index 7216bc005b66..9ab3a7315ebe 100644 --- a/test/ng/snifferSpec.js +++ b/test/ng/snifferSpec.js @@ -172,39 +172,6 @@ describe('$sniffer', function() { }); - describe('vendorPrefix', function() { - it('should return the correct vendor prefix based on the browser', function() { - inject(function($sniffer, $window) { - var expectedPrefix; - var ua = $window.navigator.userAgent.toLowerCase(); - if (/edge/i.test(ua)) { - expectedPrefix = 'Ms'; - } else if (/chrome/i.test(ua) || /safari/i.test(ua) || /webkit/i.test(ua)) { - expectedPrefix = 'Webkit'; - } else if (/firefox/i.test(ua)) { - expectedPrefix = 'Moz'; - } else if (/ie/i.test(ua) || /trident/i.test(ua)) { - expectedPrefix = 'Ms'; - } - expect($sniffer.vendorPrefix).toBe(expectedPrefix); - }); - }); - - - it('should still work for an older version of Webkit', function() { - var mockDocument = { - body: { - style: { - WebkitOpacity: '0' - } - } - }; - - expect(sniffer({}, mockDocument).vendorPrefix).toBe('webkit'); - }); - }); - - describe('animations', function() { it('should be either true or false', inject(function($sniffer) { expect($sniffer.animations).toBeDefined(); @@ -222,13 +189,12 @@ describe('$sniffer', function() { }); - it('should be true with vendor-specific animations', function() { + it('should be true with -webkit-prefixed animations', function() { var animationStyle = 'some_animation 2s linear'; var mockDocument = { body: { style: { - WebkitAnimation: animationStyle, - MozAnimation: animationStyle + webkitAnimation: animationStyle } } }; @@ -299,13 +265,12 @@ describe('$sniffer', function() { }); - it('should be true with vendor-specific transitions', function() { + it('should be true with -webkit-prefixed transitions', function() { var transitionStyle = '1s linear all'; var mockDocument = { body: { style: { - WebkitTransition: transitionStyle, - MozTransition: transitionStyle + webkitTransition: transitionStyle } } }; diff --git a/test/ngAnimate/animateCssSpec.js b/test/ngAnimate/animateCssSpec.js index b0e7cf58faef..079716f8eacf 100644 --- a/test/ngAnimate/animateCssSpec.js +++ b/test/ngAnimate/animateCssSpec.js @@ -16,8 +16,8 @@ describe('ngAnimate $animateCss', function() { } function getPossiblyPrefixedStyleValue(element, styleProp) { - var value = element.css(prefix + styleProp); - if (isUndefined(value)) value = element.css(styleProp); + var value = element.css(styleProp); + if (isUndefined(value)) value = element.css('-webkit-' + styleProp); return value; } @@ -40,11 +40,10 @@ describe('ngAnimate $animateCss', function() { color: 'blue' }; - var ss, prefix, triggerAnimationStartFrame; + var ss, triggerAnimationStartFrame; beforeEach(module(function() { return function($document, $sniffer, $$rAF, $animate) { - prefix = '-' + $sniffer.vendorPrefix.toLowerCase() + '-'; - ss = createMockStyleSheet($document, prefix); + ss = createMockStyleSheet($document); $animate.enabled(true); triggerAnimationStartFrame = function() { @@ -873,8 +872,8 @@ describe('ngAnimate $animateCss', function() { angular.element($document[0].body).append($rootElement); - ss.addRule('.ng-enter-stagger', prefix + 'animation-delay:0.2s'); - ss.addRule('.transition-animation', 'transition:2s 5s linear all;'); + ss.addPossiblyPrefixedRule('.ng-enter-stagger', 'animation-delay:0.2s'); + ss.addPossiblyPrefixedRule('.transition-animation', 'transition:2s 5s linear all;'); for (var i = 0; i < 5; i++) { var element = angular.element('
'); @@ -2508,7 +2507,7 @@ describe('ngAnimate $animateCss', function() { } }, function(testDetailsFactory) { inject(function($animateCss, $rootElement) { - var testDetails = testDetailsFactory(prefix); + var testDetails = testDetailsFactory(); ss.addPossiblyPrefixedRule('.ng-enter', testDetails.css); var options = {