From 1eb2b51703e9e1fd78cca1ccf58e501faf28db1a Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Fri, 28 Jun 2013 16:49:02 +1200 Subject: [PATCH] MDL-39212 css: we now chunk and no longer separate --- lib/csslib.php | 103 +++++++++++++------------------------- lib/outputlib.php | 61 +++++++++++----------- lib/tests/csslib_test.php | 16 +++--- theme/styles.php | 44 ++++++++-------- 4 files changed, 94 insertions(+), 130 deletions(-) diff --git a/lib/csslib.php b/lib/csslib.php index cb6faf3b26c..50ca446e1d2 100644 --- a/lib/csslib.php +++ b/lib/csslib.php @@ -76,13 +76,6 @@ function css_store_css(theme_config $theme, $csspath, array $cssfiles, $chunk = $css = $theme->post_process(css_minify_css($cssfiles)); } - if ($chunk) { - // Chunk the CSS if requried. - $css = css_chunk_by_selector_count($css, $chunkurl); - } else { - $css = array($css); - } - clearstatcache(); if (!file_exists(dirname($csspath))) { @mkdir(dirname($csspath), $CFG->directorypermissions, true); @@ -92,22 +85,24 @@ function css_store_css(theme_config $theme, $csspath, array $cssfiles, $chunk = // the rename() should be more atomic than fwrite(). ignore_user_abort(true); - $files = count($css); - $count = 0; - foreach ($css as $content) { - if ($files > 1 && ($count+1) !== $files) { - // If there is more than one file and this is not the last file. - $filename = preg_replace('#\.css$#', '.'.$count.'.css', $csspath); + // First up write out the single file for all those using decent browsers. + css_write_file($csspath, $css); + + if ($chunk) { + // If we need to chunk the CSS for browsers that are sub-par. + $css = css_chunk_by_selector_count($css, $chunkurl); + $files = count($css); + $count = 1; + foreach ($css as $content) { + if ($count === $files) { + // If there is more than one file and this IS the last file. + $filename = preg_replace('#\.css$#', '.0.css', $csspath); + } else { + // If there is more than one file and this is not the last file. + $filename = preg_replace('#\.css$#', '.'.$count.'.css', $csspath); + } $count++; - } else { - $filename = $csspath; - } - if ($fp = fopen($filename.'.tmp', 'xb')) { - fwrite($fp, $content); - fclose($fp); - rename($filename.'.tmp', $filename); - @chmod($filename, $CFG->filepermissions); - @unlink($filename.'.tmp'); // just in case anything fails + css_write_file($filename, $content); } } @@ -117,6 +112,23 @@ function css_store_css(theme_config $theme, $csspath, array $cssfiles, $chunk = } } +/** + * Writes a CSS file. + * + * @param string $filename + * @param string $content + */ +function css_write_file($filename, $content) { + global $CFG; + if ($fp = fopen($filename.'.tmp', 'xb')) { + fwrite($fp, $content); + fclose($fp); + rename($filename.'.tmp', $filename); + @chmod($filename, $CFG->filepermissions); + @unlink($filename.'.tmp'); // just in case anything fails + } +} + /** * Takes CSS and chunks it if the number of selectors within it exceeds $maxselectors. * @@ -184,7 +196,7 @@ function css_chunk_by_selector_count($css, $importurl, $maxselectors = 4095, $bu $importcss = ''; $slashargs = strpos($importurl, '.php?') === false; $parts = count($css); - for ($i = 0; $i < $parts - 1; $i++) { + for ($i = 1; $i < $parts; $i++) { if ($slashargs) { $importcss .= "@import url({$importurl}/chunk{$i});\n"; } else { @@ -197,51 +209,6 @@ function css_chunk_by_selector_count($css, $importurl, $maxselectors = 4095, $bu return $css; } -/** - * Sends IE specific CSS - * - * In writing the CSS parser I have a theory that we could optimise the CSS - * then split it based upon the number of selectors to ensure we dont' break IE - * and that we include only as many sub-stylesheets as we require. - * Of course just a theory but may be fun to code. - * - * @param string $themename The name of the theme we are sending CSS for. - * @param string $rev The revision to ensure we utilise the cache. - * @param string $etag The revision to ensure we utilise the cache. - * @param bool $slasharguments - */ -function css_send_ie_css($themename, $rev, $etag, $slasharguments) { - global $CFG; - - $lifetime = 60*60*24*60; // 60 days only - the revision may get incremented quite often - - $relroot = preg_replace('|^http.?://[^/]+|', '', $CFG->wwwroot); - - $css = "/** Unfortunately IE6-9 does not support more than 4096 selectors in one CSS file, which means we have to use some ugly hacks :-( **/"; - if ($slasharguments) { - $css .= "\n@import url($relroot/styles.php/$themename/$rev/plugins);"; - $css .= "\n@import url($relroot/styles.php/$themename/$rev/parents);"; - $css .= "\n@import url($relroot/styles.php/$themename/$rev/theme);"; - } else { - $css .= "\n@import url($relroot/styles.php?theme=$themename&rev=$rev&type=plugins);"; - $css .= "\n@import url($relroot/styles.php?theme=$themename&rev=$rev&type=parents);"; - $css .= "\n@import url($relroot/styles.php?theme=$themename&rev=$rev&type=theme);"; - } - - header('Etag: "'.$etag.'"'); - header('Content-Disposition: inline; filename="styles.php"'); - header('Last-Modified: '. gmdate('D, d M Y H:i:s', time()) .' GMT'); - header('Expires: '. gmdate('D, d M Y H:i:s', time() + $lifetime) .' GMT'); - header('Pragma: '); - header('Cache-Control: public, max-age='.$lifetime); - header('Accept-Ranges: none'); - header('Content-Type: text/css; charset=utf-8'); - header('Content-Length: '.strlen($css)); - - echo $css; - die; -} - /** * Sends a cached CSS file * diff --git a/lib/outputlib.php b/lib/outputlib.php index 9b37027fca5..7afd582459e 100644 --- a/lib/outputlib.php +++ b/lib/outputlib.php @@ -579,26 +579,23 @@ class theme_config { * Returns the stylesheet URL of this editor content * * @param bool $encoded false means use & and true use & in URLs - * @return string + * @return moodle_url */ public function editor_css_url($encoded=true) { global $CFG; - $rev = theme_get_revision(); - if ($rev > -1) { + $url = new moodle_url("$CFG->httpswwwroot/theme/styles.php"); if (!empty($CFG->slasharguments)) { - $url = new moodle_url("$CFG->httpswwwroot/theme/styles.php"); $url->set_slashargument('/'.$this->name.'/'.$rev.'/editor', 'noparam', true); - return $url; } else { - $params = array('theme'=>$this->name,'rev'=>$rev, 'type'=>'editor'); - return new moodle_url($CFG->httpswwwroot.'/theme/styles.php', $params); + $url->params(array('theme'=>$this->name,'rev'=>$rev, 'type'=>'editor')); } } else { $params = array('theme'=>$this->name, 'type'=>'editor'); - return new moodle_url($CFG->httpswwwroot.'/theme/styles_debug.php', $params); + $url = new moodle_url($CFG->httpswwwroot.'/theme/styles_debug.php', $params); } + return $url; } /** @@ -661,31 +658,33 @@ class theme_config { if ($rev > -1) { $url = new moodle_url("$CFG->httpswwwroot/theme/styles.php"); - if (check_browser_version('MSIE', 5)) { - // We need to split the CSS files for IE - $urls[] = new moodle_url($url, array('theme' => $this->name,'rev' => $rev, 'type' => 'plugins', 'svg' => '0')); - $urls[] = new moodle_url($url, array('theme' => $this->name,'rev' => $rev, 'type' => 'parents', 'svg' => '0')); - $urls[] = new moodle_url($url, array('theme' => $this->name,'rev' => $rev, 'type' => 'theme', 'svg' => '0')); - } else { - if (!empty($CFG->slasharguments)) { - $slashargs = '/'.$this->name.'/'.$rev.'/all'; - if (!$svg) { - // We add a simple /_s to the start of the path. - // The underscore is used to ensure that it isn't a valid theme name. - $slashargs = '/_s'.$slashargs; - } - $url->set_slashargument($slashargs, 'noparam', true); - } else { - $params = array('theme' => $this->name,'rev' => $rev, 'type' => 'all'); - if (!$svg) { - // We add an SVG param so that we know not to serve SVG images. - // We do this because all modern browsers support SVG and this param will one day be removed. - $params['svg'] = '0'; - } - $url->params($params); + $separate = (check_browser_version('MSIE', 5) && !check_browser_version('MSIE', 10)); + if (!empty($CFG->slasharguments)) { + $slashargs = ''; + if (!$svg) { + // We add a simple /_s to the start of the path. + // The underscore is used to ensure that it isn't a valid theme name. + $slashargs .= '/_s'.$slashargs; } - $urls[] = $url; + $slashargs .= '/'.$this->name.'/'.$rev.'/all'; + if ($separate) { + $slashargs .= '/chunk0'; + } + $url->set_slashargument($slashargs, 'noparam', true); + } else { + $params = array('theme' => $this->name,'rev' => $rev, 'type' => 'all'); + if (!$svg) { + // We add an SVG param so that we know not to serve SVG images. + // We do this because all modern browsers support SVG and this param will one day be removed. + $params['svg'] = '0'; + } + if ($separate) { + $params['chunk'] = '0'; + } + $url->params($params); } + $urls[] = $url; + } else { // find out the current CSS and cache it now for 5 seconds // the point is to construct the CSS only once and pass it through the diff --git a/lib/tests/csslib_test.php b/lib/tests/csslib_test.php index 59134f1aa37..6b226d695c6 100644 --- a/lib/tests/csslib_test.php +++ b/lib/tests/csslib_test.php @@ -1096,7 +1096,7 @@ CSS; $this->assertArrayHasKey(2, $chunks); $this->assertEquals('a{}b{}', $chunks[0]); $this->assertEquals('c{}d{}', $chunks[1]); - $this->assertEquals("@import url(styles.php?type=test&chunk=0);\n@import url(styles.php?type=test&chunk=1);\ne{}f{}", $chunks[2]); + $this->assertEquals("@import url(styles.php?type=test&chunk=1);\n@import url(styles.php?type=test&chunk=2);\ne{}f{}", $chunks[2]); // Test with an odd number of styles. $css = 'a{}b{}c{}d{}e{}'; @@ -1108,7 +1108,7 @@ CSS; $this->assertArrayHasKey(2, $chunks); $this->assertEquals('a{}b{}', $chunks[0]); $this->assertEquals('c{}d{}', $chunks[1]); - $this->assertEquals("@import url(styles.php?type=test&chunk=0);\n@import url(styles.php?type=test&chunk=1);\ne{}", $chunks[2]); + $this->assertEquals("@import url(styles.php?type=test&chunk=1);\n@import url(styles.php?type=test&chunk=2);\ne{}", $chunks[2]); // Test buffering. Set a buffer that will reduce the effective sheet size back to two. $css = 'a{}b{}c{}d{}e{}f{}'; @@ -1120,7 +1120,7 @@ CSS; $this->assertArrayHasKey(2, $chunks); $this->assertEquals('a{}b{}', $chunks[0]); $this->assertEquals('c{}d{}', $chunks[1]); - $this->assertEquals("@import url(styles.php?type=test&chunk=0);\n@import url(styles.php?type=test&chunk=1);\ne{}f{}", $chunks[2]); + $this->assertEquals("@import url(styles.php?type=test&chunk=1);\n@import url(styles.php?type=test&chunk=2);\ne{}f{}", $chunks[2]); // Test well placed commas. $css = 'a,b{}c,d{}e,f{}'; @@ -1132,7 +1132,7 @@ CSS; $this->assertArrayHasKey(2, $chunks); $this->assertEquals('a,b{}', $chunks[0]); $this->assertEquals('c,d{}', $chunks[1]); - $this->assertEquals("@import url(styles.php?type=test&chunk=0);\n@import url(styles.php?type=test&chunk=1);\ne,f{}", $chunks[2]); + $this->assertEquals("@import url(styles.php?type=test&chunk=1);\n@import url(styles.php?type=test&chunk=2);\ne,f{}", $chunks[2]); // Test unfortunately placed commas. $css = 'a{}b,c{color:red;}d{}e{}f{}'; @@ -1144,7 +1144,7 @@ CSS; $this->assertArrayHasKey(2, $chunks); $this->assertEquals('a{}b{color:red;}', $chunks[0]); $this->assertEquals('c{color:red;}d{}', $chunks[1]); - $this->assertEquals("@import url(styles.php?type=test&chunk=0);\n@import url(styles.php?type=test&chunk=1);\ne{}f{}", $chunks[2]); + $this->assertEquals("@import url(styles.php?type=test&chunk=1);\n@import url(styles.php?type=test&chunk=2);\ne{}f{}", $chunks[2]); // Test unfortunate CSS. $css = 'a,b,c,d,e,f{color:red;}'; @@ -1156,7 +1156,7 @@ CSS; $this->assertArrayHasKey(2, $chunks); $this->assertEquals('a,b{color:red;}', $chunks[0]); $this->assertEquals('c,d{color:red;}', $chunks[1]); - $this->assertEquals("@import url(styles.php?type=test&chunk=0);\n@import url(styles.php?type=test&chunk=1);\ne,f{color:red;}", $chunks[2]); + $this->assertEquals("@import url(styles.php?type=test&chunk=1);\n@import url(styles.php?type=test&chunk=2);\ne,f{color:red;}", $chunks[2]); // Test to make sure invalid CSS isn't totally ruined. $css = 'a{},,,e{},'; @@ -1168,7 +1168,7 @@ CSS; $this->assertArrayHasKey(0, $chunks); $this->assertArrayHasKey(1, $chunks); $this->assertEquals('a{},{}', $chunks[0]); - $this->assertEquals("@import url(styles.php?type=test&chunk=0);\n,e{}/** Error chunking CSS **/", $chunks[1]); + $this->assertEquals("@import url(styles.php?type=test&chunk=1);\n,e{}/** Error chunking CSS **/", $chunks[1]); // Test utter crap CSS to make sure we don't loop to our deaths. $css = 'a,b,c,d,e,f'; @@ -1180,7 +1180,7 @@ CSS; $this->assertArrayHasKey(2, $chunks); $this->assertEquals('a,b/** Error chunking CSS **/', $chunks[0]); $this->assertEquals('c,d/** Error chunking CSS **/', $chunks[1]); - $this->assertEquals("@import url(styles.php?type=test&chunk=0);\n@import url(styles.php?type=test&chunk=1);\ne,f", $chunks[2]); + $this->assertEquals("@import url(styles.php?type=test&chunk=1);\n@import url(styles.php?type=test&chunk=2);\ne,f", $chunks[2]); // Test another death situation to make sure we're invincible. $css = 'a,,,,,e'; $chunks = css_chunk_by_selector_count($css, 'styles.php?type=test', 2, 0); diff --git a/theme/styles.php b/theme/styles.php index eeab1ef7904..de426695fa0 100644 --- a/theme/styles.php +++ b/theme/styles.php @@ -65,7 +65,12 @@ if ($slashargument = min_get_slash_argument()) { $usesvg = (bool)min_optional_param('svg', '1', 'INT'); } -if (!in_array($type, array('all', 'ie', 'editor', 'plugins', 'parents', 'theme'))) { +if ($type === 'editor') { + // The editor CSS is never chunked. + $chunk = null; +} else if ($type === 'all') { + // We're fine. +} else { header('HTTP/1.0 404 not found'); die('Theme was not found, sorry.'); } @@ -79,10 +84,6 @@ if (file_exists("$CFG->dirroot/theme/$themename/config.php")) { die('Theme was not found, sorry.'); } -if ($type === 'ie') { - css_send_ie_css($themename, $rev, $etag, !empty($slashargument)); -} - $candidatedir = "$CFG->cachedir/theme/$themename/css"; $etag = "$themename/$rev/$type"; $candidatename = $type; @@ -91,6 +92,7 @@ if (!$usesvg) { $candidatedir .= '/nosvg'; $etag .= '/nosvg'; } + if ($chunk !== null) { $etag .= '/chunk'.$chunk; $candidatename .= '.'.$chunk; @@ -140,21 +142,21 @@ if ($type === 'editor') { $css = $theme->css_files(); $allfiles = array(); $relroot = preg_replace('|^http.?://[^/]+|', '', $CFG->wwwroot); - foreach ($css as $key=>$value) { - if (!empty($slashargument)) { - if ($usesvg) { - $chunkurl = "{$relroot}/theme/styles.php/{$themename}/{$rev}/{$key}"; - } else { - $chunkurl = "{$relroot}/theme/styles.php/_s/{$themename}/{$rev}/{$key}"; - } + if (!empty($slashargument)) { + if ($usesvg) { + $chunkurl = "{$relroot}/theme/styles.php/{$themename}/{$rev}/all"; } else { - if ($usesvg) { - $chunkurl = "{$relroot}/theme/styles.php?theme={$themename}&rev={$rev}&type={$key}"; - } else { - $chunkurl = "{$relroot}/theme/styles.php?theme={$themename}&rev={$rev}&type={$key}&svg=0"; - } + $chunkurl = "{$relroot}/theme/styles.php/_s/{$themename}/{$rev}/all"; } - $cssfiles = array(); + } else { + if ($usesvg) { + $chunkurl = "{$relroot}/theme/styles.php?theme={$themename}&rev={$rev}&type=all"; + } else { + $chunkurl = "{$relroot}/theme/styles.php?theme={$themename}&rev={$rev}&type=all&svg=0"; + } + } + $cssfiles = array(); + foreach ($css as $key=>$value) { foreach($value as $val) { if (is_array($val)) { foreach ($val as $k=>$v) { @@ -164,12 +166,8 @@ if ($type === 'editor') { $cssfiles[] = $val; } } - $cssfile = "$basedir/$key.css"; - css_store_css($theme, $cssfile, $cssfiles, true, $chunkurl); - $allfiles = array_merge($allfiles, $cssfiles); } - $cssfile = "$basedir/all.css"; - css_store_css($theme, $cssfile, $allfiles); + css_store_css($theme, "$basedir/all.css", $cssfiles, true, $chunkurl); } // verify nothing failed in cache file creation