From 50836f26ca3d1eb0a3711e906b5a1d19344db8df Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Mon, 7 Nov 2011 11:22:24 +1300 Subject: [PATCH] MDL-29941 csslib: Improved unit tests and fixed bugs with background and border partial style optimisation --- lib/csslib.php | 200 ++++++++++++++++++++++++++++------ lib/simpletest/testcsslib.php | 52 +++++++-- 2 files changed, 209 insertions(+), 43 deletions(-) diff --git a/lib/csslib.php b/lib/csslib.php index a46f13c7b1c..fdbde7c50b6 100644 --- a/lib/csslib.php +++ b/lib/csslib.php @@ -1030,7 +1030,7 @@ class css_rule { $consolidate = array(); foreach ($this->styles as $style) { $consolidatetoclass = $style->consolidate_to(); - if (!empty($consolidatetoclass) && class_exists('css_style_'.$consolidatetoclass)) { + if ($style->is_valid() && !empty($consolidatetoclass) && class_exists('css_style_'.$consolidatetoclass)) { $class = 'css_style_'.$consolidatetoclass; if (!array_key_exists($class, $consolidate)) { $consolidate[$class] = array(); @@ -1301,6 +1301,10 @@ abstract class css_style { } } + public function is_valid() { + return true; + } + /** * Returns the name for the style * @@ -1427,10 +1431,21 @@ class css_style_color extends css_style { * @return string */ public function out($overridevalue = null) { - if (preg_match('/#([a-fA-F0-9])\1([a-fA-F0-9])\2([a-fA-F0-9])\3/', $this->value, $matches)) { - $overridevalue = '#'.$matches[1].$matches[2].$matches[3]; + if ($overridevalue === null) { + $overridevalue = $this->value; } - return parent::out($overridevalue); + return parent::out(self::shrink_value($overridevalue)); + } + + public static function shrink_value($value) { + if (preg_match('/#([a-fA-F0-9])\1([a-fA-F0-9])\2([a-fA-F0-9])\3/', $value, $matches)) { + return '#'.$matches[1].$matches[2].$matches[3]; + } + return $value; + } + + public function is_valid() { + return css_is_colour($this->value); } } @@ -1592,9 +1607,9 @@ class css_style_border extends css_style { $nullstyles = in_array(null, $borderstyles); $nullcolors = in_array(null, $bordercolors); - $allwidthsthesame = ($uniquewidths == 1)?1:0; - $allstylesthesame = ($uniquestyles == 1)?1:0; - $allcolorsthesame = ($uniquecolors == 1)?1:0; + $allwidthsthesame = ($uniquewidths === 1)?1:0; + $allstylesthesame = ($uniquestyles === 1)?1:0; + $allcolorsthesame = ($uniquecolors === 1)?1:0; $allwidthsnull = $allwidthsthesame && $nullwidths; $allstylesnull = $allstylesthesame && $nullstyles; @@ -1649,7 +1664,7 @@ class css_style_border extends css_style { self::consolidate_styles_by_direction($return, 'css_style_bordercolor', 'border-color', $bordercolors); } - } else if (max(array_count_values($borderwidths)) == 3 && max(array_count_values($borderstyles)) == 3 && max(array_count_values($bordercolors)) == 3) { + } else if (!$nullwidths && !$nullcolors && !$nullstyles && max(array_count_values($borderwidths)) == 3 && max(array_count_values($borderstyles)) == 3 && max(array_count_values($bordercolors)) == 3) { $widthkeys = array(); $stylekeys = array(); $colorkeys = array(); @@ -1683,9 +1698,9 @@ class css_style_border extends css_style { if ($widthkeys == $stylekeys && $stylekeys == $colorkeys) { $key = $widthkeys[0][0]; - self::build_style_string($return, 'css_style_border', 'border', $borderwidths[$key].' '.$borderstyles[$key].' '.$bordercolors[$key]); + self::build_style_string($return, 'css_style_border', 'border', $borderwidths[$key], $borderstyles[$key], $bordercolors[$key]); $key = $widthkeys[1][0]; - self::build_style_string($return, 'css_style_border'.$key, 'border-'.$key, $borderwidths[$key].' '.$borderstyles[$key].' '.$bordercolors[$key]); + self::build_style_string($return, 'css_style_border'.$key, 'border-'.$key, $borderwidths[$key], $borderstyles[$key], $bordercolors[$key]); } else { self::build_style_string($return, 'css_style_bordertop', 'border-top', $borderwidths['top'], $borderstyles['top'], $bordercolors['top']); self::build_style_string($return, 'css_style_borderright', 'border-right', $borderwidths['right'], $borderstyles['right'], $bordercolors['right']); @@ -1709,7 +1724,6 @@ class css_style_border extends css_style { return 'border'; } public static function consolidate_styles_by_direction(&$array, $class, $style, $top, $right = null, $bottom = null, $left = null) { - if (is_array($top)) { $right = $top['right']; $bottom = $top['bottom']; @@ -1804,10 +1818,10 @@ class css_style_bordercolor extends css_style_color { $left = array_shift($bits); } return array( - new css_style_bordercolor('border-color-top', $top), - new css_style_bordercolor('border-color-right', $right), - new css_style_bordercolor('border-color-bottom', $bottom), - new css_style_bordercolor('border-color-left', $left) + css_style_bordercolortop::init($top), + css_style_bordercolorright::init($right), + css_style_bordercolorbottom::init($bottom), + css_style_bordercolorleft::init($left) ); } public function consolidate_to() { @@ -1818,6 +1832,14 @@ class css_style_bordercolor extends css_style_color { $values = array_map('parent::clean_value', $values); return join (' ', $values); } + public function out($overridevalue = null) { + if ($overridevalue === null) { + $overridevalue = $this->value; + } + $values = explode(' ', $overridevalue); + $values = array_map('css_style_color::shrink_value', $values); + return parent::out(join (' ', $values)); + } } class css_style_borderleft extends css_style_generic { @@ -1827,13 +1849,13 @@ class css_style_borderleft extends css_style_generic { $return = array(); if (count($bits) > 0) { - $return[] = new css_style_borderwidth('border-width-left', array_shift($bits)); + $return[] = css_style_borderwidthleft::init(array_shift($bits)); } if (count($bits) > 0) { - $return[] = new css_style_borderstyle('border-style-left', array_shift($bits)); + $return[] = css_style_borderstyleleft::init(array_shift($bits)); } if (count($bits) > 0) { - $return[] = new css_style_bordercolor('border-color-left', array_shift($bits)); + $return[] = css_style_bordercolorleft::init(array_shift($bits)); } return $return; } @@ -1849,13 +1871,13 @@ class css_style_borderright extends css_style_generic { $return = array(); if (count($bits) > 0) { - $return[] = new css_style_borderwidth('border-width-right', array_shift($bits)); + $return[] = css_style_borderwidthright::init(array_shift($bits)); } if (count($bits) > 0) { - $return[] = new css_style_borderstyle('border-style-right', array_shift($bits)); + $return[] = css_style_borderstyleright::init(array_shift($bits)); } if (count($bits) > 0) { - $return[] = new css_style_bordercolor('border-color-right', array_shift($bits)); + $return[] = css_style_bordercolorright::init(array_shift($bits)); } return $return; } @@ -1871,13 +1893,13 @@ class css_style_bordertop extends css_style_generic { $return = array(); if (count($bits) > 0) { - $return[] = new css_style_borderwidth('border-width-top', array_shift($bits)); + $return[] = css_style_borderwidthtop::init(array_shift($bits)); } if (count($bits) > 0) { - $return[] = new css_style_borderstyle('border-style-top', array_shift($bits)); + $return[] = css_style_borderstyletop::init(array_shift($bits)); } if (count($bits) > 0) { - $return[] = new css_style_bordercolor('border-color-top', array_shift($bits)); + $return[] = css_style_bordercolortop::init(array_shift($bits)); } return $return; } @@ -1893,13 +1915,13 @@ class css_style_borderbottom extends css_style_generic { $return = array(); if (count($bits) > 0) { - $return[] = new css_style_borderwidth('border-width-bottom', array_shift($bits)); + $return[] = css_style_borderwidthbottom::init(array_shift($bits)); } if (count($bits) > 0) { - $return[] = new css_style_borderstyle('border-style-bottom', array_shift($bits)); + $return[] = css_style_borderstylebottom::init(array_shift($bits)); } if (count($bits) > 0) { - $return[] = new css_style_bordercolor('border-color-bottom', array_shift($bits)); + $return[] = css_style_bordercolorbottom::init(array_shift($bits)); } return $return; } @@ -1942,10 +1964,10 @@ class css_style_borderwidth extends css_style_generic { $left = array_shift($bits); } return array( - new css_style_borderwidth('border-width-top', $top), - new css_style_borderwidth('border-width-right', $right), - new css_style_borderwidth('border-width-bottom', $bottom), - new css_style_borderwidth('border-width-left', $left) + css_style_borderwidthtop::init($top), + css_style_borderwidthright::init($right), + css_style_borderwidthbottom::init($bottom), + css_style_borderwidthleft::init($left) ); } public function consolidate_to() { @@ -1987,10 +2009,10 @@ class css_style_borderstyle extends css_style_generic { $left = array_shift($bits); } return array( - new css_style_borderstyle('border-style-top', $top), - new css_style_borderstyle('border-style-right', $right), - new css_style_borderstyle('border-style-bottom', $bottom), - new css_style_borderstyle('border-style-left', $left) + css_style_borderstyletop::init($top), + css_style_borderstyleright::init($right), + css_style_borderstylebottom::init($bottom), + css_style_borderstyleleft::init($left) ); } public function consolidate_to() { @@ -1998,6 +2020,107 @@ class css_style_borderstyle extends css_style_generic { } } +class css_style_bordercolortop extends css_style_color { + public static function init($value) { + return new css_style_bordercolortop('border-color-top', $value); + } + public function consolidate_to() { + return 'border'; + } +} +class css_style_bordercolorleft extends css_style_color { + public static function init($value) { + return new css_style_bordercolorleft('border-color-left', $value); + } + public function consolidate_to() { + return 'border'; + } +} +class css_style_bordercolorright extends css_style_color { + public static function init($value) { + return new css_style_bordercolorright('border-color-right', $value); + } + public function consolidate_to() { + return 'border'; + } +} +class css_style_bordercolorbottom extends css_style_color { + public static function init($value) { + return new css_style_bordercolorbottom('border-color-bottom', $value); + } + public function consolidate_to() { + return 'border'; + } +} + +class css_style_borderwidthtop extends css_style_generic { + public static function init($value) { + return new css_style_borderwidthtop('border-width-top', $value); + } + public function consolidate_to() { + return 'border'; + } +} +class css_style_borderwidthleft extends css_style_generic { + public static function init($value) { + return new css_style_borderwidthleft('border-width-left', $value); + } + public function consolidate_to() { + return 'border'; + } +} +class css_style_borderwidthright extends css_style_generic { + public static function init($value) { + return new css_style_borderwidthright('border-width-right', $value); + } + public function consolidate_to() { + return 'border'; + } +} +class css_style_borderwidthbottom extends css_style_generic { + public static function init($value) { + return new css_style_borderwidthbottom('border-width-bottom', $value); + } + public function consolidate_to() { + return 'border'; + } +} + + +class css_style_borderstyletop extends css_style_generic { + public static function init($value) { + return new css_style_borderstyletop('border-style-top', $value); + } + public function consolidate_to() { + return 'border'; + } +} +class css_style_borderstyleleft extends css_style_generic { + public static function init($value) { + return new css_style_borderstyleleft('border-style-left', $value); + } + public function consolidate_to() { + return 'border'; + } +} +class css_style_borderstyleright extends css_style_generic { + public static function init($value) { + return new css_style_borderstyleright('border-style-right', $value); + } + public function consolidate_to() { + return 'border'; + } +} +class css_style_borderstylebottom extends css_style_generic { + public static function init($value) { + return new css_style_borderstylebottom('border-style-bottom', $value); + } + public function consolidate_to() { + return 'border'; + } +} + + class css_style_background extends css_style { public static function init($value) { // colour - image - repeat - attachment - position @@ -2046,13 +2169,18 @@ class css_style_background extends css_style { $color = $image = $repeat = $attachment = $position = null; foreach ($styles as $style) { switch ($style->get_name()) { - case 'background-color' : $color = $style->get_value(); break; + case 'background-color' : $color = css_style_color::shrink_value($style->get_value()); break; case 'background-image' : $image = $style->get_value(); break; case 'background-repeat' : $repeat = $style->get_value(); break; case 'background-attachment' : $attachment = $style->get_value(); break; case 'background-position' : $position = $style->get_value(); break; } } + + if ((is_null($image) || is_null($position) || is_null($repeat)) && ($image!= null || $position != null || $repeat != null)) { + return $styles; + } + $value = array(); if (!is_null($color)) $value[] .= $color; if (!is_null($image)) $value[] .= $image; diff --git a/lib/simpletest/testcsslib.php b/lib/simpletest/testcsslib.php index cff238c8cfc..66555ea64aa 100644 --- a/lib/simpletest/testcsslib.php +++ b/lib/simpletest/testcsslib.php @@ -39,6 +39,15 @@ class css_optimiser_test extends UnitTestCase { public function test_process() { $optimiser = new css_optimiser; + /* + $cssin = '.test {border:1px solid;border-color-top:#111; border-color-bottom: #222;border-color-left: #333;border-color-right:#444;}'; + $cssout = '.test{border:1px solid;border-color:#111 #444 # 222 #333;}'; + $actual = $optimiser->process($cssin); + debug(compact('cssin', 'cssout', 'actual')); + $this->assertEqual($cssout, $actual); + return; + /**/ + $this->check_background($optimiser); $this->check_borders($optimiser); $this->check_colors($optimiser); @@ -62,6 +71,10 @@ class css_optimiser_test extends UnitTestCase { $cssout = '.test{background:#123456;}'; $this->assertEqual($cssout, $optimiser->process($cssin)); + $cssin = '.test {background-image: url(\'test.png\');}'; + $cssout = '.test{background-image:url(\'test.png\');}'; + $this->assertEqual($cssout, $optimiser->process($cssin)); + $cssin = '.test {background: #123456 url(\'test.png\') no-repeat top left;}'; $cssout = '.test{background:#123456 url(\'test.png\') no-repeat top left;}'; $this->assertEqual($cssout, $optimiser->process($cssin)); @@ -71,13 +84,22 @@ class css_optimiser_test extends UnitTestCase { $this->assertEqual($cssout, $optimiser->process($cssin)); $cssin = '.test {background: url( \'test.png\' )}.test{background: bottom right}.test {background:#123456;}'; - $cssout = '.test{background:#123456 url(\'test.png\') bottom right;}'; + $cssout = '.test{background-image:url(\'test.png\');background-position:bottom right;background-color:#123456;}'; $this->assertEqual($cssout, $optimiser->process($cssin)); $cssin = '.test {background-color: #123456;background-repeat: repeat-x; background-position: 100% 0%;}'; - $cssout = '.test{background:#123456 repeat-x 100% 0%;}'; + $cssout = '.test{background-color:#123456;background-repeat:repeat-x;background-position:100% 0%;}'; $this->assertEqual($cssout, $optimiser->process($cssin)); + $cssin = '.tree_item.branch {background-image: url([[pix:t/expanded]]);background-position: 0 10%;background-repeat: no-repeat;} + .tree_item.branch.navigation_node {background-image:none;padding-left:0;}'; + $cssout = '.tree_item.branch{background:url([[pix:t/expanded]]) no-repeat 0 10%;} .tree_item.branch.navigation_node{background-image:none;padding-left:0;}'; + $this->assertEqual($cssout, $optimiser->process($cssin)); + + $cssin = '.block_tree .tree_item.emptybranch {background-image: url([[pix:t/collapsed_empty]]);background-position: 0% 5%;background-repeat: no-repeat;} + .block_tree .collapsed .tree_item.branch {background-image: url([[pix:t/collapsed]]);}'; + $cssout = '.block_tree .tree_item.emptybranch{background:url([[pix:t/collapsed_empty]]) no-repeat 0% 5%;} .block_tree .collapsed .tree_item.branch{background-image:url([[pix:t/collapsed]]);}'; + $this->assertEqual($cssout, $optimiser->process($cssin)); } protected function check_borders(css_optimiser $optimiser) { @@ -89,6 +111,14 @@ class css_optimiser_test extends UnitTestCase { $cssout = '.one{border:1px solid #FF0000;}'; $this->assertEqual($cssout, $optimiser->process($cssin)); + $cssin = '.one {border:1px solid;} .one {border:2px dotted #DDD;}'; + $cssout = '.one{border:2px dotted #DDDDDD;}'; + $this->assertEqual($cssout, $optimiser->process($cssin)); + + $cssin = '.one {border:2px dotted #DDD;}.one {border:1px solid;} '; + $cssout = '.one{border:1px solid #DDDDDD;}'; + $this->assertEqual($cssout, $optimiser->process($cssin)); + $cssin = '.one, .two {border:1px solid red;}'; $cssout = ".one, .two{border:1px solid #FF0000;}"; $this->assertEqual($cssout, $optimiser->process($cssin)); @@ -124,6 +154,14 @@ class css_optimiser_test extends UnitTestCase { $cssin = '.test {border:1px solid;border-color-top:#123456;}'; $cssout = '.test{border:1px solid;border-color-top:#123456;}'; $this->assertEqual($cssout, $optimiser->process($cssin)); + + $cssin = '.test {border:1px solid;border-color-top:#111; border-color-bottom: #222;border-color-left: #333;}'; + $cssout = '.test{border:1px solid;border-color-top:#111;border-color-bottom:#222;border-color-left:#333;}'; + $this->assertEqual($cssout, $optimiser->process($cssin)); + + $cssin = '.test {border:1px solid;border-color-top:#111; border-color-bottom: #222;border-color-left: #333;border-color-right:#444;}'; + $cssout = '.test{border:1px solid;border-color:#111 #444 #222 #333;}'; + $this->assertEqual($cssout, $optimiser->process($cssin)); } protected function check_colors(css_optimiser $optimiser) { @@ -293,7 +331,7 @@ class css_optimiser_test extends UnitTestCase { '.one {background-color:red;} .one {:blue;}', '.one {background-color:red;} .one {:#00F}', ); - $cssout = '.one{background-color:#F00;}'; + $cssout = '.one{background:#F00;}'; foreach ($cssin as $css) { $this->assertEqual($cssout, $optimiser->process($css)); } @@ -315,15 +353,15 @@ class css_optimiser_test extends UnitTestCase { $this->assertEqual($cssout, $optimiser->process($cssin)); $cssin = '.one {background-color:red;color;border-color:blue}'; - $cssout = '.one{background-color:#F00;border-color:#00F;}'; + $cssout = '.one{background:#F00;border-color:#00F;}'; $this->assertEqual($cssout, $optimiser->process($cssin)); $cssin = '{background-color:#123456;color:red;}{color:green;}'; - $cssout = "{color:#008000;background-color:#123456;}"; + $cssout = "{color:#008000;background:#123456;}"; $this->assertEqual($cssout, $optimiser->process($cssin)); $cssin = '.one {color:red;} {color:green;} .one {background-color:blue;}'; - $cssout = ".one{color:#F00;background-color:#00F;} {color:#008000;}"; + $cssout = ".one{color:#F00;background:#00F;} {color:#008000;}"; $this->assertEqual($cssout, $optimiser->process($cssin)); } @@ -366,7 +404,7 @@ class css_optimiser_test extends UnitTestCase { CSS; $cssout = <<