Skip to content

Commit

Permalink
Fix for incorrect CSS selectors specificity as reported in #2531
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@nazar-pc
nazar-pc committed Nov 13, 2015
1 parent 98acb3a commit fd57784
Show file tree
Hide file tree
Showing 9 changed files with 148 additions and 37 deletions.
12 changes: 4 additions & 8 deletions src/lib/css-parse.html
View file
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -185,10 +185,6 @@

};


// exports
return api;

})();

</script>
14 changes: 6 additions & 8 deletions src/lib/style-properties.html
View file
Expand Up @@ -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;
},
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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: /^--/,
Expand Down
10 changes: 5 additions & 5 deletions src/lib/style-transformer.html
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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) :
Expand Down Expand Up @@ -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);
},
Expand Down Expand Up @@ -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 = ']';
Expand Down
5 changes: 0 additions & 5 deletions src/lib/style-util.html
View file
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/standard/styling.html
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion test/unit/css-parse.html
View file
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions test/unit/styling-remote.html
View file
Expand Up @@ -57,7 +57,7 @@
test(':host, :host(...)', function() {
assertComputed(styled, '1px');
assertComputed(styledWide, '2px');

});

test('scoped selectors, simple and complex', function() {
Expand Down Expand Up @@ -209,7 +209,7 @@
});
});
}

});

</script>
Expand Down
100 changes: 98 additions & 2 deletions test/unit/styling-scoped-elements.html
View file
Expand Up @@ -286,7 +286,6 @@
})();
</script>


<template id="dynamic-style-template">
<style>
:host {
Expand All @@ -308,4 +307,101 @@
}
});
})();
</script>
</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>
34 changes: 29 additions & 5 deletions test/unit/styling-scoped.html
View file
Expand Up @@ -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() {

Expand All @@ -61,7 +68,7 @@
test(':host, :host(...)', function() {
assertComputed(styled, '1px');
assertComputed(styledWide, '2px');

});

test(':host-context(...)', function() {
Expand Down Expand Up @@ -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'));
Expand Down Expand Up @@ -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>
Expand Down

0 comments on commit fd57784

Please sign in to comment.