Merge pull request #2169 from Polymer/fix-2113

Fixes #2113: ensures custom-style rules that use @apply combined with defining properties apply correctly

src/lib/css-parse.html

@@ -24,7 +24,7 @@
 
     // remove stuff we don't care about that may hinder parsing
     _clean: function (cssText) {
-      return cssText.replace(rx.comments, '').replace(rx.port, '');
+      return cssText.replace(this._rx.comments, '').replace(this._rx.port, '');
     },
 
     // super simple {...} lexer that returns a node tree
@@ -64,16 +64,16 @@
         // helps with mixin syntax
         t = t.substring(t.lastIndexOf(';')+1);
         var s = node.parsedSelector = node.selector = t.trim();
-        node.atRule = (s.indexOf(AT_START) === 0);
+        node.atRule = (s.indexOf(this.AT_START) === 0);
         // note, support a subset of rule types...
         if (node.atRule) {
-          if (s.indexOf(MEDIA_START) === 0) {
+          if (s.indexOf(this.MEDIA_START) === 0) {
             node.type = this.types.MEDIA_RULE;
-          } else if (s.match(rx.keyframesRule)) {
+          } else if (s.match(this._rx.keyframesRule)) {
             node.type = this.types.KEYFRAMES_RULE;
           }
         } else {
-          if (s.indexOf(VAR_START) === 0) {
+          if (s.indexOf(this.VAR_START) === 0) {
             node.type = this.types.MIXIN_RULE;
           } else {
             node.type = this.types.STYLE_RULE;
@@ -96,13 +96,13 @@
       var cssText = '';
       if (node.cssText || node.rules) {
         var r$ = node.rules;
-        if (r$ && (preserveProperties || !hasMixinRules(r$))) {
+        if (r$ && (preserveProperties || !this._hasMixinRules(r$))) {
           for (var i=0, l=r$.length, r; (i<l) && (r=r$[i]); i++) {
             cssText = this.stringify(r, preserveProperties, cssText);
           }  
         } else {
           cssText = preserveProperties ? node.cssText : 
-            removeCustomProps(node.cssText);  
+            this.removeCustomProps(node.cssText);  
           cssText = cssText.trim();
           if (cssText) {
             cssText = '  ' + cssText + '\n';
@@ -122,6 +122,27 @@
       return text;
     },
 
+    _hasMixinRules: function(rules) {
+      return (rules[0].selector.indexOf(this.VAR_START) >= 0);
+    },
+
+    removeCustomProps: function(cssText) {
+      cssText = this.removeCustomPropAssignment(cssText);
+      return this.removeCustomPropApply(cssText);
+    },
+
+    removeCustomPropAssignment: function(cssText) {
+      return cssText
+        .replace(this._rx.customProp, '')
+        .replace(this._rx.mixinProp, '');
+    },
+
+    removeCustomPropApply: function(cssText) {
+      return cssText            
+        .replace(this._rx.mixinApply, '')
+        .replace(this._rx.varApply, '');
+    },
+
     types: {
       STYLE_RULE: 1,
       KEYFRAMES_RULE: 7,
@@ -130,37 +151,26 @@
     },
 
     OPEN_BRACE: '{',
-    CLOSE_BRACE: '}'
+    CLOSE_BRACE: '}',
+
+    // helper regexp's
+    _rx: {
+      comments: /\/\*[^*]*\*+([^/*][^*]*\*+)*\//gim,
+      port: /@import[^;]*;/gim,
+      customProp: /(?:^|[\s;])--[^;{]*?:[^{};]*?(?:[;\n]|$)/gim,
+      mixinProp: /(?:^|[\s;])--[^;{]*?:[^{;]*?{[^}]*?}(?:[;\n]|$)?/gim,
+      mixinApply: /@apply[\s]*\([^)]*?\)[\s]*(?:[;\n]|$)?/gim,
+      varApply: /[^;:]*?:[^;]*var[^;]*(?:[;\n]|$)?/gim,
+      keyframesRule: /^@[^\s]*keyframes/,
+    },
 
-  };
+    VAR_START: '--',
+    MEDIA_START: '@media',
+    AT_START: '@'
 
-  function hasMixinRules(rules) {
-    return (rules[0].selector.indexOf(VAR_START) >= 0);
-  }
-
-  function removeCustomProps(cssText) {
-    return cssText
-      .replace(rx.customProp, '')
-      .replace(rx.mixinProp, '')
-      .replace(rx.mixinApply, '')
-      .replace(rx.varApply, '');
-  }
-
-  var VAR_START = '--';
-  var MEDIA_START = '@media';
-  var AT_START = '@';
-
-  // helper regexp's
-  var rx = {
-    comments: /\/\*[^*]*\*+([^/*][^*]*\*+)*\//gim,
-    port: /@import[^;]*;/gim,
-    customProp: /(?:^|[\s;])--[^;{]*?:[^{};]*?(?:[;\n]|$)/gim,
-    mixinProp: /(?:^|[\s;])--[^;{]*?:[^{;]*?{[^}]*?}(?:[;\n]|$)?/gim,
-    mixinApply: /@apply[\s]*\([^)]*?\)[\s]*(?:[;\n]|$)?/gim,
-    varApply: /[^;:]*?:[^;]*var[^;]*(?:[;\n]|$)?/gim,
-    keyframesRule: /^@[^\s]*keyframes/,
   };
 
+
   // exports 
   return api;
 

src/lib/custom-style.html

@@ -71,6 +71,7 @@
   var nativeShadow = Polymer.Settings.useNativeShadow;
   var propertyUtils = Polymer.StyleProperties;
   var styleUtil = Polymer.StyleUtil;
+  var cssParse = Polymer.CssParse;
   var styleDefaults = Polymer.StyleDefaults;
   var styleTransformer = Polymer.StyleTransformer;
 
@@ -125,9 +126,15 @@
         function(rule) {
           var css = rule.cssText = rule.parsedCssText;
           if (rule.propertyInfo && rule.propertyInfo.cssText) {
-            // TODO(sorvell): factor better
-            // remove property assignments so next function isn't confused
-            css = css.replace(propertyUtils.rx.VAR_ASSIGN, '');
+            // remove property assignments
+            // so next function isn't confused
+            // NOTE: we have 3 categories of css:
+            // (1) normal properties,
+            // (2) custom property assignments (--foo: red;), 
+            // (3) custom property usage: border: var(--foo); @apply(--foo);
+            // In elements, 1 and 3 are separated for efficiency; here they 
+            // are not and this makes this case unique.
+            css = cssParse.removeCustomPropAssignment(css);
             // replace with reified properties, scenario is same as mixin
             rule.cssText = propertyUtils.valueForProperties(css, props);
           }

src/lib/style-properties.html

@@ -147,22 +147,24 @@
       // note: we do not yet support mixin within mixin
       valueForProperties: function(property, props) {
         var parts = property.split(';');
-        for (var i=0, p, m; (i<parts.length) && (p=parts[i]); i++) {
-          m = p.match(this.rx.MIXIN_MATCH);
-          if (m) {
-            p = this.valueForProperty(props[m[1]], props);
-          } else {
-            var pp = p.split(':');
-            if (pp[1]) {
-              pp[1] = pp[1].trim();
-              pp[1] = this.valueForProperty(pp[1], props) || pp[1];
+        for (var i=0, p, m; i<parts.length; i++) {
+          if (p = parts[i]) {
+            m = p.match(this.rx.MIXIN_MATCH);
+            if (m) {
+              p = this.valueForProperty(props[m[1]], props);
+            } else {
+              var pp = p.split(':');
+              if (pp[1]) {
+                pp[1] = pp[1].trim();
+                pp[1] = this.valueForProperty(pp[1], props) || pp[1];
+              }
+              p = pp.join(':');
             }
-            p = pp.join(':');
+            parts[i] = (p && p.lastIndexOf(';') === p.length - 1) ? 
+              // strip trailing ;
+              p.slice(0, -1) :
+              p || '';
           }
-          parts[i] = (p && p.lastIndexOf(';') === p.length - 1) ? 
-            // strip trailing ;
-            p.slice(0, -1) :
-            p || '';
         }
         return parts.join(';');
       },

test/unit/custom-style.html

@@ -41,6 +41,12 @@
       display: block;
     }
 
+    :root {
+      --bag2: {
+        border: 12px solid beige;
+      };
+    }
+
 
     :root {
 
@@ -59,6 +65,7 @@
       --special: var(--primary);
 
       --after: 11px solid orange;
+      @apply(--bag2);
     }
 
     x-foo {
@@ -70,7 +77,7 @@
   </style>
   <style is="custom-style">
     .bag {
-      @apply(--bag);
+      ;@apply(--bag);
     }
 
     .italic {
@@ -242,6 +249,10 @@
         assertComputed(e, '11px', null, '::after');
       });
 
+      test('@apply in :root rule that also defines properties', function() {
+        assertComputed(document.body, '12px');
+      });
+
     });