From: Jakub Pawlowicz <contact@jakubpawlowicz.com>
Date: Mon, 28 Sep 2015 08:43:31 +0000 (+0100)
Subject: Fixes #681 - property inheritance & restructuring.
X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=fe4677efc683a22a6e54f4e38b442d8b62b31887;p=clean-css.git

Fixes #681 - property inheritance & restructuring.

We should not reorder CSS properties which are inherited and happen
to have a shorthand, which applies to `font`, `line-height` (which
shorthand is `font`), and `list-style`.

Other properties that don't have a shorthand are already not
reorderable based on their value.
---

diff --git a/History.md b/History.md
index 113e7b13..02d04dd4 100644
--- a/History.md
+++ b/History.md
@@ -6,6 +6,7 @@
 [3.4.5 / 2015-xx-xx](https://github.com/jakubpawlowicz/clean-css/compare/v3.4.4...3.4)
 ==================
 
+* Fixed issue [#681](https://github.com/jakubpawlowicz/clean-css/issues/681) - property inheritance & restructuring.
 * Fixed issue [#675](https://github.com/jakubpawlowicz/clean-css/issues/675) - overriding with `!important`.
 
 [3.4.4 / 2015-09-21](https://github.com/jakubpawlowicz/clean-css/compare/v3.4.3...v3.4.4)
diff --git a/lib/selectors/reorderable.js b/lib/selectors/reorderable.js
index 97288b30..90bef4fd 100644
--- a/lib/selectors/reorderable.js
+++ b/lib/selectors/reorderable.js
@@ -46,7 +46,7 @@ function canReorderSingle(left, right) {
     return true;
   if (leftName != rightName && leftNameRoot == rightNameRoot && leftValue == rightValue)
     return true;
-  if (rightInSpecificSelector && leftInSpecificSelector && selectorsDoNotOverlap(rightSelector, leftSelector))
+  if (rightInSpecificSelector && leftInSpecificSelector && !inheritable(leftNameRoot) && !inheritable(rightNameRoot) && selectorsDoNotOverlap(rightSelector, leftSelector))
     return true;
 
   return false;
@@ -83,6 +83,12 @@ function selectorsDoNotOverlap(s1, s2) {
   return true;
 }
 
+function inheritable(name) {
+  // According to http://www.w3.org/TR/CSS21/propidx.html
+  // Others will be catched by other, preceeding rules
+  return name == 'font' || name == 'line-height' || name == 'list-style';
+}
+
 module.exports = {
   canReorder: canReorder,
   canReorderSingle: canReorderSingle
diff --git a/test/fixtures/big-min.css b/test/fixtures/big-min.css
index c79b24d0..c8bc893e 100644
--- a/test/fixtures/big-min.css
+++ b/test/fixtures/big-min.css
@@ -1,5 +1,4 @@
 /*! normalize.css 2012-01-31T16:06 UTC - http://github.com/necolas/normalize.css */
-small,sub,sup{font-size:75%}
 .bt_abo:hover,a{text-decoration:none}
 .bt_fonce a,.btn,.btn_abo,.btn_fonce,.btn_petit{filter:progid:dximagetransform.microsoft.gradient(enabled=false)}
 article,aside,details,figcaption,figure,footer,header,hgroup,nav,section{display:block}
@@ -17,6 +16,7 @@ blockquote{margin:1em 40px}
 dfn{font-style:italic}
 mark{background:#ff0;color:#000}
 code,kbd,pre,samp{font-family:monospace,serif;font-size:1em}
+small,sub,sup{font-size:75%}
 pre{white-space:pre;white-space:pre-wrap;word-wrap:break-word}
 q{quotes:none}
 q:after,q:before{content:'';content:none}
@@ -2275,7 +2275,6 @@ form .btn-rounded-degraded input[type=button],form .btn-rounded-degraded input[t
 #core-liberation .toolbox li.spacer span{display:block;width:1px;height:20px;margin-top:5px}
 #core-liberation .toolbox .txt-min,#core-liberation .toolbox .txt-plus{display:block;font-size:15px;padding-top:4px}
 #core-liberation .toolbox .txt-reset{display:block;font-family:"Times New Roman",Times,Georgia,serif;font-size:19px;padding:4px 0 0 3px}
-#page-mailfriend,#page-paywall{font-family:Verdana,sans-serif}
 .site-liberation .toolbox li a span{display:block}
 .site-liberation .toolbox li a.print span{margin-top:7px;width:16px;height:16px}
 .site-liberation .toolbox li a.favorite span{margin-top:5px;width:20px;height:18px}
@@ -2302,6 +2301,7 @@ form .btn-rounded-degraded input[type=button],form .btn-rounded-degraded input[t
 #core-liberation .sb-podcasts ul li a.itunes{background-position:-267px 0;width:50px}
 #core-liberation .sb-podcasts ul li a.xml{background-position:-322px 0;width:41px}
 #bar-liberation{display:block;position:fixed;top:0;left:0;z-index:10000;width:100%;height:40px;border-bottom:1px solid;font-family:Arial,Verdana,sans-serif;font-size:12px;line-height:14px}
+#page-mailfriend,#page-paywall{font-family:Verdana,sans-serif}
 body.init-bar-is-closed #bar-liberation{height:15px}
 #bar-liberation a,#bar-liberation a p{text-decoration:none;outline:0}
 #bar-liberation .content{position:relative;margin:auto;height:40px;width:1068px}
diff --git a/test/fixtures/blueprint-min.css b/test/fixtures/blueprint-min.css
index a9cd7b76..71e99f05 100644
--- a/test/fixtures/blueprint-min.css
+++ b/test/fixtures/blueprint-min.css
@@ -1,22 +1,23 @@
 h1 img,h2 img,h3 img,h4 img,h5 img,h6 img,html{margin:0}
+.pull-1,.pull-10,.pull-11,.pull-12,.pull-13,.pull-14,.pull-15,.pull-16,.pull-17,.pull-18,.pull-19,.pull-2,.pull-20,.pull-21,.pull-22,.pull-23,.pull-24,.pull-3,.pull-4,.pull-5,.pull-6,.pull-7,.pull-8,.pull-9,.push-1,.push-10,.push-11,.push-12,.push-13,.push-14,.push-15,.push-16,.push-17,.push-18,.push-19,.push-2,.push-20,.push-21,.push-22,.push-23,.push-24,.push-3,.push-4,.push-5,.push-6,.push-7,.push-8,.push-9{position:relative;float:left}
 .clear,hr{clear:both}
-html{padding:0;border:0;font-size:100.01%}
 a,abbr,acronym,address,article,aside,blockquote,body,caption,code,dd,del,dfn,dialog,div,dl,dt,em,fieldset,figure,footer,form,h1,h2,h3,h4,h5,h6,header,hgroup,iframe,img,label,legend,li,nav,object,ol,p,pre,q,section,span,table,tbody,td,tfoot,th,thead,tr,ul{margin:0;padding:0;border:0;font-weight:inherit;font-style:inherit;font-size:100%;font-family:inherit;vertical-align:baseline}
-address,blockquote,dfn,em,tfoot{font-style:italic}
-h5,h6{font-size:1em;font-weight:700}
+address,blockquote,dfn,em{font-style:italic}
 article,aside,dialog,figure,footer,header,hgroup,nav,section{display:block}
-body{line-height:1.5;font-size:75%;color:#222;background:#fff;font-family:"Helvetica Neue",Arial,Helvetica,sans-serif}
 caption,td,th{text-align:left;font-weight:400;float:none!important}
 table,td,th{vertical-align:middle}
 blockquote:after,blockquote:before,q:after,q:before{content:''}
 blockquote,q{quotes:"" ""}
 a img{border:none}
 :focus{outline:0}
+html{padding:0;border:0;font-size:100.01%}
+body{line-height:1.5;font-size:75%;color:#222;background:#fff;font-family:"Helvetica Neue",Arial,Helvetica,sans-serif}
 h1,h2,h3,h4,h5,h6{font-weight:400;color:#111}
 h1{font-size:3em;line-height:1;margin-bottom:.5em}
 h2{font-size:2em;margin-bottom:.75em}
 h3{font-size:1.5em;line-height:1;margin-bottom:1em}
 h4{font-size:1.2em;line-height:1.25;margin-bottom:1.25em}
+h5,h6{font-size:1em;font-weight:700}
 h5{margin-bottom:1.5em}
 p{margin:0 0 1.5em}
 .left{float:left!important}
@@ -33,6 +34,7 @@ sub,sup{line-height:0}
 abbr,acronym{border-bottom:1px dotted #666}
 pre{margin:1.5em 0;white-space:pre}
 code,pre,tt{font:1em 'andale mono','lucida console',monospace;line-height:1.5}
+label,legend{font-weight:700}
 li ol,li ul{margin:0}
 ol,ul{margin:0 1.5em 1.5em 0;padding-left:1.5em}
 ul{list-style-type:disc}
@@ -42,6 +44,7 @@ table{border-collapse:separate;border-spacing:0;margin-bottom:1.4em;width:100%}
 thead th{background:#c3d9ff}
 caption,td,th{padding:4px 10px 4px 5px}
 tbody tr.even td,tbody tr:nth-child(even) td{background:#e5ecf9}
+tfoot{font-style:italic}
 caption{background:#eee}
 .small{font-size:.8em;margin-bottom:1.875em;line-height:1.875em}
 .large,legend{font-size:1.2em}
@@ -55,9 +58,7 @@ caption{background:#eee}
 .last{padding-right:0}
 .top{margin-top:0;padding-top:0}
 .bottom{margin-bottom:0;padding-bottom:0}
-label{font-weight:700}
 fieldset{padding:0 1.4em 1.4em;margin:0 0 1.5em;border:1px solid #ccc}
-legend{font-weight:700}
 #IE8#HACK,fieldset{padding-top:1.4em}
 #IE8#HACK,legend{margin-top:0;margin-bottom:0}
 input.text,input.title,input[type=password],input[type=text],textarea{background-color:#fff;border:1px solid #bbb}
@@ -204,7 +205,6 @@ input.span-24,textarea.span-24{width:938px}
 .pull-22{margin-left:-880px}
 .pull-23{margin-left:-920px}
 .pull-24{margin-left:-960px}
-.pull-1,.pull-10,.pull-11,.pull-12,.pull-13,.pull-14,.pull-15,.pull-16,.pull-17,.pull-18,.pull-19,.pull-2,.pull-20,.pull-21,.pull-22,.pull-23,.pull-24,.pull-3,.pull-4,.pull-5,.pull-6,.pull-7,.pull-8,.pull-9{float:left;position:relative}
 .push-1{margin:0 -40px 1.5em 40px}
 .push-2{margin:0 -80px 1.5em 80px}
 .push-3{margin:0 -120px 1.5em 120px}
@@ -230,7 +230,6 @@ input.span-24,textarea.span-24{width:938px}
 .push-23{margin:0 -920px 1.5em 920px}
 .push-24{margin:0 -960px 1.5em 960px}
 .append-bottom,.box,div.append-bottom{margin-bottom:1.5em}
-.push-1,.push-10,.push-11,.push-12,.push-13,.push-14,.push-15,.push-16,.push-17,.push-18,.push-19,.push-2,.push-20,.push-21,.push-22,.push-23,.push-24,.push-3,.push-4,.push-5,.push-6,.push-7,.push-8,.push-9{float:left;position:relative}
 .prepend-top,div.prepend-top{margin-top:1.5em}
 .box{padding:1.5em;background:#e5eCf9}
 hr{background:#ddd;color:#ddd;float:none;width:100%;height:1px;margin:0 0 1.45em;border:none}
diff --git a/test/fixtures/bootstrap-min.css b/test/fixtures/bootstrap-min.css
index 84d5cf47..d19f3f99 100644
--- a/test/fixtures/bootstrap-min.css
+++ b/test/fixtures/bootstrap-min.css
@@ -2,7 +2,7 @@ hr,img{border:0}
 body,figure{margin:0}
 .btn-group>.btn-group,.btn-toolbar .btn-group,.btn-toolbar .input-group,.col-xs-1,.col-xs-10,.col-xs-11,.col-xs-12,.col-xs-2,.col-xs-3,.col-xs-4,.col-xs-5,.col-xs-6,.col-xs-7,.col-xs-8,.col-xs-9,.dropdown-menu{float:left}
 .navbar-fixed-bottom .navbar-collapse,.navbar-fixed-top .navbar-collapse,.pre-scrollable{max-height:340px}
-html{font-family:sans-serif;-webkit-text-size-adjust:100%;-ms-text-size-adjust:100%;font-size:10px;-webkit-tap-highlight-color:transparent}
+html{font-family:sans-serif;-webkit-text-size-adjust:100%;-ms-text-size-adjust:100%}
 article,aside,details,figcaption,figure,footer,header,hgroup,main,menu,nav,section,summary{display:block}
 audio,canvas,progress,video{display:inline-block;vertical-align:baseline}
 audio:not([controls]){display:none;height:0}
@@ -316,6 +316,7 @@ select{background:#fff!important}
 .glyphicon-menu-down:before{content:"\e259"}
 .glyphicon-menu-up:before{content:"\e260"}
 *,:after,:before{-webkit-box-sizing:border-box;-moz-box-sizing:border-box;box-sizing:border-box}
+html{font-size:10px;-webkit-tap-highlight-color:transparent}
 body{font-family:"Helvetica Neue",Helvetica,Arial,sans-serif;font-size:14px;line-height:1.42857143;color:#333}
 button,input,select,textarea{font-family:inherit;font-size:inherit;line-height:inherit}
 a{color:#337ab7;text-decoration:none}
diff --git a/test/selectors/reorderable-test.js b/test/selectors/reorderable-test.js
index 08bcc87b..ea8fcb08 100644
--- a/test/selectors/reorderable-test.js
+++ b/test/selectors/reorderable-test.js
@@ -241,6 +241,14 @@ vows.describe(canReorderSingle)
         assert.isTrue(result);
       }
     },
+    'different, non-overlapping simple selectors with inheritable property': {
+      'topic': function () {
+        return canReorderSingle(propertiesIn('a{font:inherit}')[0], propertiesIn('div{font-family:Helvetica}')[0]);
+      },
+      'must be false': function (result) {
+        assert.isFalse(result);
+      }
+    },
     'different, non-overlapping complex selectors': {
       'topic': function () {
         return canReorderSingle(propertiesIn('.one{border:none}')[0], propertiesIn('div{border:1px solid #f00}')[0]);
diff --git a/test/selectors/restructure-test.js b/test/selectors/restructure-test.js
index 3a036028..48d11224 100644
--- a/test/selectors/restructure-test.js
+++ b/test/selectors/restructure-test.js
@@ -4,10 +4,14 @@ var optimizerContext = require('../test-helper').optimizerContext;
 vows.describe('restructure')
   .addBatch(
     optimizerContext('advanced on', {
-      'up until changed': [
+      'up until changed #1': [
         'a{color:#000}div{color:red}.one{display:block}.two{display:inline;color:red}',
         'a{color:#000}.two,div{color:red}.one{display:block}.two{display:inline}'
       ],
+      'up until changed #2': [
+        'p{margin:0;font:inherit}h1{font-family:Helvetica;margin:20px}section h1{font-family:Helvetica;margin:40px}',
+        'p{margin:0;font:inherit}h1,section h1{font-family:Helvetica}h1{margin:20px}section h1{margin:40px}'
+      ],
       'up until top': [
         'a{width:75px}div{color:red}.one{display:block}.two{display:inline;color:red}',
         '.two,div{color:red}a{width:75px}.one{display:block}.two{display:inline}'