From fd5778470551f677c2aa5827398681abb1994a88 Mon Sep 17 00:00:00 2001
From: Nazar Mokrynskyi <nazar@mokrynskyi.com>
Date: Thu, 29 Oct 2015 03:22:04 +0100
Subject: [PATCH] Fix for incorrect CSS selectors specificity as reported in
 #2531 Fix for overriding mixin properties, fixes #1873 Added awareness from
 `@apply()` position among other rules so that it is preserved after CSS
 variables/mixing substitution. `Polymer.StyleUtil.clearStyleRules()` method
 removed as it is not used anywhere. Some unused variables removed. Typos,
 unused variables and unnecessary escaping in regexps corrected. Tests added.

---
 src/lib/css-parse.html                 |  12 +--
 src/lib/style-properties.html          |  14 ++--
 src/lib/style-transformer.html         |  10 +--
 src/lib/style-util.html                |   5 --
 src/standard/styling.html              |   3 +-
 test/unit/css-parse.html               |   3 +-
 test/unit/styling-remote.html          |   4 +-
 test/unit/styling-scoped-elements.html | 100 ++++++++++++++++++++++++-
 test/unit/styling-scoped.html          |  34 +++++++--
 9 files changed, 148 insertions(+), 37 deletions(-)

diff --git a/src/lib/css-parse.html b/src/lib/css-parse.html
index 2199fdb778..c3454b4d21 100644
--- a/src/lib/css-parse.html
+++ b/src/lib/css-parse.html
@@ -11,11 +11,11 @@
 
 /*
   Extremely simple css parser. Intended to be not more than what we need
-  and definitely not necessarly correct =).
+  and definitely not necessarily correct =).
 */
 Polymer.CssParse = (function() {
 
-  var api = {
+  return {
     // given a string of css, return a simple rule tree
     parse: function(text) {
       text = this._clean(text);
@@ -31,7 +31,7 @@
     _lex: function(text) {
       var root = {start: 0, end: text.length};
       var n = root;
-      for (var i=0, s=0, l=text.length; i < l; i++) {
+      for (var i=0, l=text.length; i < l; i++) {
         switch (text[i]) {
           case this.OPEN_BRACE:
             //console.group(i);
@@ -123,7 +123,7 @@
           }
         }
       }
-      // emit rule iff there is cssText
+      // emit rule if there is cssText
       if (cssText) {
         if (node.selector) {
           text += node.selector + ' ' + this.OPEN_BRACE + '\n';
@@ -185,10 +185,6 @@
 
   };
 
-
-  // exports
-  return api;
-
 })();
 
 </script>
diff --git a/src/lib/style-properties.html b/src/lib/style-properties.html
index b007551bd5..df455af892 100644
--- a/src/lib/style-properties.html
+++ b/src/lib/style-properties.html
@@ -87,9 +87,7 @@
         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)) {
-            customCssText += p + ';\n';
-          }
+          customCssText += p + ';\n';
         }
         return customCssText;
       },
@@ -181,7 +179,7 @@
         rule.cssText = output;
       },
 
-      // Test if the rules in these styles matche the given `element` and if so,
+      // Test if the rules in these styles matches the given `element` and if so,
       // collect any custom properties into `props`.
       propertyDataFromStyles: function(styles, element) {
         var props = {}, self = this;
@@ -208,7 +206,7 @@
         return {properties: props, key: o};
       },
 
-      // Test if a rule matches scope crteria (* or :root) and if so,
+      // Test if a rule matches scope criteria (* or :root) and if so,
       // collect any custom properties into `props`.
       scopePropertiesFromStyles: function(styles) {
         if (!styles._scopeStyleProperties) {
@@ -218,7 +216,7 @@
         return styles._scopeStyleProperties;
       },
 
-      // Test if a rule matches host crteria (:host) and if so,
+      // Test if a rule matches host criteria (:host) and if so,
       // collect any custom properties into `props`.
       //
       // TODO(sorvell): this should change to collecting properties from any
@@ -319,7 +317,7 @@
           // otherwise, if we have css to apply, do so
           } else if (cssText) {
             // apply css after the scope style of the element to help with
-            // style predence rules.
+            // style precedence rules.
             style = styleUtil.applyCss(cssText, selector,
               nativeShadow ? element.root : null, element._scopeStyle);
           }
@@ -356,7 +354,7 @@
         // var(--a)
         // var(--a, --b)
         // var(--a, fallback-literal)
-        // var(--a, fallback-literal(with-one-nested-parens))
+        // var(--a, fallback-literal(with-one-nested-parentheses))
         VAR_MATCH: /(^|\W+)var\([\s]*([^,)]*)[\s]*,?[\s]*((?:[^,)]*)|(?:[^;]*\([^;)]*\)))[\s]*?\)/gi,
         VAR_CAPTURE: /\([\s]*(--[^,\s)]*)(?:,[\s]*(--[^,\s)]*))?(?:\)|,)/gi,
         IS_VAR: /^--/,
diff --git a/src/lib/style-transformer.html b/src/lib/style-transformer.html
index 27afb0fdb3..5d4eead472 100644
--- a/src/lib/style-transformer.html
+++ b/src/lib/style-transformer.html
@@ -34,7 +34,7 @@
        cannot otherwise be scoped:
        e.g. :host ::content > .bar -> x-foo > .bar
 
-     * ::shadow, /deep/: processed simimlar to ::content
+     * ::shadow, /deep/: processed similar to ::content
 
      * :host-context(...): scopeName..., ... scopeName
 
@@ -97,7 +97,7 @@
       elementStyles: function(element, callback) {
         var styles = element._styles;
         var cssText = '';
-        for (var i=0, l=styles.length, s, text; (i<l) && (s=styles[i]); i++) {
+        for (var i=0, l=styles.length, s; (i<l) && (s=styles[i]); i++) {
           var rules = styleUtil.rulesForStyle(s);
           cssText += nativeShadow ?
             styleUtil.toCssText(rules, callback) :
@@ -152,7 +152,7 @@
           p$[i] = transformer.call(this, p, scope, hostScope);
         }
         // NOTE: save transformedSelector for subsequent matching of elements
-        // agsinst selectors (e.g. when calculating style properties)
+        // against selectors (e.g. when calculating style properties)
         rule.selector = rule.transformedSelector =
           p$.join(COMPLEX_SELECTOR_SEP);
       },
@@ -258,9 +258,9 @@
     // parsing which seems like overkill
     var HOST_PAREN = /(\:host)(?:\(((?:\([^)(]*\)|[^)(]*)+?)\))/g;
     var HOST_CONTEXT = ':host-context';
-    var HOST_CONTEXT_PAREN = /(.*)(?:\:host-context)(?:\(((?:\([^)(]*\)|[^)(]*)+?)\))(.*)/;
+    var HOST_CONTEXT_PAREN = /(.*)(?::host-context)(?:\(((?:\([^)(]*\)|[^)(]*)+?)\))(.*)/;
     var CONTENT = '::content';
-    var SCOPE_JUMP = /\:\:content|\:\:shadow|\/deep\//;
+    var SCOPE_JUMP = /::content|::shadow|\/deep\//;
     var CSS_CLASS_PREFIX = '.';
     var CSS_ATTR_PREFIX = '[' + SCOPE_NAME + '~=';
     var CSS_ATTR_SUFFIX = ']';
diff --git a/src/lib/style-util.html b/src/lib/style-util.html
index afc0c5ab1d..c8d2ec868d 100644
--- a/src/lib/style-util.html
+++ b/src/lib/style-util.html
@@ -44,15 +44,10 @@
         return style.__cssRules;
       },
 
-      clearStyleRules: function(style) {
-        style.__cssRules = null;
-      },
-
       forEachStyleRule: function(node, callback) {
         if (!node) {
           return;
         }
-        var s = node.parsedSelector;
         var skipRules = false;
         if (node.type === this.ruleTypes.STYLE_RULE) {
           callback(node);
diff --git a/src/standard/styling.html b/src/standard/styling.html
index f4fa704dbe..dce7682c4b 100644
--- a/src/standard/styling.html
+++ b/src/standard/styling.html
@@ -79,6 +79,7 @@
           style.textContent = cssText;
           // extends!!
           if (styleExtends.hasExtends(style.textContent)) {
+            // TODO(sorvell): variable is not used, should it update `style.textContent`?
             cssText = styleExtends.transform(style);
           }
           styles.push(style);
@@ -109,7 +110,7 @@
 
       /**
        * Apply style scoping to the specified `container` and all its
-       * descendants. If `shoudlObserve` is true, changes to the container are
+       * descendants. If `shouldObserve` is true, changes to the container are
        * monitored via mutation observer and scoping is applied.
        *
        * This method is useful for ensuring proper local DOM CSS scoping
diff --git a/test/unit/css-parse.html b/test/unit/css-parse.html
index 0defe0af71..c4ed25d277 100644
--- a/test/unit/css-parse.html
+++ b/test/unit/css-parse.html
@@ -166,7 +166,8 @@
       assert.equal(t.rules[3].selector, '.\\0c3333d-model');
       assert.equal(t.rules[4].selector, '.\\d33333d-model');
       assert.equal(t.rules[5].selector, '.\\e33333d-model');
-    });
+    });
+
     test('multiple consequent spaces in CSS selector', function() {
       var s4 = document.querySelector('#multiple-spaces');
       var t = css.parse(s4.textContent);
diff --git a/test/unit/styling-remote.html b/test/unit/styling-remote.html
index 348fcdab71..4fd71c0df1 100644
--- a/test/unit/styling-remote.html
+++ b/test/unit/styling-remote.html
@@ -57,7 +57,7 @@
   test(':host, :host(...)', function() {
     assertComputed(styled, '1px');
     assertComputed(styledWide, '2px');
-    
+
   });
 
   test('scoped selectors, simple and complex', function() {
@@ -209,7 +209,7 @@
       });
     });
   }
-  
+
 });
 
 </script>
diff --git a/test/unit/styling-scoped-elements.html b/test/unit/styling-scoped-elements.html
index 769f7ade21..673d6c75ce 100644
--- a/test/unit/styling-scoped-elements.html
+++ b/test/unit/styling-scoped-elements.html
@@ -286,7 +286,6 @@
 })();
 </script>
 
-
 <template id="dynamic-style-template">
   <style>
     :host {
@@ -308,4 +307,101 @@
     }
   });
 })();
-</script>
\ No newline at end of file
+</script>
+
+<dom-module id="x-specificity">
+  <template>
+    <style>
+      :host {
+        border-top: 1px solid red;
+      }
+      :host(.bar) {
+        border-top: 2px solid red;
+      }
+    </style>
+    <content></content>
+  </template>
+  <script>
+    Polymer({is: 'x-specificity'});
+  </script>
+</dom-module>
+
+<style is="custom-style">
+  :root {
+    --x-specificity-parent : {
+      border: 10px solid blue;
+    };
+    --x-specificity-nested : {
+      border: 3px solid red;
+    };
+  }
+</style>
+
+<dom-module id="x-specificity-parent">
+  <template>
+    <style>
+      /* TODO remove `:host` when https://github.com/Polymer/polymer/pull/2419 merged */
+      :host ::content > :not(template) {
+        @apply(--x-specificity-parent);
+      }
+    </style>
+    <content></content>
+  </template>
+  <script>
+    Polymer({is: 'x-specificity-parent', extends: 'div'});
+  </script>
+</dom-module>
+
+<dom-module id="x-specificity-nested">
+  <template>
+    <style>
+      :host {
+        @apply(--x-specificity-nested);
+      }
+    </style>
+  </template>
+  <script>
+    Polymer({is: 'x-specificity-nested', extends: 'div'});
+  </script>
+</dom-module>
+
+<style is="custom-style">
+  :root {
+    --x-overriding : {
+      border-top: 1px solid red;
+    }
+  }
+</style>
+
+<dom-module id="x-overriding">
+  <template>
+    <style>
+      .red {
+        @apply(--x-overriding);
+      }
+      .green {
+        @apply(--x-overriding);
+        border-top: 2px solid green;
+      }
+      .red-2 {
+        border-top: 2px solid green;
+        @apply(--x-overriding);
+      }
+      .blue {
+        @apply(--x-overriding);
+        border-top: 3px solid blue;
+      }
+    </style>
+
+    <div class="red">red</div>
+    <div class="green">green</div>
+    <div class="red-2">green-2</div>
+    <div class="blue">blue</div>
+  </template>
+</dom-module>
+
+<script>
+  Polymer({
+    is: 'x-overriding'
+  });
+</script>
diff --git a/test/unit/styling-scoped.html b/test/unit/styling-scoped.html
index e445fa09d6..6f749beea2 100644
--- a/test/unit/styling-scoped.html
+++ b/test/unit/styling-scoped.html
@@ -43,6 +43,13 @@
     <span id="dom-bind-dynamic" class$="[[dynamic]]">[[dynamic]]</span>
   </template>
 
+  <x-specificity></x-specificity>
+  <x-specificity class="bar"></x-specificity>
+  <div is="x-specificity-parent">
+    <div is="x-specificity-nested"></div>
+  </div>
+  <x-overriding></x-overriding>
+
 <script>
   suite('scoped-styling', function() {
 
@@ -61,7 +68,7 @@
   test(':host, :host(...)', function() {
     assertComputed(styled, '1px');
     assertComputed(styledWide, '2px');
-    
+
   });
 
   test(':host-context(...)', function() {
@@ -210,8 +217,9 @@
 
       test('styles shimmed in registration order', function() {
         var s$ = document.head.querySelectorAll('style[scope]');
-        var expected = ['x-gchild', 'x-child2', 'x-styled', 'x-button', 
-        'x-mixed-case', 'x-mixed-case-button', 'x-dynamic-scope', 'x-dynamic-template'];
+        var expected = ['x-gchild', 'x-child2', 'x-styled', 'x-button', 'x-mixed-case',
+          'x-mixed-case-button', 'x-dynamic-scope', 'x-dynamic-template', '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'));
@@ -252,10 +260,26 @@
         x = document.createElement('button', 'x-mixed-case-button');
         document.body.appendChild(x);
         assertComputed(x, '14px');
-       
+      });
+
+      test('specificity of :host selector with class', function() {
+        assertComputed(document.querySelector('x-specificity'), '1px');
+        assertComputed(document.querySelector('x-specificity.bar'), '2px');
+      });
+
+      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');
       });
     });
-  
+
 });
 
 </script>