Merge pull request #3470 from Polymer/fix-3461

Fixes #3461: Only avoid creating a statically scoped stylesheet when …

Author: kevinpschaaf

polymer.html

@@ -32,10 +32,8 @@
       this._prepConstructor();
       // template
       this._prepTemplate();
-      // styles
+      // styles and style properties
       this._prepStyles();
-      // style properties
-      this._prepStyleProperties();
       // template markup
       this._prepAnnotations();
       // accessors

src/lib/style-properties.html

@@ -82,12 +82,14 @@
 
       // returns cssText of properties that consume variables/mixins
       collectCssText: function(rule) {
-        var cssText = rule.parsedCssText;
-        // NOTE: we support consumption inside mixin assignment
-        // but not production, so strip out {...}
-        cssText = cssText.replace(this.rx.BRACKETED, '')
+        return this.collectConsumingCssText(rule.parsedCssText);
+      },
+
+      // NOTE: we support consumption inside mixin assignment
+      // but not production, so strip out {...}
+      collectConsumingCssText: function(cssText) {
+        return cssText.replace(this.rx.BRACKETED, '')
           .replace(this.rx.VAR_ASSIGN, '');
-        return cssText;
       },
 
       collectPropertiesInCssText: function(cssText, props) {
@@ -312,7 +314,7 @@
       _elementKeyframeTransforms: function(element, scopeSelector) {
         var keyframesRules = element._styles._keyframes;
         var keyframeTransforms = {};
-        if (!nativeShadow) {
+        if (!nativeShadow && keyframesRules) {
           // For non-ShadowDOM, we transform all known keyframes rules in
           // advance for the current scope. This allows us to catch keyframes
           // rules that appear anywhere in the stylesheet:

src/standard/styling.html

@@ -45,8 +45,13 @@
           // 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);
-          // do we really need to output shimmed styles
-          var needsStatic = this._needsStaticStyles(this._styles);
+          // prepare to shim style properties.
+          this._prepStyleProperties();
+          // do we really need to output static shimmed styles?
+          // only if no custom properties are used since otherwise
+          // styles are applied via property shimming.
+          var needsStatic = this._styles.length && 
+            !this._needsStyleProperties();
           // 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. 

src/standard/x-styling.html

@@ -18,34 +18,17 @@
     var serializeValueToAttribute = Polymer.Base.serializeValueToAttribute;
 
     var propertyUtils = Polymer.StyleProperties;
-    var styleUtil = Polymer.StyleUtil;
     var styleTransformer = Polymer.StyleTransformer;
     var styleDefaults = Polymer.StyleDefaults;
 
     var nativeShadow = Polymer.Settings.useNativeShadow;
 
     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 = styleUtil.parser._clean(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
-        this._ownStylePropertyNames = this._styles ?
+        this._ownStylePropertyNames = this._styles && this._styles.length ?
           propertyUtils.decorateStyles(this._styles) :
           null;
       },

test/unit/styling-cross-scope-apply.html

@@ -269,6 +269,27 @@
     </script>
   </dom-module>
 
+  <dom-module id="x-var-produce-via-consume">
+  <template>
+    <style>
+      :host {
+        display: block;
+        border: 10px solid orange;
+        --foo: {
+          color: var(--bar);
+        }
+      }
+    </style>
+        </template>
+  <script>
+  HTMLImports.whenReady(function() {
+    Polymer({
+      is: 'x-var-produce-via-consume'
+    });
+  });
+  </script>
+</dom-module>
+
 
 <script>
 suite('scoped-styling-apply', function() {
@@ -361,10 +382,28 @@
     });
   });
 
+  test('producing a var that consumes another var preserves static styling', function() {
+    var d = document.createElement('x-var-produce-via-consume');
+    document.body.appendChild(d);
+    CustomElements.takeRecords();
+    assertComputed(d, '10px');
+  });
+
+  test('producing a var that consumes results in static and not dynamic stylesheet', function() {
+    var d = document.createElement('x-var-produce-via-consume');
+    document.body.appendChild(d);
+    CustomElements.takeRecords();
+    var styleRoot = d.shadowRoot ? d.shadowRoot : document.head;
+    var staticStyle = styleRoot.querySelector('style[scope=x-var-produce-via-consume]');
+    assert.ok(staticStyle);
+    assert.match(staticStyle.textContent, /display/, 'static style does not contain style content');
+    assert.equal(styleRoot.querySelectorAll('style[scope~=x-var-produce-via-consume]').length, 1);
+  });
+
   // TODO(sorvell): fix for #1761 was reverted; include test once this issue is addressed
-  // test('mixin values can be overridden by subsequent concrete properties', function() {
-  //   assertComputed(styled.$.override, '19px');
-  // });
+  test('mixin values can be overridden by subsequent concrete properties', function() {
+    assertComputed(styled.$.override, '19px');
+  });
 });
 
 </script>

test/unit/styling-cross-scope-var.html

@@ -599,6 +599,25 @@
   </script>
 </dom-module>
 
+<dom-module id="x-var-produce-via-consume">
+  <template>
+    <style>
+      :host {
+        display: block;
+        border: 10px solid orange;
+        --foo: var(--bar);
+      }
+    </style>
+        </template>
+  <script>
+  HTMLImports.whenReady(function() {
+    Polymer({
+      is: 'x-var-produce-via-consume'
+    });
+  });
+  </script>
+</dom-module>
+
 <script>
   suite('scoped-styling-var', function() {
 
@@ -864,6 +883,24 @@
     assertComputed(styled.$.overridesConcrete, '4px');
   });
 
+  test('producing a var that consumes another var preserves static styling', function() {
+    var d = document.createElement('x-var-produce-via-consume');
+    document.body.appendChild(d);
+    CustomElements.takeRecords();
+    assertComputed(d, '10px');
+  });
+
+  test('producing a var that consumes results in static and not dynamic stylesheet', function() {
+    var d = document.createElement('x-var-produce-via-consume');
+    document.body.appendChild(d);
+    CustomElements.takeRecords();
+    var styleRoot = d.shadowRoot ? d.shadowRoot : document.head;
+    var staticStyle = styleRoot.querySelector('style[scope=x-var-produce-via-consume]');
+    assert.ok(staticStyle);
+    assert.match(staticStyle.textContent, /display/, 'static style does not contain style content');
+    assert.equal(styleRoot.querySelectorAll('style[scope~=x-var-produce-via-consume]').length, 1);
+  });
+
 });
 
 </script>