Skip to content

Commit fd57784

Browse files
nazar-pcnazar-pc
committed
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.
1 parent 98acb3a commit fd57784

9 files changed

Lines changed: 148 additions & 37 deletions

‎src/lib/css-parse.html‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@
1111

1212
/*
1313
Extremely simple css parser. Intended to be not more than what we need
14-
and definitely not necessarly correct =).
14+
and definitely not necessarily correct =).
1515
*/
1616
Polymer.CssParse = (function() {
1717

18-
var api = {
18+
return {
1919
// given a string of css, return a simple rule tree
2020
parse: function(text) {
2121
text = this._clean(text);
@@ -31,7 +31,7 @@
3131
_lex: function(text) {
3232
var root = {start: 0, end: text.length};
3333
var n = root;
34-
for (var i=0, s=0, l=text.length; i < l; i++) {
34+
for (var i=0, l=text.length; i < l; i++) {
3535
switch (text[i]) {
3636
case this.OPEN_BRACE:
3737
//console.group(i);
@@ -123,7 +123,7 @@
123123
}
124124
}
125125
}
126-
// emit rule iff there is cssText
126+
// emit rule if there is cssText
127127
if (cssText) {
128128
if (node.selector) {
129129
text += node.selector + ' ' + this.OPEN_BRACE + '\n';
@@ -185,10 +185,6 @@
185185

186186
};
187187

188-
189-
// exports
190-
return api;
191-
192188
})();
193189

194190
</script>

‎src/lib/style-properties.html‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,7 @@
8787
var parts = cssText.split(';');
8888
for (var i=0, p; i<parts.length; i++) {
8989
p = parts[i];
90-
if (p.match(this.rx.MIXIN_MATCH) || p.match(this.rx.VAR_MATCH)) {
91-
customCssText += p + ';\n';
92-
}
90+
customCssText += p + ';\n';
9391
}
9492
return customCssText;
9593
},
@@ -181,7 +179,7 @@
181179
rule.cssText = output;
182180
},
183181

184-
// Test if the rules in these styles matche the given `element` and if so,
182+
// Test if the rules in these styles matches the given `element` and if so,
185183
// collect any custom properties into `props`.
186184
propertyDataFromStyles: function(styles, element) {
187185
var props = {}, self = this;
@@ -208,7 +206,7 @@
208206
return {properties: props, key: o};
209207
},
210208

211-
// Test if a rule matches scope crteria (* or :root) and if so,
209+
// Test if a rule matches scope criteria (* or :root) and if so,
212210
// collect any custom properties into `props`.
213211
scopePropertiesFromStyles: function(styles) {
214212
if (!styles._scopeStyleProperties) {
@@ -218,7 +216,7 @@
218216
return styles._scopeStyleProperties;
219217
},
220218

221-
// Test if a rule matches host crteria (:host) and if so,
219+
// Test if a rule matches host criteria (:host) and if so,
222220
// collect any custom properties into `props`.
223221
//
224222
// TODO(sorvell): this should change to collecting properties from any
@@ -319,7 +317,7 @@
319317
// otherwise, if we have css to apply, do so
320318
} else if (cssText) {
321319
// apply css after the scope style of the element to help with
322-
// style predence rules.
320+
// style precedence rules.
323321
style = styleUtil.applyCss(cssText, selector,
324322
nativeShadow ? element.root : null, element._scopeStyle);
325323
}
@@ -356,7 +354,7 @@
356354
// var(--a)
357355
// var(--a, --b)
358356
// var(--a, fallback-literal)
359-
// var(--a, fallback-literal(with-one-nested-parens))
357+
// var(--a, fallback-literal(with-one-nested-parentheses))
360358
VAR_MATCH: /(^|\W+)var\([\s]*([^,)]*)[\s]*,?[\s]*((?:[^,)]*)|(?:[^;]*\([^;)]*\)))[\s]*?\)/gi,
361359
VAR_CAPTURE: /\([\s]*(--[^,\s)]*)(?:,[\s]*(--[^,\s)]*))?(?:\)|,)/gi,
362360
IS_VAR: /^--/,

‎src/lib/style-transformer.html‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
cannot otherwise be scoped:
3535
e.g. :host ::content > .bar -> x-foo > .bar
3636
37-
* ::shadow, /deep/: processed simimlar to ::content
37+
* ::shadow, /deep/: processed similar to ::content
3838
3939
* :host-context(...): scopeName..., ... scopeName
4040
@@ -97,7 +97,7 @@
9797
elementStyles: function(element, callback) {
9898
var styles = element._styles;
9999
var cssText = '';
100-
for (var i=0, l=styles.length, s, text; (i<l) && (s=styles[i]); i++) {
100+
for (var i=0, l=styles.length, s; (i<l) && (s=styles[i]); i++) {
101101
var rules = styleUtil.rulesForStyle(s);
102102
cssText += nativeShadow ?
103103
styleUtil.toCssText(rules, callback) :
@@ -152,7 +152,7 @@
152152
p$[i] = transformer.call(this, p, scope, hostScope);
153153
}
154154
// NOTE: save transformedSelector for subsequent matching of elements
155-
// agsinst selectors (e.g. when calculating style properties)
155+
// against selectors (e.g. when calculating style properties)
156156
rule.selector = rule.transformedSelector =
157157
p$.join(COMPLEX_SELECTOR_SEP);
158158
},
@@ -258,9 +258,9 @@
258258
// parsing which seems like overkill
259259
var HOST_PAREN = /(\:host)(?:\(((?:\([^)(]*\)|[^)(]*)+?)\))/g;
260260
var HOST_CONTEXT = ':host-context';
261-
var HOST_CONTEXT_PAREN = /(.*)(?:\:host-context)(?:\(((?:\([^)(]*\)|[^)(]*)+?)\))(.*)/;
261+
var HOST_CONTEXT_PAREN = /(.*)(?::host-context)(?:\(((?:\([^)(]*\)|[^)(]*)+?)\))(.*)/;
262262
var CONTENT = '::content';
263-
var SCOPE_JUMP = /\:\:content|\:\:shadow|\/deep\//;
263+
var SCOPE_JUMP = /::content|::shadow|\/deep\//;
264264
var CSS_CLASS_PREFIX = '.';
265265
var CSS_ATTR_PREFIX = '[' + SCOPE_NAME + '~=';
266266
var CSS_ATTR_SUFFIX = ']';

‎src/lib/style-util.html‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,10 @@
4444
return style.__cssRules;
4545
},
4646

47-
clearStyleRules: function(style) {
48-
style.__cssRules = null;
49-
},
50-
5147
forEachStyleRule: function(node, callback) {
5248
if (!node) {
5349
return;
5450
}
55-
var s = node.parsedSelector;
5651
var skipRules = false;
5752
if (node.type === this.ruleTypes.STYLE_RULE) {
5853
callback(node);

‎src/standard/styling.html‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@
7979
style.textContent = cssText;
8080
// extends!!
8181
if (styleExtends.hasExtends(style.textContent)) {
82+
// TODO(sorvell): variable is not used, should it update `style.textContent`?
8283
cssText = styleExtends.transform(style);
8384
}
8485
styles.push(style);
@@ -109,7 +110,7 @@
109110

110111
/**
111112
* Apply style scoping to the specified `container` and all its
112-
* descendants. If `shoudlObserve` is true, changes to the container are
113+
* descendants. If `shouldObserve` is true, changes to the container are
113114
* monitored via mutation observer and scoping is applied.
114115
*
115116
* This method is useful for ensuring proper local DOM CSS scoping

‎test/unit/css-parse.html‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,8 @@
166166
assert.equal(t.rules[3].selector, '.\\0c3333d-model');
167167
assert.equal(t.rules[4].selector, '.\\d33333d-model');
168168
assert.equal(t.rules[5].selector, '.\\e33333d-model');
169-
});
169+
});
170+
170171
test('multiple consequent spaces in CSS selector', function() {
171172
var s4 = document.querySelector('#multiple-spaces');
172173
var t = css.parse(s4.textContent);

‎test/unit/styling-remote.html‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
test(':host, :host(...)', function() {
5858
assertComputed(styled, '1px');
5959
assertComputed(styledWide, '2px');
60-
60+
6161
});
6262

6363
test('scoped selectors, simple and complex', function() {
@@ -209,7 +209,7 @@
209209
});
210210
});
211211
}
212-
212+
213213
});
214214

215215
</script>

‎test/unit/styling-scoped-elements.html‎

Lines changed: 98 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,6 @@
286286
})();
287287
</script>
288288

289-
290289
<template id="dynamic-style-template">
291290
<style>
292291
:host {
@@ -308,4 +307,101 @@
308307
}
309308
});
310309
})();
311-
</script>
310+
</script>
311+
312+
<dom-module id="x-specificity">
313+
<template>
314+
<style>
315+
:host {
316+
border-top: 1px solid red;
317+
}
318+
:host(.bar) {
319+
border-top: 2px solid red;
320+
}
321+
</style>
322+
<content></content>
323+
</template>
324+
<script>
325+
Polymer({is: 'x-specificity'});
326+
</script>
327+
</dom-module>
328+
329+
<style is="custom-style">
330+
:root {
331+
--x-specificity-parent : {
332+
border: 10px solid blue;
333+
};
334+
--x-specificity-nested : {
335+
border: 3px solid red;
336+
};
337+
}
338+
</style>
339+
340+
<dom-module id="x-specificity-parent">
341+
<template>
342+
<style>
343+
/* TODO remove `:host` when https://github.com/Polymer/polymer/pull/2419 merged */
344+
:host ::content > :not(template) {
345+
@apply(--x-specificity-parent);
346+
}
347+
</style>
348+
<content></content>
349+
</template>
350+
<script>
351+
Polymer({is: 'x-specificity-parent', extends: 'div'});
352+
</script>
353+
</dom-module>
354+
355+
<dom-module id="x-specificity-nested">
356+
<template>
357+
<style>
358+
:host {
359+
@apply(--x-specificity-nested);
360+
}
361+
</style>
362+
</template>
363+
<script>
364+
Polymer({is: 'x-specificity-nested', extends: 'div'});
365+
</script>
366+
</dom-module>
367+
368+
<style is="custom-style">
369+
:root {
370+
--x-overriding : {
371+
border-top: 1px solid red;
372+
}
373+
}
374+
</style>
375+
376+
<dom-module id="x-overriding">
377+
<template>
378+
<style>
379+
.red {
380+
@apply(--x-overriding);
381+
}
382+
.green {
383+
@apply(--x-overriding);
384+
border-top: 2px solid green;
385+
}
386+
.red-2 {
387+
border-top: 2px solid green;
388+
@apply(--x-overriding);
389+
}
390+
.blue {
391+
@apply(--x-overriding);
392+
border-top: 3px solid blue;
393+
}
394+
</style>
395+
396+
<div class="red">red</div>
397+
<div class="green">green</div>
398+
<div class="red-2">green-2</div>
399+
<div class="blue">blue</div>
400+
</template>
401+
</dom-module>
402+
403+
<script>
404+
Polymer({
405+
is: 'x-overriding'
406+
});
407+
</script>

‎test/unit/styling-scoped.html‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@
4343
<span id="dom-bind-dynamic" class$="[[dynamic]]">[[dynamic]]</span>
4444
</template>
4545

46+
<x-specificity></x-specificity>
47+
<x-specificity class="bar"></x-specificity>
48+
<div is="x-specificity-parent">
49+
<div is="x-specificity-nested"></div>
50+
</div>
51+
<x-overriding></x-overriding>
52+
4653
<script>
4754
suite('scoped-styling', function() {
4855

@@ -61,7 +68,7 @@
6168
test(':host, :host(...)', function() {
6269
assertComputed(styled, '1px');
6370
assertComputed(styledWide, '2px');
64-
71+
6572
});
6673

6774
test(':host-context(...)', function() {
@@ -210,8 +217,9 @@
210217

211218
test('styles shimmed in registration order', function() {
212219
var s$ = document.head.querySelectorAll('style[scope]');
213-
var expected = ['x-gchild', 'x-child2', 'x-styled', 'x-button',
214-
'x-mixed-case', 'x-mixed-case-button', 'x-dynamic-scope', 'x-dynamic-template'];
220+
var expected = ['x-gchild', 'x-child2', 'x-styled', 'x-button', 'x-mixed-case',
221+
'x-mixed-case-button', 'x-dynamic-scope', 'x-dynamic-template', 'x-specificity', 'x-overriding',
222+
'x-overriding-0', 'x-specificity-parent-0', 'x-specificity-nested-0'];
215223
var actual = [];
216224
for (var i=0; i<s$.length; i++) {
217225
actual.push(s$[i].getAttribute('scope'));
@@ -252,10 +260,26 @@
252260
x = document.createElement('button', 'x-mixed-case-button');
253261
document.body.appendChild(x);
254262
assertComputed(x, '14px');
255-
263+
});
264+
265+
test('specificity of :host selector with class', function() {
266+
assertComputed(document.querySelector('x-specificity'), '1px');
267+
assertComputed(document.querySelector('x-specificity.bar'), '2px');
268+
});
269+
270+
test('specificity of ::content > :not(template) selector', function() {
271+
assertComputed(document.querySelector('[is=x-specificity-nested]'), '10px');
272+
});
273+
274+
test('overwriting mixin properties', function() {
275+
var root = document.querySelector('x-overriding');
276+
assertComputed(root.querySelector('.red'), '1px');
277+
assertComputed(root.querySelector('.green'), '2px');
278+
assertComputed(root.querySelector('.red-2'), '1px');
279+
assertComputed(root.querySelector('.blue'), '3px');
256280
});
257281
});
258-
282+
259283
});
260284

261285
</script>

0 commit comments

Comments
 (0)