Merge pull request #3413 from Polymer/style-fixes

Includes changes from a few style related PRs and adds more tests

src/lib/style-properties.html

@@ -82,23 +82,12 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
 
       // returns cssText of properties that consume variables/mixins
       collectCssText: function(rule) {
-        var customCssText = '';
         var cssText = rule.parsedCssText;
         // NOTE: we support consumption inside mixin assignment
         // but not production, so strip out {...}
         cssText = cssText.replace(this.rx.BRACKETED, '')
           .replace(this.rx.VAR_ASSIGN, '');
-        var parts = cssText.split(';');
+        return cssText;
-        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;
       },
 
       collectPropertiesInCssText: function(cssText, props) {
@@ -360,7 +349,12 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
 
       // Strategy: x scope shim a selector e.g. to scope `.x-foo-42` (via classes):
       // non-host selector: .a.x-foo -> .x-foo-42 .a.x-foo
-      // host selector: x-foo.wide -> x-foo.x-foo-42.wide
+      // host selector: x-foo.wide -> .x-foo-42.wide
+      // note: we use only the scope class (.x-foo-42) and not the hostSelector
+      // (x-foo) to scope :host rules; this helps make property host rules
+      // have low specificity. They are overrideable by class selectors but,
+      // unfortunately, not by type selectors (e.g. overriding via 
+      // `.special` is ok, but not by `x-foo`).
       _scopeSelector: function(rule, hostRx, hostSelector, viaAttr, scopeId) {
         rule.transformedSelector = rule.transformedSelector || rule.selector;
         var selector = rule.transformedSelector;
@@ -370,7 +364,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
         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(',');

src/lib/style-transformer.html

@@ -253,7 +253,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
     var SCOPE_DOC_SELECTOR = ':not([' + SCOPE_NAME + '])' +
       ':not(.' + SCOPE_NAME + ')';
     var COMPLEX_SELECTOR_SEP = ',';
-    var SIMPLE_SELECTOR_SEP = /(^|[\s>+~]+)([^\s>+~]+)/g;
+    var SIMPLE_SELECTOR_SEP = /(^|[\s>+~]+)((?:\[.+?\]|[^\s>+~=\[])+)/g;
     var HOST = ':host';
     var ROOT = ':root';
     // NOTE: this supports 1 nested () pair for things like

src/standard/styling.html

@@ -42,8 +42,19 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
         }
         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);
+          // 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) {
+            // NOTE: IE has css style ordering issues unless there's at least a
+            // space in the stylesheet.
+            cssText = needsStatic ? cssText : ' ';
             var style = styleUtil.applyCss(cssText, this.is,
               nativeShadow ? this._template.content : null);
             // keep track of style when in document scope (polyfill) so we can

src/standard/x-styling.html

@@ -18,6 +18,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
     var serializeValueToAttribute = Polymer.Base.serializeValueToAttribute;
 
     var propertyUtils = Polymer.StyleProperties;
+    var styleUtil = Polymer.StyleUtil;
     var styleTransformer = Polymer.StyleTransformer;
     var styleDefaults = Polymer.StyleDefaults;
 
@@ -25,6 +26,22 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
 
     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

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

@@ -14,6 +14,11 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
   <script src="../../../webcomponentsjs/webcomponents-lite.js"></script>
   <script src="../../../web-component-tester/browser.js"></script>
   <link rel="import" href="../../polymer.html">
+  <style>
+  .variable-override {
+    border-top-width: 10px;
+  }
+</style>
 </head>
 <body>
 
@@ -533,12 +538,6 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
   </dom-module>
 
 
-<style>
-  .variable-override {
-    border-top-width: 10px;
-  }
-</style>
-
 <dom-module id="x-variable-override">
   <template>
     <style>
@@ -558,11 +557,53 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
   </script>
 </dom-module>
 
+<dom-module id="x-variable-consumer">
+  <template>
+    <style>
+      :host(.foo) {
+        border: var(--consumer);
+      }
+    </style>
+    test
+  </template>
+  <script>
+    HTMLImports.whenReady(function() {
+      Polymer({
+        is: 'x-variable-consumer'
+      });
+    });
+  </script>
+</dom-module>
+
+<dom-module id="x-host-variable-consumer">
+  <template>
+    <style>
+      :host {
+        --consumer: 2px solid orange;
+      }
+
+      .foo {
+        display: block;
+        /* should override */
+        border: 10px solid black;
+      }
+    </style>
+    <x-variable-consumer id="consumer" class="foo"></x-variable-consumer>
+  </template>
+  <script>
+    HTMLImports.whenReady(function() {
+      Polymer({
+        is: 'x-host-variable-consumer'
+      });
+    });
+  </script>
+</dom-module>
+
 <script>
   suite('scoped-styling-var', function() {
 
   function assertComputed(element, value, pseudo, name) {
-    var name = name || 'border-top-width';
+    name = name || 'border-top-width';
     var computed = element.getComputedStyleValue && !pseudo ?
       element.getComputedStyleValue(name) :
       getComputedStyle(element, pseudo)[name];
@@ -800,21 +841,28 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
   });
 
   test('variable with parenthesis', function() {
-    assertComputed(styled.$.parenthesis, 'url("http://placehold.it/400x300")', false, 'background-image');
+    var url = getComputedStyle(styled.$.parenthesis).backgroundImage.replace(/['"]/g, '');
+    assert.equal('url(http://placehold.it/400x300)', url);
   });
 
-  // skip for now, until #3326 is fixed
+  test('custom style class overrides css variable', function() {
-  test.skip('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');
   });
 
-  // TODO(sorvell): fix for #1761 was reverted; include test once this issue is addressed
+  test('class selector overrides :host(class) property on element with only dynamic styles', function() {
-  // test('var values can be overridden by subsequent concrete properties', function() {
+    var d = document.createElement('x-host-variable-consumer');
-  //   assertComputed(styled.$.overridesConcrete, '4px');
+    document.body.appendChild(d);
-  // });
+    CustomElements.takeRecords();
+    assertComputed(d.$.consumer, '10px');
+  });
+
+  test('var values can be overridden by subsequent concrete properties', function() {
+    assertComputed(styled.$.overridesConcrete, '4px');
+  });
 
 });
 

test/unit/styling-remote.html

@@ -199,13 +199,23 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
       });
 
       test('styles shimmed in registration order', function() {
-        var s$ = document.head.querySelectorAll('style[scope]');
+        var regList = Polymer.telemetry.registrations.map(function(reg) {
-        var expected = ['x-order', 'x-child2', 'x-styled', 'x-styled-0', 'x-button', 'x-dynamic-scope'];
+          return reg.is;
-        var actual = [];
+        });
-        for (var i=0; i<s$.length; i++) {
+        function regIndex(styleScope) {
-          actual.push(s$[i].getAttribute('scope'));
+          for (var i=0, r; (r=regList[i]); i++) {
+            if (styleScope.match(new RegExp(r + '\\-?\\d*$'))) {
+              return i;
+            }
+          }
+        }
+        var s$ = document.head.querySelectorAll('style[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/unit/styling-scoped-elements.html

@@ -506,6 +506,34 @@
   });
 </script>
 
+<dom-module id="x-attr-selector">
+  <template>
+    <style>
+      #foo1 ~ #bar1 {
+        border: 2px solid red;
+      }
+
+      #foo1 ~ #bar1 ~ #foo2[attr~=foo2] ~ #bar2[attr~=bar2]  {
+        border: 4px solid red;
+      }
+
+      #foo1 ~ #bar1 ~ #foo2[attr~=foo2] ~ #bar2[attr~=bar2] ~ #foo3[attr~=foo3][a~=a] ~ #bar3[attr~=bar3][a~=a] {
+        border: 6px solid red;
+      }
+    </style>
+    <div id="foo1"></div>
+    <div id="bar1">bar1</div>
+    <div id="foo2" attr="foo2"></div>
+    <div id="bar2" attr="bar2">bar2</div>
+    <div id="foo3" attr="foo3" a="a"></div>
+    <div id="bar3" attr="bar3" a="a">bar3</div>
+   
+  </template>
+  <script>
+    Polymer({is: 'x-attr-selector'});
+  </script>
+</dom-module>
+
 <script>
   Polymer({
     is: 'x-scope-no-class',

test/unit/styling-scoped.html

@@ -240,6 +240,14 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
     document.body.removeChild(el);
   });
 
+  test('attribute inclusive selector and general sibling selectors', function() {
+    var e = document.createElement('x-attr-selector');
+    document.body.appendChild(e);
+    assertComputed(e.$.bar1, '2px');
+    assertComputed(e.$.bar2, '4px');
+    assertComputed(e.$.bar3, '6px');
+  });
+
     suite('scoped-styling-shady-only', function() {
 
       suiteSetup(function() {
@@ -253,17 +261,23 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
       });
 
       test('styles shimmed in registration order', function() {
-        var s$ = document.head.querySelectorAll('style[scope]');
+        var regList = Polymer.telemetry.registrations.map(function(reg) {
-        var expected = ['x-keyframes', 'x-keyframes-1', 'x-keyframes-0', 'x-gchild', 'x-child2',
+          return reg.is;
-          'x-styled', 'x-button', 'x-mixed-case', 'x-mixed-case-button',
+        });
-          'x-dynamic-scope', 'x-dynamic-template', 'x-dynamic-svg',
+        function regIndex(styleScope) {
-          'x-specificity', 'x-overriding', 'x-overriding-0',
+          for (var i=0, r; (r=regList[i]); i++) {
-          'x-specificity-parent-0', 'x-specificity-nested-0'];
+            if (styleScope.match(new RegExp(r + '\\-?\\d*$'))) {
-        var actual = [];
+              return i;
-        for (var i=0; i<s$.length; i++) {
+            }
-          actual.push(s$[i].getAttribute('scope'));
+          }
+        }
+        var s$ = document.head.querySelectorAll('style[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() {
@@ -311,9 +325,18 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
         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('x-scope with no class attribute', function () {
         assert.equal(document.querySelector('x-scope-no-class > div').className, 'style-scope x-scope-no-class');
-      })
+      });
+      
     });
 
     test('svg classes are dynamically scoped correctly', function() {
@@ -326,6 +349,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
       computed = getComputedStyle(circle);
       assert.equal(computed['fill-opacity'], '0.5');
     });
+
 });
 
 </script>