From b5ba9a82195a993a214ee6ff4d10d4f51f5c9597 Mon Sep 17 00:00:00 2001
From: Steven Orvell <sorvell@google.com>
Date: Thu, 4 Feb 2016 17:55:52 -0800
Subject: [PATCH] Fixes #3326. Changes inspired by #3276 and #3344

---
 src/lib/style-properties.html          | 14 ++---------
 src/standard/styling.html              | 11 ++++++++-
 src/standard/x-styling.html            | 16 +++++++++++++
 test/unit/styling-cross-scope-var.html |  3 ++-
 test/unit/styling-remote.html          | 20 ++++++++++++----
 test/unit/styling-scoped.html          | 33 +++++++++++++++++++-------
 6 files changed, 69 insertions(+), 28 deletions(-)

diff --git a/src/lib/style-properties.html b/src/lib/style-properties.html
index 544a0be984..b1c649558f 100644
--- a/src/lib/style-properties.html
+++ b/src/lib/style-properties.html
@@ -88,17 +88,7 @@
         // but not production, so strip out {...}
         cssText = cssText.replace(this.rx.BRACKETED, '')
           .replace(this.rx.VAR_ASSIGN, '');
-        var parts = cssText.split(';');
-        for (var i=0, p; i<parts.length; i++) {
-          p = parts[i];
-          if (p.match(this.rx.MIXIN_MATCH) ||
-              p.match(this.rx.VAR_MATCH) ||
-              this.rx.ANIMATION_MATCH.test(p) ||
-              styleUtil.isKeyframesSelector(rule)) {
-            customCssText += p + ';\n';
-          }
-        }
-        return customCssText;
+        return cssText;
       },
 
       collectPropertiesInCssText: function(cssText, props) {
@@ -369,7 +359,7 @@
         var parts = selector.split(',');
         for (var i=0, l=parts.length, p; (i<l) && (p=parts[i]); i++) {
           parts[i] = p.match(hostRx) ?
-            p.replace(hostSelector, hostSelector + scope) :
+            p.replace(hostSelector, scope) :
             scope + ' ' + p;
         }
         rule.selector = parts.join(',');
diff --git a/src/standard/styling.html b/src/standard/styling.html
index 134d6362fb..80bc2ed1b9 100644
--- a/src/standard/styling.html
+++ b/src/standard/styling.html
@@ -42,8 +42,17 @@
         }
         if (this._template) {
           this._styles = this._collectStyles();
+          // calculate shimmed styles (we must always do this as it
+          // stores shimmed style data in the css rules for later use)
           var cssText = styleTransformer.elementStyles(this);
-          if (cssText) {
+          // do we really need to output shimmed styles
+          var needsStatic = this._needsStaticStyles(this._styles);
+          cssText = needsStatic ? cssText : '';
+          // under shady dom we always output a shimmed style (which may be 
+          // empty) so that other dynamic stylesheets can always be placed 
+          // after the element's main stylesheet. 
+          // This helps ensure element styles are always in registration order.
+          if (needsStatic || !nativeShadow) {
             var style = styleUtil.applyCss(cssText, this.is,
               nativeShadow ? this._template.content : null);
             // keep track of style when in document scope (polyfill) so we can
diff --git a/src/standard/x-styling.html b/src/standard/x-styling.html
index 8e0ef8ba00..b64d5a9c45 100644
--- a/src/standard/x-styling.html
+++ b/src/standard/x-styling.html
@@ -26,6 +26,22 @@
 
     Polymer.Base._addFeature({
 
+      // Skip applying CSS if there are some mixins or variables used
+      // since styles with mixins and variables will be added on later stages anyway,
+      // and will include styles applied here, no need to do this twice
+      _needsStaticStyles: function(styles) {
+        var needsStatic;
+        for (var i=0, l=styles.length, css; i < l; i++) {
+          css = styles[i].textContent;
+          needsStatic = needsStatic || Boolean(css);
+          if (css.match(propertyUtils.rx.MIXIN_MATCH) || 
+              css.match(propertyUtils.rx.VAR_MATCH)) {
+            return false;
+          }
+        }
+        return needsStatic;    
+      },
+
       _prepStyleProperties: function() {
         // note: an element should produce an x-scope stylesheet
         // if it has any _stylePropertyNames
diff --git a/test/unit/styling-cross-scope-var.html b/test/unit/styling-cross-scope-var.html
index ffa5540135..03e368350c 100644
--- a/test/unit/styling-cross-scope-var.html
+++ b/test/unit/styling-cross-scope-var.html
@@ -795,10 +795,11 @@
   });
 
   // skip for now, until #3326 is fixed
-  test.skip('custom style class overrides css variable', function() {
+  test('custom style class overrides css variable', function() {
     var d = document.createElement('x-variable-override');
     d.classList.add('variable-override');
     document.body.appendChild(d);
+    CustomElements.takeRecords();
     assertComputed(d, '10px');
   });
 
diff --git a/test/unit/styling-remote.html b/test/unit/styling-remote.html
index 4fd71c0df1..80eb3ed13a 100644
--- a/test/unit/styling-remote.html
+++ b/test/unit/styling-remote.html
@@ -199,13 +199,23 @@
       });
 
       test('styles shimmed in registration order', function() {
+        var regList = Polymer.telemetry.registrations.map(function(reg) {
+          return reg.is;
+        });
+        function regIndex(styleScope) {
+          for (var i=0; (r=regList[i]); i++) {
+            if (styleScope.match(new RegExp(r + '\\-?\\d*$'))) {
+              return i;
+            }
+          }
+        }
         var s$ = document.head.querySelectorAll('style[scope]');
-        var expected = ['x-order', 'x-child2', 'x-styled', 'x-styled-0', 'x-button', 'x-dynamic-scope'];
-        var actual = [];
-        for (var i=0; i<s$.length; i++) {
-          actual.push(s$[i].getAttribute('scope'));
+        for (var i=0, scope, n=0, o=0; i<s$.length; i++) {
+          scope = s$[i].getAttribute('scope');
+          n = regIndex(scope);
+          assert.isTrue(n >= o, 'style not in registration order: ' + scope);
+          o = n;
         }
-        assert.deepEqual(actual, expected);
       });
     });
   }
diff --git a/test/unit/styling-scoped.html b/test/unit/styling-scoped.html
index 6a27b2aa6b..d3215818d6 100644
--- a/test/unit/styling-scoped.html
+++ b/test/unit/styling-scoped.html
@@ -249,17 +249,23 @@
       });
 
       test('styles shimmed in registration order', function() {
+        var regList = Polymer.telemetry.registrations.map(function(reg) {
+          return reg.is;
+        });
+        function regIndex(styleScope) {
+          for (var i=0; (r=regList[i]); i++) {
+            if (styleScope.match(new RegExp(r + '\\-?\\d*$'))) {
+              return i;
+            }
+          }
+        }
         var s$ = document.head.querySelectorAll('style[scope]');
-        var expected = ['x-keyframes', 'x-keyframes-1', 'x-keyframes-0', 'x-gchild', 'x-child2',
-          'x-styled', 'x-button', 'x-mixed-case', 'x-mixed-case-button',
-          'x-dynamic-scope', 'x-dynamic-template', 'x-dynamic-svg',
-          'x-specificity', 'x-overriding', 'x-overriding-0',
-          'x-specificity-parent-0', 'x-specificity-nested-0'];
-        var actual = [];
-        for (var i=0; i<s$.length; i++) {
-          actual.push(s$[i].getAttribute('scope'));
+        for (var i=0, scope, n=0, o=0; i<s$.length; i++) {
+          scope = s$[i].getAttribute('scope');
+          n = regIndex(scope);
+          assert.isTrue(n >= o, 'style not in registration order: ' + scope);
+          o = n;
         }
-        assert.deepEqual(actual, expected);
       });
 
       test('svg elements properly scoped', function() {
@@ -306,6 +312,15 @@
       test('specificity of ::content > :not(template) selector', function() {
         assertComputed(document.querySelector('[is=x-specificity-nested]'), '10px');
       });
+
+      test('overwriting mixin properties', function() {
+        var root = document.querySelector('x-overriding');
+        assertComputed(root.querySelector('.red'), '1px');
+        assertComputed(root.querySelector('.green'), '2px');
+        assertComputed(root.querySelector('.red-2'), '1px');
+        assertComputed(root.querySelector('.blue'), '3px');
+      });
+
     });
 
     test('svg classes are dynamically scoped correctly', function() {