r72695 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r72694‎ | r72695 | r72696 >
Date:00:21, 10 September 2010
Author:tparscal
Status:resolved (Comments)
Tags:
Comment:
Added media-type support for static and dynamic ResourceLoader requests.
Modified paths:
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/includes/ResourceLoader.php (modified) (history)
  • /trunk/phase3/includes/ResourceLoaderContext.php (modified) (history)
  • /trunk/phase3/includes/ResourceLoaderModule.php (modified) (history)
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)
  • /trunk/phase3/resources/Resources.php (modified) (history)
  • /trunk/phase3/resources/mediawiki/mediawiki.js (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/ResourceLoaderContext.php
@@ -33,6 +33,7 @@
3434 protected $skin;
3535 protected $debug;
3636 protected $only;
 37+ protected $media;
3738 protected $hash;
3839
3940 /* Methods */
@@ -49,6 +50,7 @@
5051 $this->skin = $request->getVal( 'skin' );
5152 $this->debug = $request->getVal( 'debug' ) === 'true' || $request->getBool( 'debug' );
5253 $this->only = $request->getVal( 'only' );
 54+ $this->media = $request->getVal( 'media', 'all' );
5355
5456 // Fallback on system defaults
5557 if ( !$this->language ) {
@@ -95,6 +97,10 @@
9698 public function getOnly() {
9799 return $this->only;
98100 }
 101+
 102+ public function getMedia() {
 103+ return $this->media;
 104+ }
99105
100106 public function shouldIncludeScripts() {
101107 return is_null( $this->only ) || $this->only === 'scripts';
@@ -110,6 +116,7 @@
111117
112118 public function getHash() {
113119 return isset( $this->hash ) ?
114 - $this->hash : $this->hash = implode( '|', array( $this->language, $this->skin, $this->debug ) );
 120+ $this->hash : $this->hash =
 121+ implode( '|', array( $this->language, $this->skin, $this->debug, $this->only, $this->media ) );
115122 }
116123 }
Index: trunk/phase3/includes/OutputPage.php
@@ -2198,7 +2198,7 @@
21992199 global $wgUser, $wgRequest, $wgLang;
22002200
22012201 if ( $sk->commonPrintStylesheet() ) {
2202 - $this->addStyle( 'common/wikiprintable.css', 'print' );
 2202+ $this->addModuleStyles( 'mediawiki.legacy.wikiprintable' );
22032203 }
22042204 $sk->setupUserCss( $this );
22052205
@@ -2280,7 +2280,7 @@
22812281 return $ret;
22822282 }
22832283
2284 - static function makeResourceLoaderLink( $skin, $modules, $only ) {
 2284+ static function makeResourceLoaderLink( $skin, $modules, $only, $media = 'all' ) {
22852285 global $wgUser, $wgLang, $wgRequest;
22862286 // TODO: Should this be a static function of ResourceLoader instead?
22872287 $query = array(
@@ -2292,7 +2292,8 @@
22932293 );
22942294 // Automatically select style/script elements
22952295 if ( $only === 'styles' ) {
2296 - return Html::linkedStyle( wfAppendQuery( wfScript( 'load' ), $query ) );
 2296+ $query['media'] = $media;
 2297+ return Html::linkedStyle( wfAppendQuery( wfScript( 'load' ), $query ), $media );
22972298 } else {
22982299 return Html::linkedScript( wfAppendQuery( wfScript( 'load' ), $query ) );
22992300 }
@@ -2488,11 +2489,15 @@
24892490 // Support individual script requests in debug mode
24902491 if ( $wgRequest->getBool( 'debug' ) && $wgRequest->getVal( 'debug' ) !== 'false' ) {
24912492 foreach ( $this->getModuleStyles() as $name ) {
2492 - $tags[] = self::makeResourceLoaderLink( $sk, $name, 'styles' );
 2493+ $tags[] = self::makeResourceLoaderLink( $sk, $name, 'styles', 'all' );
 2494+ $tags[] = self::makeResourceLoaderLink( $sk, $name, 'styles', 'screen' );
 2495+ $tags[] = self::makeResourceLoaderLink( $sk, $name, 'styles', 'print' );
24932496 }
24942497 } else {
24952498 if ( count( $this->getModuleStyles() ) ) {
2496 - $tags[] = self::makeResourceLoaderLink( $sk, $this->getModuleStyles(), 'styles' );
 2499+ $tags[] = self::makeResourceLoaderLink( $sk, $this->getModuleStyles(), 'styles', 'all' );
 2500+ $tags[] = self::makeResourceLoaderLink( $sk, $this->getModuleStyles(), 'styles', 'screen' );
 2501+ $tags[] = self::makeResourceLoaderLink( $sk, $this->getModuleStyles(), 'styles', 'print' );
24972502 }
24982503 }
24992504
Index: trunk/phase3/includes/ResourceLoader.php
@@ -287,16 +287,20 @@
288288 }
289289
290290 // Styles
291 - $styles = '';
 291+ $styles = array();
292292
293293 if (
294294 $context->shouldIncludeStyles() &&
295 - ( $styles .= self::$modules[$name]->getStyle( $context ) ) !== ''
 295+ ( count( $styles = self::$modules[$name]->getStyles( $context ) ) )
296296 ) {
297 - if ( self::$modules[$name]->getFlip( $context ) ) {
298 - $styles = self::filter( 'flip-css', $styles );
 297+ foreach ( $styles as $media => $style ) {
 298+ if ( self::$modules[$name]->getFlip( $context ) ) {
 299+ $styles[$media] = self::filter( 'flip-css', $style );
 300+ }
 301+ if ( !$context->getDebug() ) {
 302+ $styles[$media] = self::filter( 'minify-css', $style );
 303+ }
299304 }
300 - $styles = $context->getDebug() ? $styles : self::filter( 'minify-css', $styles );
301305 }
302306
303307 // Messages
@@ -304,14 +308,16 @@
305309
306310 // Output
307311 if ( $context->getOnly() === 'styles' ) {
308 - echo $styles;
 312+ if ( isset( $styles[$context->getMedia()] ) ) {
 313+ echo $styles[$context->getMedia()];
 314+ }
309315 } else if ( $context->getOnly() === 'scripts' ) {
310316 echo $scripts;
311317 } else if ( $context->getOnly() === 'messages' ) {
312318 echo "mediaWiki.msg.set( $messages );\n";
313319 } else {
314 - $styles = Xml::escapeJsString( $styles );
315 - echo "mediaWiki.loader.implement( '$name', function() {{$scripts}},\n'$styles',\n$messages );\n";
 320+ $styles = FormatJson::encode( $styles );
 321+ echo "mediaWiki.loader.implement( '$name', function() {{$scripts}},\n$styles,\n$messages );\n";
316322 }
317323 }
318324
Index: trunk/phase3/includes/SkinTemplate.php
@@ -107,9 +107,7 @@
108108 * @param $out OutputPage
109109 */
110110 function setupSkinUserCss( OutputPage $out ){
111 - $out->addModuleStyles( 'mediawiki.legacy.shared' );
112 - // ResourceLoader does not support CSS media types yet, so we must include this on it's own the old way
113 - $out->addStyle( 'common/commonPrint.css', 'print' );
 111+ $out->addModuleStyles( array( 'mediawiki.legacy.shared', 'mediawiki.legacy.commonPrint' ) );
114112 }
115113
116114 /**
Index: trunk/phase3/includes/ResourceLoaderModule.php
@@ -96,9 +96,9 @@
9797 * Get all CSS for this module for a given skin.
9898 *
9999 * @param $context ResourceLoaderContext object
100 - * @return String: CSS
 100+ * @return array: strings of CSS keyed by media type
101101 */
102 - public abstract function getStyle( ResourceLoaderContext $context );
 102+ public abstract function getStyles( ResourceLoaderContext $context );
103103
104104 /**
105105 * Get the messages needed for this module.
@@ -191,9 +191,11 @@
192192 * // Non-raw module options
193193 * 'dependencies' => 'module' | array( 'module1', 'module2' ... )
194194 * 'loaderScripts' => 'dir/loader.js' | array( 'dir/loader1.js', 'dir/loader2.js' ... ),
195 - * 'styles' => 'dir/file.css' | array( 'dir/file1.css', 'dir/file2.css' ... ),
 195+ * 'styles' => 'dir/file.css' | array( 'dir/file1.css', 'dir/file2.css' ... ), |
 196+ * array( 'dir/file1.css' => array( 'media' => 'print' ) ),
196197 * 'skinStyles' => array(
197 - * '[skin name]' => 'dir/skin.css' | '[skin name]' => array( 'dir/skin1.css', 'dir/skin2.css' ... )
 198+ * '[skin name]' => 'dir/skin.css' | array( 'dir/skin1.css', 'dir/skin2.css' ... ) |
 199+ * array( 'dir/file1.css' => array( 'media' => 'print' )
198200 * ...
199201 * ),
200202 * 'messages' => array( 'message1', 'message2' ... ),
@@ -362,12 +364,28 @@
363365 return $retval;
364366 }
365367
366 - public function getStyle( ResourceLoaderContext $context ) {
367 - $style = $this->getPrimaryStyle() . "\n" . $this->getSkinStyle( $context->getSkin() );
368 -
369 - // Extract and store the list of referenced files
370 - $files = CSSMin::getLocalFileReferences( $style );
371 -
 368+ public function getStyles( ResourceLoaderContext $context ) {
 369+ $styles = array();
 370+ foreach ( $this->getPrimaryStyles() as $media => $style ) {
 371+ if ( !isset( $styles[$media] ) ) {
 372+ $styles[$media] = '';
 373+ }
 374+ $styles[$media] .= $style;
 375+ }
 376+ foreach ( $this->getPrimaryStyles() as $media => $style ) {
 377+ if ( !isset( $styles[$media] ) ) {
 378+ $styles[$media] = '';
 379+ }
 380+ $styles[$media] .= $this->getSkinStyles( $context->getSkin() );
 381+ }
 382+
 383+ // Collect referenced files
 384+ $files = array();
 385+ foreach ( $styles as $media => $style ) {
 386+ // Extract and store the list of referenced files
 387+ $files = array_merge( $files, CSSMin::getLocalFileReferences( $style ) );
 388+ }
 389+
372390 // Only store if modified
373391 if ( $files !== $this->getFileDependencies( $context->getSkin() ) ) {
374392 $encFiles = FormatJson::encode( $files );
@@ -379,15 +397,15 @@
380398 'md_deps' => $encFiles,
381399 )
382400 );
383 -
 401+
384402 // Save into memcached
385403 global $wgMemc;
386 -
 404+
387405 $key = wfMemcKey( 'resourceloader', 'module_deps', $this->getName(), $context->getSkin() );
388406 $wgMemc->set( $key, $encFiles );
389407 }
390 -
391 - return $style;
 408+
 409+ return $styles;
392410 }
393411
394412 public function getMessages() {
@@ -421,19 +439,31 @@
422440 if ( isset( $this->modifiedTime[$context->getHash()] ) ) {
423441 return $this->modifiedTime[$context->getHash()];
424442 }
425 -
 443+
 444+ // Sort of nasty way we can get a flat list of files depended on by all styles
 445+ $styles = array();
 446+ foreach ( self::organizeFilesByOption( $this->styles, 'media', 'all' ) as $media => $styleFiles ) {
 447+ $styles = array_merge( $styles, $styleFiles );
 448+ }
 449+ $skinFiles = (array) self::getSkinFiles(
 450+ $context->getSkin(), self::organizeFilesByOption( $this->skinStyles, 'media', 'all' )
 451+ );
 452+ foreach ( $skinFiles as $media => $styleFiles ) {
 453+ $styles = array_merge( $styles, $styleFiles );
 454+ }
 455+
 456+ // Final merge, this should result in a master list of dependent files
426457 $files = array_merge(
427458 $this->scripts,
428 - $this->styles,
 459+ $styles,
429460 $context->getDebug() ? $this->debugScripts : array(),
430461 isset( $this->languageScripts[$context->getLanguage()] ) ?
431462 (array) $this->languageScripts[$context->getLanguage()] : array(),
432463 (array) self::getSkinFiles( $context->getSkin(), $this->skinScripts ),
433 - (array) self::getSkinFiles( $context->getSkin(), $this->skinStyles ),
434464 $this->loaders,
435465 $this->getFileDependencies( $context->getSkin() )
436466 );
437 -
 467+
438468 $filesMtime = max( array_map( 'filemtime', array_map( array( __CLASS__, 'remapFilename' ), $files ) ) );
439469
440470 // Get the mtime of the message blob
@@ -468,7 +498,7 @@
469499 *
470500 * @return String: JS
471501 */
472 - protected function getPrimaryStyle() {
 502+ protected function getPrimaryStyles() {
473503 return self::concatStyles( $this->styles );
474504 }
475505
@@ -509,9 +539,9 @@
510540 * Get the skin-specific CSS for a given skin. This is pulled from the
511541 * skin-specific CSS files added through addSkinStyles()
512542 *
513 - * @return String: CSS
 543+ * @return Array: list of CSS strings keyed by media type
514544 */
515 - protected function getSkinStyle( $skin ) {
 545+ protected function getSkinStyles( $skin ) {
516546 return self::concatStyles( self::getSkinFiles( $skin, $this->skinStyles ) );
517547 }
518548
@@ -582,15 +612,41 @@
583613 return implode( "\n", array_map( 'file_get_contents', array_map( array( __CLASS__, 'remapFilename' ), array_unique( (array) $files ) ) ) );
584614 }
585615
 616+ protected static function organizeFilesByOption( $files, $option, $default ) {
 617+ $organizedFiles = array();
 618+ foreach ( (array) $files as $key => $value ) {
 619+ if ( is_int( $key ) ) {
 620+ // File name as the value
 621+ if ( !isset( $organizedFiles[$default] ) ) {
 622+ $organizedFiles[$default] = array();
 623+ }
 624+ $organizedFiles[$default][] = $value;
 625+ } else if ( is_array( $value ) ) {
 626+ // File name as the key, options array as the value
 627+ $media = isset( $value[$option] ) ? $value[$option] : $default;
 628+ if ( !isset( $organizedFiles[$media] ) ) {
 629+ $organizedFiles[$media] = array();
 630+ }
 631+ $organizedFiles[$media][] = $key;
 632+ }
 633+ }
 634+ return $organizedFiles;
 635+ }
 636+
586637 /**
587638 * Get the contents of a set of CSS files, remap then and concatenate
588639 * them, with newlines in between. Each file is used only once.
589640 *
590641 * @param $files Array of file names
591 - * @return String: concatenated and remapped contents of $files
 642+ * @return Array: list of concatenated and remapped contents of $files keyed by media type
592643 */
593 - protected static function concatStyles( $files ) {
594 - return implode( "\n", array_map( array( __CLASS__, 'remapStyle' ), array_unique( (array) $files ) ) );
 644+ protected static function concatStyles( $styles ) {
 645+ $styles = self::organizeFilesByOption( $styles, 'media', 'all' );
 646+ foreach ( $styles as $media => $files ) {
 647+ $styles[$media] =
 648+ implode( "\n", array_map( array( __CLASS__, 'remapStyle' ), array_unique( (array) $files ) ) );
 649+ }
 650+ return $styles;
595651 }
596652
597653 /**
@@ -662,7 +718,7 @@
663719 return $this->modifiedTime;
664720 }
665721
666 - public function getStyle( ResourceLoaderContext $context ) { return ''; }
 722+ public function getStyles( ResourceLoaderContext $context ) { return array(); }
667723 public function getMessages() { return array(); }
668724 public function getLoaderScript() { return ''; }
669725 public function getDependencies() { return array(); }
@@ -738,7 +794,7 @@
739795 return 300; // 5 minutes
740796 }
741797
742 - public function getStyle( ResourceLoaderContext $context ) { return ''; }
 798+ public function getStyles( ResourceLoaderContext $context ) { return array(); }
743799
744800 public function getFlip( $context ) {
745801 global $wgContLang;
Index: trunk/phase3/resources/Resources.php
@@ -9,7 +9,9 @@
1010
1111 /* Skins */
1212
13 - 'vector' => new ResourceLoaderFileModule( array( 'styles' => 'skins/vector/main-ltr.css' ) ),
 13+ 'vector' => new ResourceLoaderFileModule(
 14+ array( 'styles' => array( 'skins/vector/main-ltr.css' => array( 'media' => 'screen' ) ) )
 15+ ),
1416
1517 /* jQuery */
1618
@@ -315,7 +317,7 @@
316318 'dependencies' => 'mediawiki.legacy.wikibits',
317319 ) ),
318320 'mediawiki.legacy.commonPrint' => new ResourceLoaderFileModule( array(
319 - 'styles' => 'skins/common/commonPrint.css',
 321+ 'styles' => array( 'skins/common/commonPrint.css' => array( 'media' => 'print' ) ),
320322 ) ),
321323 'mediawiki.legacy.config' => new ResourceLoaderFileModule( array(
322324 'scripts' => 'skins/common/config.js',
@@ -381,10 +383,10 @@
382384 'dependencies' => 'mediawiki.legacy.wikibits',
383385 ) ),
384386 'mediawiki.legacy.shared' => new ResourceLoaderFileModule( array(
385 - 'styles' => 'skins/common/shared.css',
 387+ 'styles' => array( 'skins/common/shared.css' => array( 'media' => 'screen' ) ),
386388 ) ),
387389 'mediawiki.legacy.oldshared' => new ResourceLoaderFileModule( array(
388 - 'styles' => 'skins/common/oldshared.css',
 390+ 'styles' => array( 'skins/common/oldshared.css' => array( 'media' => 'screen' ) ),
389391 ) ),
390392 'mediawiki.legacy.upload' => new ResourceLoaderFileModule( array(
391393 'scripts' => 'skins/common/upload.js',
@@ -394,4 +396,7 @@
395397 'scripts' => 'skins/common/wikibits.js',
396398 'messages' => array( 'showtoc', 'hidetoc' ),
397399 ) ),
 400+ 'mediawiki.legacy.wikiprintable' => new ResourceLoaderFileModule( array(
 401+ 'styles' => array( 'skins/common/wikiprintable.css' => array( 'media' => 'print' ) ),
 402+ ) ),
398403 ) );
Index: trunk/phase3/resources/mediawiki/mediawiki.js
@@ -323,6 +323,12 @@
324324 // Add style sheet to document
325325 if ( typeof registry[module].style === 'string' && registry[module].style.length ) {
326326 $( 'head' ).append( '<style type="text/css">' + registry[module].style + '</style>' );
 327+ } else if ( typeof registry[module].style === 'object' ) {
 328+ for ( var media in registry[module].style ) {
 329+ $( 'head' ).append(
 330+ '<style type="text/css" media="' + media + '">' + registry[module].style[media] + '</style>'
 331+ );
 332+ }
327333 }
328334 // Add localizations to message system
329335 if ( typeof registry[module].messages === 'object' ) {
@@ -526,8 +532,8 @@
527533 if ( typeof script !== 'function' ) {
528534 throw new Error( 'script must be a function, not a ' + typeof script );
529535 }
530 - if ( typeof style !== 'undefined' && typeof style !== 'string' ) {
531 - throw new Error( 'style must be a string, not a ' + typeof style );
 536+ if ( typeof style !== 'undefined' && typeof style !== 'string' && typeof style !== 'object' ) {
 537+ throw new Error( 'style must be a string or object, not a ' + typeof style );
532538 }
533539 if ( typeof localization !== 'undefined' && typeof localization !== 'object' ) {
534540 throw new Error( 'localization must be an object, not a ' + typeof localization );

Follow-up revisions

RevisionCommit summaryAuthorDate
r72731Fixes issue in r72695 where ResourceLoaderFileModule::getStyles was iterating...tparscal18:42, 10 September 2010

Comments

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

ResourceLoaderFileModule::getStyles has a broken section where it loops over getPrimaryStyles two times and make an assignment from getSkinStyles. It should be iterating over getSkinStyles in the second loop and appending $style.

Status & tagging log