r82000 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r81999‎ | r82000 | r82001 >
Date:22:57, 11 February 2011
Author:tparscal
Status:resolved (Comments)
Tags:
Comment:
Resolves bug #27328 by supporting URL rewriting for CSS that comes from the Wiki, such as user and site scripts.
Modified paths:
  • /trunk/phase3/includes/libs/CSSMin.php (modified) (history)
  • /trunk/phase3/includes/resourceloader/ResourceLoaderWikiModule.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/resourceloader/ResourceLoaderWikiModule.php
@@ -88,6 +88,7 @@
8989 }
9090
9191 public function getStyles( ResourceLoaderContext $context ) {
 92+ global $wgScriptPath;
9293
9394 $styles = array();
9495 foreach ( $this->getPages( $context ) as $titleText => $options ) {
@@ -106,6 +107,7 @@
107108 if ( $this->getFlip( $context ) ) {
108109 $style = CSSJanus::transform( $style, true, false );
109110 }
 111+ $style = CSSMin::remap( $style, false, $wgScriptPath, true );
110112 if ( !isset( $styles[$media] ) ) {
111113 $styles[$media] = '';
112114 }
Index: trunk/phase3/includes/libs/CSSMin.php
@@ -36,7 +36,7 @@
3737 * which when base64 encoded will result in a 1/3 increase in size.
3838 */
3939 const EMBED_SIZE_LIMIT = 24576;
40 - const URL_REGEX = 'url\(\s*[\'"]?(?P<file>[^\?\)\:\'"]*)\??[^\)\'"]*[\'"]?\s*\)';
 40+ const URL_REGEX = 'url\(\s*[\'"]?(?P<file>[^\?\)\'"]*)\??[^\)\'"]*[\'"]?\s*\)';
4141
4242 /* Protected Static Members */
4343
@@ -79,6 +79,32 @@
8080 return $files;
8181 }
8282
 83+ protected static function getMimeType( $file ) {
 84+ $realpath = realpath( $file );
 85+ // Try a couple of different ways to get the mime-type of a file, in order of
 86+ // preference
 87+ if (
 88+ $realpath
 89+ && function_exists( 'finfo_file' )
 90+ && function_exists( 'finfo_open' )
 91+ && defined( 'FILEINFO_MIME_TYPE' )
 92+ ) {
 93+ // As of PHP 5.3, this is how you get the mime-type of a file; it uses the Fileinfo
 94+ // PECL extension
 95+ return finfo_file( finfo_open( FILEINFO_MIME_TYPE ), $realpath );
 96+ } else if ( function_exists( 'mime_content_type' ) ) {
 97+ // Before this was deprecated in PHP 5.3, this was how you got the mime-type of a file
 98+ return mime_content_type( $file );
 99+ } else {
 100+ // Worst-case scenario has happened, use the file extension to infer the mime-type
 101+ $ext = strtolower( pathinfo( $file, PATHINFO_EXTENSION ) );
 102+ if ( isset( self::$mimeTypes[$ext] ) ) {
 103+ return self::$mimeTypes[$ext];
 104+ }
 105+ }
 106+ return false;
 107+ }
 108+
83109 /**
84110 * Remaps CSS URL paths and automatically embeds data URIs for URL rules
85111 * preceded by an /* @embed * / comment
@@ -94,63 +120,56 @@
95121 self::URL_REGEX . '(?P<post>[^;]*)[\;]?/';
96122 $offset = 0;
97123 while ( preg_match( $pattern, $source, $match, PREG_OFFSET_CAPTURE, $offset ) ) {
 124+ // Skip absolute URIs
 125+ if ( preg_match( '/^https?:\/\//', $match['file'][0] ) ) {
 126+ // Move the offset to the end of the match, leaving it alone
 127+ $offset = $match[0][1] + strlen( $match[0][0] );
 128+ continue;
 129+ }
98130 // Shortcuts
99131 $embed = $match['embed'][0];
100132 $pre = $match['pre'][0];
101133 $post = $match['post'][0];
 134+ $url = "{$remote}/{$match['file'][0]}";
102135 $file = "{$local}/{$match['file'][0]}";
103 - $url = "{$remote}/{$match['file'][0]}";
104 - // Only proceed if we can access the file
105 - if ( file_exists( $file ) ) {
 136+ $replacement = false;
 137+ if ( $local !== false && file_exists( $file ) ) {
106138 // Add version parameter as a time-stamp in ISO 8601 format,
107139 // using Z for the timezone, meaning GMT
108140 $url .= '?' . gmdate( 'Y-m-d\TH:i:s\Z', round( filemtime( $file ), -2 ) );
109 - // If we the mime-type can't be determined, no embedding will take place
110 - $type = false;
111 - $realpath = realpath( $file );
112 - // Try a couple of different ways to get the mime-type of a file,
113 - // in order of preference
114 - if ( $realpath
115 - && function_exists( 'finfo_file' ) && function_exists( 'finfo_open' )
116 - && defined( 'FILEINFO_MIME_TYPE' ) )
117 - {
118 - // As of PHP 5.3, this is how you get the mime-type of a file;
119 - // it uses the Fileinfo PECL extension
120 - $type = finfo_file( finfo_open( FILEINFO_MIME_TYPE ), $realpath );
121 - } else if ( function_exists( 'mime_content_type' ) ) {
122 - // Before this was deprecated in PHP 5.3,
123 - // this used to be how you get the mime-type of a file
124 - $type = mime_content_type( $file );
125 - } else {
126 - // Worst-case scenario has happened,
127 - // use the file extension to infer the mime-type
128 - $ext = strtolower( pathinfo( $file, PATHINFO_EXTENSION ) );
129 - if ( isset( self::$mimeTypes[$ext] ) ) {
130 - $type = self::$mimeTypes[$ext];
 141+ // Embedding requires a bit of extra processing, so let's skip that if we can
 142+ if ( $embed ) {
 143+ $type = self::getMimeType( $file );
 144+ // Detect when URLs were preceeded with embed tags, and also verify file size is
 145+ // below the limit
 146+ if (
 147+ $type
 148+ && $match['embed'][1] > 0
 149+ && filesize( $file ) < self::EMBED_SIZE_LIMIT
 150+ ) {
 151+ // Strip off any trailing = symbols (makes browsers freak out)
 152+ $data = base64_encode( file_get_contents( $file ) );
 153+ // Build 2 CSS properties; one which uses a base64 encoded data URI in place
 154+ // of the @embed comment to try and retain line-number integrity, and the
 155+ // other with a remapped an versioned URL and an Internet Explorer hack
 156+ // making it ignored in all browsers that support data URIs
 157+ $replacement = "{$pre}url(data:{$type};base64,{$data}){$post};";
 158+ $replacement .= "{$pre}url({$url}){$post}!ie;";
131159 }
132160 }
133 - // Detect when URLs were preceeded with embed tags,
134 - // and also verify file size is below the limit
135 - if ( $embed && $type && $match['embed'][1] > 0
136 - && filesize( $file ) < self::EMBED_SIZE_LIMIT )
137 - {
138 - // Strip off any trailing = symbols (makes browsers freak out)
139 - $data = base64_encode( file_get_contents( $file ) );
140 - // Build 2 CSS properties; one which uses a base64 encoded data URI
141 - // in place of the @embed comment to try and retain line-number integrity,
142 - // and the other with a remapped an versioned URL and an Internet Explorer
143 - // hack making it ignored in all browsers that support data URIs
144 - $replacement = "{$pre}url(data:{$type};base64,{$data}){$post};";
145 - $replacement .= "{$pre}url({$url}){$post}!ie;";
146 - } else {
147 - // Build a CSS property with a remapped and versioned URL,
148 - // preserving comment for debug mode
149 - $replacement = "{$embed}{$pre}url({$url}){$post};";
 161+ if ( $replacement === false ) {
 162+ // Assume that all paths are relative to $remote, and make them absolute
 163+ $replacement = "{$embed}{$pre}url({$url}){$post};";
150164 }
151 -
 165+ } else if ( $local === false ) {
 166+ // Assume that all paths are relative to $remote, and make them absolute
 167+ $replacement = "{$embed}{$pre}url({$url}){$post};";
 168+ }
 169+ if ( $replacement !== false ) {
152170 // Perform replacement on the source
153 - $source = substr_replace( $source,
154 - $replacement, $match[0][1], strlen( $match[0][0] ) );
 171+ $source = substr_replace(
 172+ $source, $replacement, $match[0][1], strlen( $match[0][0] )
 173+ );
155174 // Move the offset to the end of the replacement in the source
156175 $offset = $match[0][1] + strlen( $replacement );
157176 continue;
@@ -160,7 +179,7 @@
161180 }
162181 return $source;
163182 }
164 -
 183+
165184 /**
166185 * Removes whitespace from CSS data
167186 *

Follow-up revisions

RevisionCommit summaryAuthorDate
r82156Resolves remaining issues in r82000 with remapping relative CSS URLs that con...tparscal00:49, 15 February 2011
r822201.17wmf1: MFT r80495, r80765, r81177, r82000, r82155, r82156, r82191, r82200,...catrope07:23, 16 February 2011
r85151MFT: r82000, r82004, r82020, r82025, r82038, r82039, r82048, r82070, r82081, ...demon20:39, 1 April 2011

Comments

#Comment by Bryan (talk | contribs)   23:00, 11 February 2011

We have MimeMagic::singleton()->guessMimeType(), you don't need to write that function yourself.

#Comment by Trevor Parscal (WMF) (talk | contribs)   18:17, 14 February 2011

That would be a reasonable thing to try to use first, if it's there, but this is a stand-alone library, and we need to keep it that way.

#Comment by Bryan (talk | contribs)   23:00, 14 February 2011

Oh ok that makes sense. I hadn't realized it could be used standalone.

#Comment by Trevor Parscal (WMF) (talk | contribs)   00:38, 15 February 2011

That's the purpose of the libs folder.

#Comment by TheDJ (talk | contribs)   12:20, 12 February 2011

Also as reported by Krinkle:

@import url("?title=MediaWiki:Common.css/WikiTable.css&action=raw&ctype=text/css") screen;

Which first tried to access:

Willl now access:

  • /windex?title=MediaWiki:Common.css/WikiTable.css&action=raw&ctype=text/css

We should probably use a path concatenation function for safety when we do stuff like this....

Status & tagging log