Fixes #2356: issue a warning and don't throw an exception when a styl…

…e include cannot be found. Fixes #2357: include data now comes before any textContent in a style element.
Polymer · Aug 26, 2015 · a16ada1 · a16ada1
1 parent 9bb6fdc
commit a16ada1
Show file tree

Hide file tree

Showing 7 changed files with 122 additions and 31 deletions.
diff --git a/src/lib/custom-style.html b/src/lib/custom-style.html
@@ -133,31 +133,37 @@
       // used applied element from HTMLImports polyfill or this
       var e = this.__appliedElement || this;
       if (this.include) {
-        e.textContent += styleUtil.cssFromModules(this.include);
+        e.textContent = styleUtil.cssFromModules(this.include, true) + 
+          e.textContent;
       }
-      var rules = styleUtil.rulesForStyle(e);
-      styleUtil.forEachStyleRule(rules, function(rule) {
-        styleTransformer.documentRule(rule);
-      });
+      if (e.textContent) {
+        styleUtil.forEachStyleRule(styleUtil.rulesForStyle(e), function(rule) {
+          styleTransformer.documentRule(rule);
+        });
+        this._applyCustomProperties(e);
+      }
+    },
+
+    _applyCustomProperties: function(element) {
       this._computeStyleProperties();
       var props = this._styleProperties;
-      e.textContent = styleUtil.toCssText(rules, function(rule) {
-          var css = rule.cssText = rule.parsedCssText;
-          if (rule.propertyInfo && rule.propertyInfo.cssText) {
-            // 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);
-          }
+      var rules = styleUtil.rulesForStyle(element);
+      element.textContent = styleUtil.toCssText(rules, function(rule) {
+        var css = rule.cssText = rule.parsedCssText;
+        if (rule.propertyInfo && rule.propertyInfo.cssText) {
+          // 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);
         }
-      );
+      });
     }
 
   });

diff --git a/src/lib/style-util.html b/src/lib/style-util.html
@@ -49,6 +49,9 @@
       },
 
       forEachStyleRule: function(node, callback) {
+        if (!node) {
+          return;
+        }
         var s = node.parsedSelector;
         var skipRules = false;
         if (node.type === this.ruleTypes.STYLE_RULE) {
@@ -82,22 +85,25 @@
         return style;
       },
 
-      cssFromModules: function(moduleIds) {
+      cssFromModules: function(moduleIds, warnIfNotFound) {
         var modules = moduleIds.trim().split(' ');
         var cssText = '';
         for (var i=0; i < modules.length; i++) {
-          cssText += this.cssFromModule(modules[i]);
+          cssText += this.cssFromModule(modules[i], warnIfNotFound);
         }
         return cssText;
       },
 
       // returns cssText of styles in a given module; also un-applies any
       // styles that apply to the document.
-      cssFromModule: function(moduleId) {
+      cssFromModule: function(moduleId, warnIfNotFound) {
         var m = Polymer.DomModule.import(moduleId);
         if (m && !m._cssText) {
           m._cssText = this._cssFromElement(m);
         }
+        if (!m && warnIfNotFound) {
+          console.warn('Could not find style data in module named', moduleId);
+        }
         return m && m._cssText || '';
       },
 
@@ -118,14 +124,14 @@
             // we don't want this, so we remove them here.
             if (e.localName === 'style') {
               var include = e.getAttribute(this.INCLUDE_ATTR);
+              // now support module refs on 'styling' elements
+              if (include) {
+                cssText += this.cssFromModules(include, true);
+              }
               // get style element applied to main doc via HTMLImports polyfill
               e = e.__appliedElement || e;
               e.parentNode.removeChild(e);
               cssText += this.resolveCss(e.textContent, element.ownerDocument);
-              // now support module refs on 'styling' elements
-              if (include) {
-                cssText += this.cssFromModules(include);
-              }
             // it's an import, assume this is a text file of css content.
             // TODO(sorvell): plan is to deprecate this way to get styles;
             // remember to add deprecation warning when this is done.

diff --git a/test/unit/custom-style-import.html b/test/unit/custom-style-import.html
@@ -8,6 +8,16 @@
   </template>
 </dom-module>
 
+<dom-module id="shared-style2">
+  <template>
+    <style>
+      .zazz {
+        border: 16px solid orange;
+      }
+    </style>
+  </template>
+</dom-module>
+
 <link rel="import" href="sub/style-import.html">
 
 <style is="custom-style" include="shared-style style-import">
@@ -19,5 +29,4 @@
 
     padding: 10px;
   }
-</style>
-
+</style>
diff --git a/test/unit/custom-style.html b/test/unit/custom-style.html
@@ -105,6 +105,12 @@
       border: var(--after);
     }
   </style>
+
+  <style is="custom-style" include="shared-style2">
+    .zazz {
+      border: 20px solid blue; 
+    }
+  </style>
 </head>
 <body>
   <div class="italic">italic</div>
@@ -274,6 +280,57 @@
         assertComputed(xFoo.$.deep, '6px');
       });
 
+      test('imperative custom style', function() {
+        var style = document.createElement('style', 'custom-style');
+        style.textContent = '.zonk { border: 13px solid tomato;}';
+        var d = document.createElement('div');
+        d.classList.add('zonk');
+        document.body.appendChild(d);
+        document.body.appendChild(style);
+        CustomElements.takeRecords();
+        assertComputed(d, '13px');
+        document.body.removeChild(d);
+        document.body.removeChild(style);
+      });
+
+      test('imperative custom style with include', function() {
+        var style = document.createElement('style', 'custom-style');
+        style.include = 'shared-style2';
+        var d = document.createElement('div');
+        d.classList.add('zazz');
+        document.body.appendChild(d);
+        document.body.appendChild(style);
+        CustomElements.takeRecords();
+        assertComputed(d, '16px');
+        document.body.removeChild(d);
+        document.body.removeChild(style);
+      });
+
+      test('imperative custom style with non-existent include', function() {
+        var s1 = document.createElement('style', 'custom-style');
+        s1.include = 'does-not-exist';
+        var style = document.createElement('style', 'custom-style');
+        style.textContent = '.ziz { border: 14px solid tomato;}';
+        var d = document.createElement('div');
+        d.classList.add('ziz');
+        document.body.appendChild(d);
+        document.body.appendChild(s1)
+        document.body.appendChild(style);
+        CustomElements.takeRecords();
+        assertComputed(d, '14px');
+        document.body.removeChild(d);
+        document.body.removeChild(s1);
+        document.body.removeChild(style);
+      });
+
+      test('include style data applied before textContent', function() {
+        var d = document.createElement('div');
+        d.classList.add('zazz');
+        document.body.appendChild(d);
+        assertComputed(d, '20px');
+        document.body.removeChild(d);
+      });
+
     });
 
 

diff --git a/test/unit/styling-remote-elements.html b/test/unit/styling-remote-elements.html
@@ -90,7 +90,11 @@
   </style>
   <template>
     <!-- style in template using module! -->
-    <style include="remote-styles"></style>
+    <style include="remote-styles">
+      #zazz {
+        border: 20px solid goldenrod;
+      }
+    </style>
     <content select=".blank"></content>
     <div id="simple">simple</div>
     <div id="complex1" class="scoped">complex1</div>
@@ -112,6 +116,7 @@
     <content></content>
     <div id="url" class="foo"></div>
     <div id="bg" class="bg"></div>
+    <div id="zazz"></div>
   </template>
 </dom-module>
 <script>

diff --git a/test/unit/styling-remote-module-sheet.html b/test/unit/styling-remote-module-sheet.html
@@ -10,6 +10,10 @@
     .scoped, [selected] {
       border: 4px solid pink;
     }
+
+    #zazz {
+      border: 6px solid tomato;
+    }
   </style>
 </template>
 </dom-module>
diff --git a/test/unit/styling-remote.html b/test/unit/styling-remote.html
@@ -185,6 +185,10 @@
     assert.include(url, 'sub/foo.png');
   });
 
+  test('include style data applied before textContent', function() {
+    assertComputed(styled.$.zazz, '20px');
+  });
+
   // weird check allows testing under HTMLImports polyfill
   if (!window.Polymer || !Polymer.Settings.useNativeShadow) {