Support redirects in JavaScriptContent

When a JavaScript page is moved, a "redirect" in the form of
mw.loader.load(...) will be left behind, so any other
JavaScript loading the page that way will still work, albeit
with an extra HTTP request.

This also implements Content::getRedirectTarget(), so redirects
are marked properly in the database, and users viewing them
are redirected properly. A magic "/* #REDIRECT */" comment
must be in front of the mw.loader.load call. This is done so
that pages which currently are just one mw.loader.load call
aren't turned into redirects.

Bug: 71200
Bug: 33973
Change-Id: I10fdff087a901da56fad64531f0e382f90ebcf37

diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index 5510d35..f488aa2 100644 (file)
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -363,9 +363,8 @@ class MediaWiki {
                        $this->context->setWikiPage( $article->getPage() );
                }
 
-               // NS_MEDIAWIKI has no redirects.
-               // It is also used for CSS/JS, so performance matters here...
-               if ( $title->getNamespace() == NS_MEDIAWIKI ) {
+               // Skip some unnecessary code if the content model doesn't support redirects
+               if ( !ContentHandler::getForTitle( $title )->supportsRedirects() ) {
                        return $article;
                }
 

diff --git a/includes/content/JavaScriptContent.php b/includes/content/JavaScriptContent.php
index c0194c2..6d23656 100644 (file)
--- a/includes/content/JavaScriptContent.php
+++ b/includes/content/JavaScriptContent.php
@@ -32,6 +32,11 @@
  */
 class JavaScriptContent extends TextContent {
 
+       /**
+        * @var bool|Title|null
+        */
+       private $redirectTarget = false;
+
        /**
         * @param string $text JavaScript code.
         * @param string $modelId the content model name
@@ -73,4 +78,46 @@ class JavaScriptContent extends TextContent {
                return $html;
        }
 
+       /**
+        * If this page is a redirect, return the content
+        * if it should redirect to $target instead
+        *
+        * @param Title $target
+        * @return JavaScriptContent
+        */
+       public function updateRedirect( Title $target ) {
+               if ( !$this->isRedirect() ) {
+                       return $this;
+               }
+
+               return $this->getContentHandler()->makeRedirectContent( $target );
+       }
+
+       /**
+        * @return Title|null
+        */
+       public function getRedirectTarget() {
+               if ( $this->redirectTarget !== false ) {
+                       return $this->redirectTarget;
+               }
+               $this->redirectTarget = null;
+               $text = $this->getNativeData();
+               if ( strpos( $text, '/* #REDIRECT */' ) === 0 ) {
+                       // Extract the title from the url
+                       preg_match( '/title=(.*?)\\\\u0026action=raw/', $text, $matches );
+                       if ( isset( $matches[1] ) ) {
+                               $title = Title::newFromText( $matches[1] );
+                               if ( $title ) {
+                                       // Have a title, check that the current content equals what
+                                       // the redirect content should be
+                                       if ( $this->equals( $this->getContentHandler()->makeRedirectContent( $title ) ) ) {
+                                               $this->redirectTarget = $title;
+                                       }
+                               }
+                       }
+               }
+
+               return $this->redirectTarget;
+       }
+
 }

diff --git a/includes/content/JavaScriptContentHandler.php b/includes/content/JavaScriptContentHandler.php
index d221897..65e3a6f 100644 (file)
--- a/includes/content/JavaScriptContentHandler.php
+++ b/includes/content/JavaScriptContentHandler.php
@@ -41,4 +41,22 @@ class JavaScriptContentHandler extends CodeContentHandler {
        protected function getContentClass() {
                return 'JavaScriptContent';
        }
+
+       public function supportsRedirects() {
+               return true;
+       }
+
+       /**
+        * Create a redirect that is also valid JavaScript
+        *
+        * @param Title $destination
+        * @param string $text ignored
+        * @return JavaScriptContent
+        */
+       public function makeRedirectContent( Title $destination, $text = '' ) {
+               // The parameters are passed as a string so the / is not url-encoded by wfArrayToCgi
+               $url = $destination->getFullURL( 'action=raw&ctype=text/javascript', false, PROTO_RELATIVE );
+               $class = $this->getContentClass();
+               return new $class( '/* #REDIRECT */' . Xml::encodeJsCall( 'mw.loader.load', array( $url ) ) );
+       }
 }

diff --git a/includes/resourceloader/ResourceLoaderWikiModule.php b/includes/resourceloader/ResourceLoaderWikiModule.php
index a4d94f8..0023de2 100644 (file)
--- a/includes/resourceloader/ResourceLoaderWikiModule.php
+++ b/includes/resourceloader/ResourceLoaderWikiModule.php
@@ -149,7 +149,7 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule {
         */
        protected function getContent( $titleText ) {
                $title = Title::newFromText( $titleText );
-               if ( !$title || $title->isRedirect() ) {
+               if ( !$title ) {
                        return null;
                }
 

diff --git a/tests/phpunit/includes/content/CssContentTest.php b/tests/phpunit/includes/content/CssContentTest.php
index 40484d3..44427ba 100644 (file)
--- a/tests/phpunit/includes/content/CssContentTest.php
+++ b/tests/phpunit/includes/content/CssContentTest.php
@@ -4,6 +4,8 @@
  * @group ContentHandler
  * @group Database
  *        ^--- needed, because we do need the database to test link updates
+ *
+ * @FIXME this should not extend JavaScriptContentTest.
  */
 class CssContentTest extends JavaScriptContentTest {
 
@@ -68,7 +70,28 @@ class CssContentTest extends JavaScriptContentTest {
                $this->assertEquals( CONTENT_MODEL_CSS, $content->getContentHandler()->getModelID() );
        }
 
-       public static function dataEquals() {
+       /**
+        * Redirects aren't supported
+        */
+       public static function provideUpdateRedirect() {
+               return array(
+                       array(
+                               '#REDIRECT [[Someplace]]',
+                               '#REDIRECT [[Someplace]]',
+                       ),
+               );
+       }
+
+       /**
+        * Override this since CssContent does not support redirects yet
+        */
+       public static function provideGetRedirectTarget() {
+               return array(
+                       array( null, '' ),
+               );
+       }
+
+               public static function dataEquals() {
                return array(
                        array( new CssContent( 'hallo' ), null, false ),
                        array( new CssContent( 'hallo' ), new CssContent( 'hallo' ), true ),

diff --git a/tests/phpunit/includes/content/JavaScriptContentHandlerTest.php b/tests/phpunit/includes/content/JavaScriptContentHandlerTest.php
new file mode 100644 (file)
index 0000000..0f41020
--- /dev/null
+++ b/tests/phpunit/includes/content/JavaScriptContentHandlerTest.php
@@ -0,0 +1,30 @@
+<?php
+
+class JavaScriptContentHandlerTest extends MediaWikiTestCase {
+
+       /**
+        * @dataProvider provideMakeRedirectContent
+        * @covers JavaScriptContentHandler::makeRedirectContent
+        */
+       public function testMakeRedirectContent( $title, $expected ) {
+               $this->setMwGlobals( array(
+                       'wgServer' => '//example.org',
+                       'wgScript' => '/w/index.php',
+               ) );
+               $ch = new JavaScriptContentHandler();
+               $content = $ch->makeRedirectContent( Title::newFromText( $title ) );
+               $this->assertInstanceOf( 'JavaScriptContent', $content );
+               $this->assertEquals( $expected, $content->serialize( CONTENT_FORMAT_JAVASCRIPT ) );
+       }
+
+       /**
+        * Keep this in sync with JavaScriptContentTest::provideGetRedirectTarget()
+        */
+       public static function provideMakeRedirectContent() {
+               return array(
+                       array( 'MediaWiki:MonoBook.js', '/* #REDIRECT */mw.loader.load("//example.org/w/index.php?title=MediaWiki:MonoBook.js\u0026action=raw\u0026ctype=text/javascript");' ),
+                       array( 'User:FooBar/common.js', '/* #REDIRECT */mw.loader.load("//example.org/w/index.php?title=User:FooBar/common.js\u0026action=raw\u0026ctype=text/javascript");' ),
+                       array( 'Gadget:FooBaz.js', '/* #REDIRECT */mw.loader.load("//example.org/w/index.php?title=Gadget:FooBaz.js\u0026action=raw\u0026ctype=text/javascript");' ),
+               );
+       }
+}

diff --git a/tests/phpunit/includes/content/JavaScriptContentTest.php b/tests/phpunit/includes/content/JavaScriptContentTest.php
index 7193ec9..0ee2712 100644 (file)
--- a/tests/phpunit/includes/content/JavaScriptContentTest.php
+++ b/tests/phpunit/includes/content/JavaScriptContentTest.php
@@ -251,16 +251,31 @@ class JavaScriptContentTest extends TextContentTest {
 
        /**
         * @covers JavaScriptContent::updateRedirect
+        * @dataProvider provideUpdateRedirect
         */
-       public function testUpdateRedirect() {
+       public function testUpdateRedirect( $oldText, $expectedText) {
+               $this->setMwGlobals( array(
+                       'wgServer' => '//example.org',
+                       'wgScriptPath' => '/w/index.php',
+               ) );
                $target = Title::newFromText( "testUpdateRedirect_target" );
 
-               $content = $this->newContent( "#REDIRECT [[Someplace]]" );
+               $content = new JavaScriptContent( $oldText );
                $newContent = $content->updateRedirect( $target );
 
-               $this->assertTrue(
-                       $content->equals( $newContent ),
-                       "content should be unchanged since it's not wikitext"
+               $this->assertEquals( $expectedText, $newContent->getNativeData() );
+       }
+
+       public static function provideUpdateRedirect() {
+               return array(
+                       array(
+                               '#REDIRECT [[Someplace]]',
+                               '#REDIRECT [[Someplace]]',
+                       ),
+                       array(
+                               '/* #REDIRECT */mw.loader.load("//example.org/w/index.php?title=MediaWiki:MonoBook.js\u0026action=raw\u0026ctype=text/javascript");',
+                               '/* #REDIRECT */mw.loader.load("//example.org/w/index.php?title=TestUpdateRedirect_target\u0026action=raw\u0026ctype=text/javascript");'
+                       )
                );
        }
 
@@ -290,4 +305,32 @@ class JavaScriptContentTest extends TextContentTest {
                        array( new JavaScriptContent( "hallo" ), new JavaScriptContent( "HALLO" ), false ),
                );
        }
+
+       /**
+        * @dataProvider provideGetRedirectTarget
+        */
+       public function testGetRedirectTarget( $title, $text ) {
+               $this->setMwGlobals( array(
+                       'wgServer' => '//example.org',
+                       'wgScriptPath' => '/w/index.php',
+               ) );
+               $content = new JavaScriptContent( $text );
+               $target = $content->getRedirectTarget();
+               $this->assertEquals( $title, $target ? $target->getPrefixedText() : null );
+       }
+
+       /**
+        * Keep this in sync with JavaScriptContentHandlerTest::provideMakeRedirectContent()
+        */
+       public static function provideGetRedirectTarget() {
+               return array(
+                       array( 'MediaWiki:MonoBook.js', '/* #REDIRECT */mw.loader.load("//example.org/w/index.php?title=MediaWiki:MonoBook.js\u0026action=raw\u0026ctype=text/javascript");' ),
+                       array( 'User:FooBar/common.js', '/* #REDIRECT */mw.loader.load("//example.org/w/index.php?title=User:FooBar/common.js\u0026action=raw\u0026ctype=text/javascript");' ),
+                       array( 'Gadget:FooBaz.js', '/* #REDIRECT */mw.loader.load("//example.org/w/index.php?title=Gadget:FooBaz.js\u0026action=raw\u0026ctype=text/javascript");' ),
+                       // No #REDIRECT comment
+                       array( null, 'mw.loader.load("//example.org/w/index.php?title=MediaWiki:NoRedirect.js\u0026action=raw\u0026ctype=text/javascript");' ),
+                       // Different domain
+                       array( null, '/* #REDIRECT */mw.loader.load("//example.com/w/index.php?title=MediaWiki:OtherWiki.js\u0026action=raw\u0026ctype=text/javascript");' ),
+               );
+       }
 }