Skip to content

Commit

Permalink
Fix parsing of custom properties with 'var' in value
Browse files Browse the repository at this point in the history
Fixes #2660
  • Loading branch information
TimvdLippe committed Oct 31, 2015
1 parent 0dc69df commit 61abfbd
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/lib/css-parse.html
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
customProp: /(?:^|[\s;])--[^;{]*?:[^{};]*?(?:[;\n]|$)/gim,
mixinProp: /(?:^|[\s;])?--[^;{]*?:[^{;]*?{[^}]*?}(?:[;\n]|$)?/gim,
mixinApply: /@apply[\s]*\([^)]*?\)[\s]*(?:[;\n]|$)?/gim,
varApply: /[^;:]*?:[^;]*var[^;]*(?:[;\n]|$)?/gim,
varApply: /[^;:]*?:[^;]*?var\([^;]*\)(?:[;\n]|$)?/gim,
keyframesRule: /^@[^\s]*keyframes/,
},

Expand Down
24 changes: 12 additions & 12 deletions src/lib/style-transformer.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

/* Transforms ShadowDOM styling into ShadyDOM styling
* scoping:
* scoping:
* elements in scope get scoping selector class="x-foo-scope"
* selectors re-written as follows:
Expand All @@ -41,8 +41,8 @@
*/
var api = {

// Given a node and scope name, add a scoping class to each node
// in the tree. This facilitates transforming css into scoped rules.
// Given a node and scope name, add a scoping class to each node
// in the tree. This facilitates transforming css into scoped rules.
dom: function(node, scope, useAttr, shouldRemoveScope) {
this._transformDom(node, scope || '', useAttr, shouldRemoveScope);
},
Expand Down Expand Up @@ -86,7 +86,7 @@
.replace(scope, ''));
}
} else {
element.setAttribute(CLASS, c + (c ? ' ' : '') +
element.setAttribute(CLASS, c + (c ? ' ' : '') +
SCOPE_NAME + ' ' + scope);
}
}
Expand Down Expand Up @@ -153,7 +153,7 @@
}
// NOTE: save transformedSelector for subsequent matching of elements
// agsinst selectors (e.g. when calculating style properties)
rule.selector = rule.transformedSelector =
rule.selector = rule.transformedSelector =
p$.join(COMPLEX_SELECTOR_SEP);
},

Expand All @@ -167,16 +167,16 @@
stop = stop || info.stop;
hostContext = hostContext || info.hostContext;
c = info.combinator;
s = info.value;
s = info.value;
} else {
s = s.replace(SCOPE_JUMP, ' ');
}
return c + s;
});
if (hostContext) {
selector = selector.replace(HOST_CONTEXT_PAREN,
selector = selector.replace(HOST_CONTEXT_PAREN,
function(m, pre, paren, post) {
return pre + paren + ' ' + hostScope + post +
return pre + paren + ' ' + hostScope + post +
COMPLEX_SELECTOR_SEP + ' ' + pre + hostScope + paren + post;
});
}
Expand All @@ -198,7 +198,7 @@
selector = selector.replace(HOST, hostScope);
// replace other selectors with scoping class
} else if (jumpIndex !== 0) {
selector = scope ? this._transformSimpleSelector(selector, scope) :
selector = scope ? this._transformSimpleSelector(selector, scope) :
selector;
}
// remove left-side combinator when dealing with ::content.
Expand All @@ -212,7 +212,7 @@
selector = selector.replace(SCOPE_JUMP, ' ');
stop = true;
}
return {value: selector, combinator: combinator, stop: stop,
return {value: selector, combinator: combinator, stop: stop,
hostContext: hostContext};
},

Expand Down Expand Up @@ -247,13 +247,13 @@
};

var SCOPE_NAME = api.SCOPE_NAME;
var SCOPE_DOC_SELECTOR = ':not([' + SCOPE_NAME + '])' +
var SCOPE_DOC_SELECTOR = ':not([' + SCOPE_NAME + '])' +
':not(.' + SCOPE_NAME + ')';
var COMPLEX_SELECTOR_SEP = ',';
var SIMPLE_SELECTOR_SEP = /(^|[\s>+~]+)([^\s>+~]+)/g;
var HOST = ':host';
var ROOT = ':root';
// NOTE: this supports 1 nested () pair for things like
// NOTE: this supports 1 nested () pair for things like
// :host(:not([selected]), more general support requires
// parsing which seems like overkill
var HOST_PAREN = /(\:host)(?:\(((?:\([^)(]*\)|[^)(]*)+?)\))/g;
Expand Down
12 changes: 6 additions & 6 deletions src/lib/style-util.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
toCssText: function(rules, callback, preserveProperties) {
if (typeof rules === 'string') {
rules = this.parser.parse(rules);
}
}
if (callback) {
this.forEachStyleRule(rules, callback);
}
Expand Down Expand Up @@ -56,7 +56,7 @@
var skipRules = false;
if (node.type === this.ruleTypes.STYLE_RULE) {
callback(node);
} else if (node.type === this.ruleTypes.KEYFRAMES_RULE ||
} else if (node.type === this.ruleTypes.KEYFRAMES_RULE ||
node.type === this.ruleTypes.MIXIN_RULE) {
skipRules = true;
}
Expand All @@ -79,8 +79,8 @@
if (!afterNode) {
var n$ = target.querySelectorAll('style[scope]');
afterNode = n$[n$.length-1];
}
target.insertBefore(style,
}
target.insertBefore(style,
(afterNode && afterNode.nextSibling) || target.firstChild);
return style;
},
Expand Down Expand Up @@ -142,7 +142,7 @@
}
return cssText;
},

resolveCss: Polymer.ResolveUrl.resolveCss,
parser: Polymer.CssParse,
ruleTypes: Polymer.CssParse.types
Expand All @@ -151,4 +151,4 @@

})();

</script>
</script>
81 changes: 79 additions & 2 deletions test/unit/custom-style.html
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@
body /deep/ * {
--deeep: 6px solid orange;
}


</style>
<style is="custom-style">
.bag {
Expand Down Expand Up @@ -138,6 +136,8 @@

<x-blue-bold-text></x-blue-bold-text>

<parent-variable-with-var></parent-variable-with-var>

<br><br>
<div id="after"></div>

Expand Down Expand Up @@ -212,10 +212,59 @@
</template>
</dom-module>

<dom-module id="parent-variable-with-var">
<template>
<style>
child-variable-with-var {
--variable-property-own-line: 1px;
--variable-property-preceded-property: 2px;
--variable-property-before-property: yellow;
--variable-property-after-property: 3px;
--variable-property-after-assignment: 4px;
--variable-property-before-assignment: 5px;
}
</style>
<child-variable-with-var id="child"></child-variable-with-var>
</template>
</dom-module>

<dom-module id="child-variable-with-var">
<template>
<style>
child-of-child-with-var {
--variable-own-line: "Varela font";
margin-top: var(--variable-property-own-line);
margin-bottom: var(--variable-property-preceded-property);
--variable-between-properties: 6px;
background-color: var(--variable-property-before-property); padding-top: var(--variable-property-after-property);
--variable-assignment-before-property: 7px; padding-bottom: var(--variable-property-after-assignment);
padding-left: var(--variable-property-before-assignment);--variable-assignment-after-property: 8px
}
</style>
<child-of-child-with-var id="child"></child-of-child-with-var>
</template>
</dom-module>

<dom-module id="child-of-child-with-var">
<template>
<style>
:host {
font-family: var(--variable-own-line);
padding-right: var(--variable-between-properties);
margin-left: var(--variable-assignment-before-property);
margin-right: var(--variable-assignment-after-property);
}
</style>
Text
</template>
</dom-module>

<script>

suite('custom-style', function() {

var xBar, xFoo;

suiteSetup(function() {

Polymer({
Expand Down Expand Up @@ -385,6 +434,34 @@
document.body.removeChild(d);
});

test('variable name with assignment including var correctly applied', function() {
Polymer({
is: 'parent-variable-with-var'
});
Polymer({
is: 'child-variable-with-var'
});
Polymer({
is: 'child-of-child-with-var'
});

var d = document.querySelector('parent-variable-with-var');
var el = d.$.child.$.child;
assertComputed(el, '1px', 'margin-top');
assertComputed(el, '2px', 'margin-bottom');
assertComputed(el, '3px', 'padding-top');
assertComputed(el, '4px', 'padding-bottom');
assertComputed(el, '5px', 'padding-left');
assertComputed(el, '6px', 'padding-right');
assertComputed(el, '7px', 'margin-left');
assertComputed(el, '8px', 'margin-right');
assertComputed(el, 'rgb(255, 255, 0)', 'background-color');

// Because FireFox and Chrome parse font-family differently...
var computed = getComputedStyle(el);
assert.equal(computed['font-family'].replace(/['"]+/g, ''), 'Varela font');
});

});


Expand Down

0 comments on commit 61abfbd

Please sign in to comment.