r71767 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r71766‎ | r71767 | r71768 >
Date:23:36, 26 August 2010
Author:tparscal
Status:resolved (Comments)
Tags:
Comment:
Improved performance of CSSMin::minify by using arrays with str_replace and preg_replace, switched to using consistent delimiters for regex (/), added modified time based versioning for CSS URLs of local files, and adjusted the maximum size of a file to be embedded to push to the edge of IE's 32k limit.
Modified paths:
  • /branches/resourceloader/phase3/includes/CSSMin.php (modified) (history)

Diff [purge]

Index: branches/resourceloader/phase3/includes/CSSMin.php
@@ -4,52 +4,79 @@
55
66 /* Constants */
77
8 - const MAX_EMBED_SIZE = 1024;
 8+ /**
 9+ * Maximum file size to still qualify for in-line embedding as a data-URI
 10+ *
 11+ * 24,576 is used because Internet Explorer has a 32,768 byte limit for data URIs, which when base64 encoded will
 12+ * result in a 1/4 increase in size.
 13+ */
 14+ const EMBED_SIZE_LIMIT = 24576;
915
1016 /* Static Methods */
1117
1218 /*
1319 * Remaps CSS URL paths and automatically embeds data URIs for URL rules preceded by an /* @embed * / comment
 20+ *
 21+ * @param $source string CSS data to remap
 22+ * @param $path string File path where the source was read from
1423 */
1524 public static function remap( $source, $path ) {
16 - // Pre-process for URL rewriting
 25+ $pattern = '/((?<embed>\s*\/\*\s*\@embed\s*\*\/)(?<rule>[^\;\}]*))?url\((?<file>[^)]*)\)(?<extra>[^;]*)[\;]?/';
1726 $offset = 0;
18 - while ( preg_match(
19 - '/((?<embed>\s*\/\*\s*\@embed\s*\*\/)(?<rule>[^\;\}]*))?url\((?<file>[^)]*)\)(?<extra>[^;]*)[\;]?/',
20 - $source, $match, PREG_OFFSET_CAPTURE, $offset
21 - ) ) {
22 - $url = $path . '/' . trim( $match['file'][0], "'\"" );
23 - $file = preg_replace( '/([^\?]*)(.*)/', '$1', $url );
24 - $embed = $match['embed'][0];
25 - $rule = $match['rule'][0];
26 - $extra = $match['extra'][0];
27 - if ( $match['embed'][1] > 0 && file_exists( $file ) && filesize( $file ) <= self::MAX_EMBED_SIZE ) {
28 - // If we ever get to PHP 5.3, we should use the Fileinfo extension instead of mime_content_type
29 - $type = mime_content_type( $file );
30 - $data = rtrim( base64_encode( file_get_contents( $file ) ), '=' );
31 - $replacement = "{$rule}url(data:{$type};base64,{$data}){$extra};{$rule}url({$url}){$extra}!ie;";
32 - } else {
33 - $replacement = "{$embed}{$rule}url({$url}){$extra};";
 27+ while ( preg_match( $pattern, $source, $match, PREG_OFFSET_CAPTURE, $offset ) ) {
 28+ // Remove single or double quotes
 29+ $url = trim( $match['file'][0], "'\"" );
 30+ // Only proceed if the URL is to a local file
 31+ if ( !preg_match( '/^[a-zA-Z]*\:\/\//', $url ) ) {
 32+ // Shortcuts
 33+ $embed = $match['embed'][0];
 34+ $rule = $match['rule'][0];
 35+ $extra = $match['extra'][0];
 36+ // Strip query string from URL
 37+ $file = "{$path}/" . preg_replace( '/([^\?]*)(.*)/', '$1', $url );
 38+ // Only proceed if we can access the file
 39+ if ( file_exists( $file ) ) {
 40+ // Add version parameter as a time-stamp in ISO 8601 format, using Z for the timezone, meaning GMT
 41+ $url = "{$file}?" . gmdate( 'Y-m-d\TH:i:s\Z', round( filemtime( $file ), -2 ) );
 42+ // Detect when URLs were preceeded with embed tags, and also verify file size is below the limit
 43+ if ( $match['embed'][1] > 0 && filesize( $file ) < self::EMBED_SIZE_LIMIT ) {
 44+ // If we ever get to PHP 5.3, we should use the Fileinfo extension instead of mime_content_type
 45+ $type = mime_content_type( $file );
 46+ // Strip off any trailing = symbols (makes browsers freak out)
 47+ $data = rtrim( base64_encode( file_get_contents( $file ) ), '=' );
 48+ // Build 2 CSS properties; one which uses a base64 encoded data URI in place of the @embed
 49+ // comment to try and retain line-number integrity , and the other with a remapped an versioned
 50+ // URL and an Internet Explorer hack making it ignored in all browsers that support data URIs
 51+ $replacement = "{$rule}url(data:{$type};base64,{$data}){$extra};{$rule}url({$url}){$extra}!ie;";
 52+ } else {
 53+ // Build a CSS property with a remapped and versioned URL
 54+ $replacement = "{$embed}{$rule}url({$url}){$extra};";
 55+ }
 56+ // Perform replacement on the source
 57+ $source = substr_replace( $source, $replacement, $match[0][1], strlen( $match[0][0] ) );
 58+ // Move the offset to the end of the replacement in the source
 59+ $offset = $match[0][1] + strlen( $replacement );
 60+ continue;
 61+ }
3462 }
35 - $source = substr_replace( $source, $replacement, $match[0][1], strlen( $match[0][0] ) );
36 - $offset = $match[0][1] + strlen( $replacement );
 63+ // Move the offset to the end of the match, leaving it alone
 64+ $offset = $match[0][1] + strlen( $match[0][0] );
3765 }
3866 return $source;
3967 }
4068
4169 /*
42 - * As seen at http://www.lateralcode.com/css-minifier/
 70+ * Removes whitespace from CSS data
 71+ *
 72+ * @param $source string CSS data to minify
4373 */
4474 public static function minify( $css ) {
45 - $css = preg_replace( '#\s+#', ' ', $css );
46 - $css = preg_replace( '#/\*.*?\*/#s', '', $css );
47 - $css = str_replace( '; ', ';', $css );
48 - $css = str_replace( ': ', ':', $css );
49 - $css = str_replace( ' {', '{', $css );
50 - $css = str_replace( '{ ', '{', $css );
51 - $css = str_replace( ', ', ',', $css );
52 - $css = str_replace( '} ', '}', $css );
53 - $css = str_replace( ';}', '}', $css );
54 - return trim( $css );
 75+ return trim(
 76+ str_replace(
 77+ array( '; ', ': ', ' {', '{ ', ', ', '} ', ';}' ),
 78+ array( ';', ':', '{', '{', ',', '}', '}' ),
 79+ preg_replace( array( '/\s+/', '/\/\*.*?\*\//s' ), array( ' ', '' ), $css )
 80+ )
 81+ );
5582 }
5683 }
\ No newline at end of file

Follow-up revisions

RevisionCommit summaryAuthorDate
r72365Fixes bug introduced r71767 - the stripping of = from the right side of data ...tparscal10:20, 4 September 2010

Comments

#Comment by Nikerabbit (talk | contribs)   10:03, 4 September 2010
// Strip off any trailing = symbols (makes browsers freak out)

Explanation needed. This breaks inline images in Webkit-based browsers.

#Comment by Trevor Parscal (WMF) (talk | contribs)   10:29, 4 September 2010

I misdiagnosed a problem with FF rendering data URLs apparently.

Status & tagging log