From 079c0c4a297926f50d4d104729164aef2dced846 Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Mon, 30 May 2016 08:18:30 +0200 Subject: [PATCH] Fixes #765 - two values of border-radius. Horizontal and vertical values of longhand properties were not correctly merged into shorthand border-radius. --- History.md | 1 + lib/properties/break-up.js | 13 +++++----- lib/properties/restore.js | 5 +++- test/properties/break-up-test.js | 26 ++++++++++---------- test/properties/shorthand-compacting-test.js | 24 ++++++++++++++++++ test/selectors/simple-test.js | 12 ++++++--- 6 files changed, 57 insertions(+), 24 deletions(-) diff --git a/History.md b/History.md index ed841708..d330c711 100644 --- a/History.md +++ b/History.md @@ -8,6 +8,7 @@ * Fixed issue [#751](https://github.com/jakubpawlowicz/clean-css/issues/751) - stringifying CSS variables. * Fixed issue [#763](https://github.com/jakubpawlowicz/clean-css/issues/763) - data URI SVG and quoting. +* Fixed issue [#765](https://github.com/jakubpawlowicz/clean-css/issues/765) - two values of border-radius. [3.4.13 / 2016-05-23](https://github.com/jakubpawlowicz/clean-css/compare/v3.4.12...v3.4.13) ================== diff --git a/lib/properties/break-up.js b/lib/properties/break-up.js index b48b146d..c5dfc414 100644 --- a/lib/properties/break-up.js +++ b/lib/properties/break-up.js @@ -131,20 +131,21 @@ function borderRadius(property, compactable) { } } - if (splitAt == -1) - return fourValues(property, compactable); - var target = _wrapDefault(property.name, property, compactable); - target.value = values.slice(0, splitAt); + target.value = splitAt > -1 ? + values.slice(0, splitAt) : + values.slice(0); target.components = fourValues(target, compactable); var remainder = _wrapDefault(property.name, property, compactable); - remainder.value = values.slice(splitAt + 1); + remainder.value = splitAt > -1 ? + values.slice(splitAt + 1) : + values.slice(0); remainder.components = fourValues(remainder, compactable); for (var j = 0; j < 4; j++) { target.components[j].multiplex = true; - target.components[j].value = target.components[j].value.concat([['/']]).concat(remainder.components[j].value); + target.components[j].value = target.components[j].value.concat(remainder.components[j].value); } return target.components; diff --git a/lib/properties/restore.js b/lib/properties/restore.js index d9d40f90..a9f8e6f9 100644 --- a/lib/properties/restore.js +++ b/lib/properties/restore.js @@ -110,7 +110,10 @@ function borderRadius(property, compactable) { horizontal.components.push(horizontalComponent); var verticalComponent = shallowClone(property); - verticalComponent.value = [component.value[2]]; + // FIXME: only shorthand compactor (see breakup#borderRadius) knows that border radius + // longhands have two values, whereas tokenizer does not care about populating 2nd value + // if it's missing, hence this fallback + verticalComponent.value = [component.value[1] || component.value[0]]; vertical.components.push(verticalComponent); } diff --git a/test/properties/break-up-test.js b/test/properties/break-up-test.js index ad92d0c7..a0c8cea8 100644 --- a/test/properties/break-up-test.js +++ b/test/properties/break-up-test.js @@ -312,19 +312,19 @@ vows.describe(breakUp) }, 'has border-top-left-radius': function (components) { assert.equal(components[0].name, 'border-top-left-radius'); - assert.deepEqual(components[0].value, [['0px']]); + assert.deepEqual(components[0].value, [['0px'], ['0px']]); }, 'has border-top-right-radius': function (components) { assert.equal(components[1].name, 'border-top-right-radius'); - assert.deepEqual(components[1].value, [['1px']]); + assert.deepEqual(components[1].value, [['1px'], ['1px']]); }, 'has border-bottom-right-radius': function (components) { assert.equal(components[2].name, 'border-bottom-right-radius'); - assert.deepEqual(components[2].value, [['2px']]); + assert.deepEqual(components[2].value, [['2px'], ['2px']]); }, 'has border-bottom-left': function (components) { assert.equal(components[3].name, 'border-bottom-left-radius'); - assert.deepEqual(components[3].value, [['3px']]); + assert.deepEqual(components[3].value, [['3px'], ['3px']]); } }, 'horizontal vertical split': { @@ -336,22 +336,22 @@ vows.describe(breakUp) }, 'has border-top-left-radius': function (components) { assert.equal(components[0].name, 'border-top-left-radius'); - assert.deepEqual(components[0].value, [['0px'], ['/'], ['1px']]); + assert.deepEqual(components[0].value, [['0px'], ['1px']]); }, 'has border-top-right-radius': function (components) { assert.equal(components[1].name, 'border-top-right-radius'); - assert.deepEqual(components[1].value, [['1px'], ['/'], ['2px']]); + assert.deepEqual(components[1].value, [['1px'], ['2px']]); }, 'has border-bottom-right-radius': function (components) { assert.equal(components[2].name, 'border-bottom-right-radius'); - assert.deepEqual(components[2].value, [['2px'], ['/'], ['3px']]); + assert.deepEqual(components[2].value, [['2px'], ['3px']]); }, 'has border-bottom-left': function (components) { assert.equal(components[3].name, 'border-bottom-left-radius'); - assert.deepEqual(components[3].value, [['3px'], ['/'], ['4px']]); + assert.deepEqual(components[3].value, [['3px'], ['4px']]); } }, - 'vendor prefix asymetrical horizontal vertical split': { + 'vendor prefix asymmetrical horizontal vertical split': { 'topic': function () { return _breakUp([[['-webkit-border-radius'], ['0px'], ['1px'], ['2px'], ['/'], ['1px'], ['4px']]]); }, @@ -360,19 +360,19 @@ vows.describe(breakUp) }, 'has border-top-left-radius': function (components) { assert.equal(components[0].name, '-webkit-border-top-left-radius'); - assert.deepEqual(components[0].value, [['0px'], ['/'], ['1px']]); + assert.deepEqual(components[0].value, [['0px'], ['1px']]); }, 'has border-top-right-radius': function (components) { assert.equal(components[1].name, '-webkit-border-top-right-radius'); - assert.deepEqual(components[1].value, [['1px'], ['/'], ['4px']]); + assert.deepEqual(components[1].value, [['1px'], ['4px']]); }, 'has border-bottom-right-radius': function (components) { assert.equal(components[2].name, '-webkit-border-bottom-right-radius'); - assert.deepEqual(components[2].value, [['2px'], ['/'], ['1px']]); + assert.deepEqual(components[2].value, [['2px'], ['1px']]); }, 'has border-bottom-left': function (components) { assert.equal(components[3].name, '-webkit-border-bottom-left-radius'); - assert.deepEqual(components[3].value, [['1px'], ['/'], ['4px']]); + assert.deepEqual(components[3].value, [['1px'], ['4px']]); } } }, diff --git a/test/properties/shorthand-compacting-test.js b/test/properties/shorthand-compacting-test.js index bee1225b..4dc63a07 100644 --- a/test/properties/shorthand-compacting-test.js +++ b/test/properties/shorthand-compacting-test.js @@ -85,6 +85,30 @@ vows.describe(optimize) ]); } }, + 'shorthand multiplexed border-radius': { + 'topic': 'p{border-radius:7px/3px}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['border-radius'], ['7px'], ['/'], ['3px']] + ]); + } + }, + 'shorthand asymmetric border-radius with same values': { + 'topic': 'p{border-top-left-radius:7px 3px;border-top-right-radius:7px 3px;border-bottom-right-radius:7px 3px;border-bottom-left-radius:7px 3px}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['border-radius'], ['7px'], ['/'], ['3px']] + ]); + } + }, + 'shorthand asymmetric border-radius': { + 'topic': 'p{border-top-left-radius:7px 3px;border-top-right-radius:6px 2px;border-bottom-right-radius:5px 1px;border-bottom-left-radius:4px 0}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['border-radius'], ['7px'], ['6px'], ['5px'], ['4px'], ['/'], ['3px'], ['2px'], ['1px'], ['0']] + ]); + } + }, 'shorthand multiple !important': { 'topic': 'a{border-color:#123 !important;border-top-color: #456 !important}', 'into': function (topic) { diff --git a/test/selectors/simple-test.js b/test/selectors/simple-test.js index 2fcd7214..3f4584bd 100644 --- a/test/selectors/simple-test.js +++ b/test/selectors/simple-test.js @@ -159,12 +159,16 @@ vows.describe('simple optimizations') .addBatch( propertyContext('@border-*-radius', { 'spaces around /': [ - 'a{border-top-left-radius:2em / 1em}', - [['border-top-left-radius', '2em', '/', '1em']] + 'a{border-radius:2em / 1em}', + [['border-radius', '2em', '/', '1em']] ], 'symmetric expanded to shorthand': [ - 'a{border-top-left-radius:1em 2em 3em 4em / 1em 2em 3em 4em}', - [['border-top-left-radius', '1em', '2em', '3em', '4em']] + 'a{border-radius:1em 2em 3em 4em / 1em 2em 3em 4em}', + [['border-radius', '1em', '2em', '3em', '4em']] + ], + 'asymmetric kept as is': [ + 'a{border-top-left-radius:1em 2em}', + [['border-top-left-radius', '1em', '2em']] ] }) ) -- 2.34.1