From fc6f65a85b4096f270c4b02feede86f66b509749 Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Tue, 8 Feb 2011 06:34:38 +0000 Subject: [PATCH] Fixes for ResourceLoaderWikiModule r72776. No serious bugs found, do not merge before deployment. * Specify page titles as strings instead of split NS/DBK, as suggested by Roan on CR. It seemed sensible to me. * Pass a Title object to getContent() instead of a string, to avoid unnecessary object construction overhead * "*" and "/" are valid title characters. Check module input for JS comment end tokens. * Fixed inappropriate conversion to boolean, when checking result of getContent(). Presumably the idea was to omit empty sections and errors, so that's what I did. Maybe an informative error message would be better in the error case. * Use LinkBatch for selecting multiple page rows instead of Database::makeWhereFrom2d(). * Fixed assignment expression. --- .../ResourceLoaderSiteModule.php | 13 ++--- .../ResourceLoaderUserModule.php | 12 ++-- .../ResourceLoaderWikiModule.php | 56 +++++++++++-------- 3 files changed, 46 insertions(+), 35 deletions(-) diff --git a/includes/resourceloader/ResourceLoaderSiteModule.php b/includes/resourceloader/ResourceLoaderSiteModule.php index 2336acbf3a..977d16bbf1 100644 --- a/includes/resourceloader/ResourceLoaderSiteModule.php +++ b/includes/resourceloader/ResourceLoaderSiteModule.php @@ -36,15 +36,14 @@ class ResourceLoaderSiteModule extends ResourceLoaderWikiModule { global $wgHandheldStyle; $pages = array( - 'Common.js' => array( 'ns' => NS_MEDIAWIKI, 'type' => 'script' ), - 'Common.css' => array( 'ns' => NS_MEDIAWIKI, 'type' => 'style' ), - ucfirst( $context->getSkin() ) . '.js' => array( 'ns' => NS_MEDIAWIKI, 'type' => 'script' ), - ucfirst( $context->getSkin() ) . '.css' => array( 'ns' => NS_MEDIAWIKI, 'type' => 'style' ), - 'Print.css' => array( 'ns' => NS_MEDIAWIKI, 'type' => 'style', 'media' => 'print' ), + 'MediaWiki:Common.js' => array( 'type' => 'script' ), + 'MediaWiki:Common.css' => array( 'type' => 'style' ), + 'MediaWiki:' . ucfirst( $context->getSkin() ) . '.js' => array( 'type' => 'script' ), + 'MediaWiki:' . ucfirst( $context->getSkin() ) . '.css' => array( 'type' => 'style' ), + 'MediaWiki:Print.css' => array( 'type' => 'style', 'media' => 'print' ), ); if ( $wgHandheldStyle ) { - $pages['Handheld.css'] = array( - 'ns' => NS_MEDIAWIKI, + $pages['MediaWiki:Handheld.css'] = array( 'type' => 'style', 'media' => 'handheld' ); } diff --git a/includes/resourceloader/ResourceLoaderUserModule.php b/includes/resourceloader/ResourceLoaderUserModule.php index 4791a0f06e..f2413fd67b 100644 --- a/includes/resourceloader/ResourceLoaderUserModule.php +++ b/includes/resourceloader/ResourceLoaderUserModule.php @@ -32,12 +32,12 @@ class ResourceLoaderUserModule extends ResourceLoaderWikiModule { if ( $context->getUser() ) { $username = $context->getUser(); return array( - "$username/common.js" => array( 'ns' => NS_USER, 'type' => 'script' ), - "$username/" . $context->getSkin() . '.js' => - array( 'ns' => NS_USER, 'type' => 'script' ), - "$username/common.css" => array( 'ns' => NS_USER, 'type' => 'style' ), - "$username/" . $context->getSkin() . '.css' => - array( 'ns' => NS_USER, 'type' => 'style' ), + "User:$username/common.js" => array( 'type' => 'script' ), + "User:$username/" . $context->getSkin() . '.js' => + array( 'type' => 'script' ), + "User:$username/common.css" => array( 'type' => 'style' ), + "User:$username/" . $context->getSkin() . '.css' => + array( 'type' => 'style' ), ); } return array(); diff --git a/includes/resourceloader/ResourceLoaderWikiModule.php b/includes/resourceloader/ResourceLoaderWikiModule.php index 0abd74f68d..d8cd79bb4c 100644 --- a/includes/resourceloader/ResourceLoaderWikiModule.php +++ b/includes/resourceloader/ResourceLoaderWikiModule.php @@ -45,12 +45,12 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule { /* Protected Methods */ - protected function getContent( $page, $ns ) { - if ( $ns === NS_MEDIAWIKI ) { - return wfEmptyMsg( $page ) ? '' : wfMsgExt( $page, 'content' ); + protected function getContent( $title ) { + if ( $title->getNamespace() === NS_MEDIAWIKI ) { + $dbkey = $title->getDBkey(); + return wfEmptyMsg( $dbkey ) ? '' : wfMsgExt( $dbkey, 'content' ); } - $title = Title::newFromText( $page, $ns ); - if ( !$title || !$title->isCssJsSubpage() ) { + if ( !$title->isCssJsSubpage() ) { return null; } $revision = Revision::newFromTitle( $title ); @@ -64,14 +64,20 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule { public function getScript( ResourceLoaderContext $context ) { $scripts = ''; - foreach ( $this->getPages( $context ) as $page => $options ) { + foreach ( $this->getPages( $context ) as $titleText => $options ) { if ( $options['type'] !== 'script' ) { continue; } - $script = $this->getContent( $page, $options['ns'] ); - if ( $script ) { - $ns = MWNamespace::getCanonicalName( $options['ns'] ); - $scripts .= "/* $ns:$page */\n$script\n"; + $title = Title::newFromText( $titleText ); + if ( !$title ) { + continue; + } + $script = $this->getContent( $title ); + if ( strval( $script ) !== '' ) { + if ( strpos( $titleText, '*/' ) === false ) { + $scripts .= "/* $titleText */\n"; + } + $scripts .= $script . "\n"; } } return $scripts; @@ -80,13 +86,17 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule { public function getStyles( ResourceLoaderContext $context ) { $styles = array(); - foreach ( $this->getPages( $context ) as $page => $options ) { + foreach ( $this->getPages( $context ) as $titleText => $options ) { if ( $options['type'] !== 'style' ) { continue; } + $title = Title::newFromText( $titleText ); + if ( !$title ) { + continue; + } $media = isset( $options['media'] ) ? $options['media'] : 'all'; - $style = $this->getContent( $page, $options['ns'] ); - if ( !$style ) { + $style = $this->getContent( $title ); + if ( strval( $style ) === '' ) { continue; } if ( $this->getFlip( $context ) ) { @@ -95,8 +105,10 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule { if ( !isset( $styles[$media] ) ) { $styles[$media] = ''; } - $ns = MWNamespace::getCanonicalName( $options['ns'] ); - $styles[$media] .= "/* $ns:$page */\n$style\n"; + if ( strpos( $titleText, '*/' ) === false ) { + $styles[$media] .= "/* $titleText */\n"; + } + $styles[$media] .= $style . "\n"; } return $styles; } @@ -107,17 +119,16 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule { return $this->modifiedTime[$hash]; } - $titles = array(); - foreach ( $this->getPages( $context ) as $page => $options ) { - $titles[$options['ns']][$page] = true; + $batch = new LinkBatch; + foreach ( $this->getPages( $context ) as $titleText => $options ) { + $batch->addObj( Title::newFromText( $titleText ) ); } $modifiedTime = 1; // wfTimestamp() interprets 0 as "now" - - if ( $titles ) { + if ( !$batch->isEmpty() ) { $dbr = wfGetDB( DB_SLAVE ); $latest = $dbr->selectField( 'page', 'MAX(page_touched)', - $dbr->makeWhereFrom2d( $titles, 'page_namespace', 'page_title' ), + $batch->constructSet( 'page', $dbr ), __METHOD__ ); if ( $latest ) { @@ -125,6 +136,7 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule { } } - return $this->modifiedTime[$hash] = $modifiedTime; + $this->modifiedTime[$hash] = $modifiedTime; + return $modifiedTime; } } -- 2.20.1