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() );
                }
 
                        $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;
                }
 
                        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 {
 
  */
 class JavaScriptContent extends TextContent {
 

+       /**
+        * @var bool|Title|null
+        */
+       private $redirectTarget = false;
+

        /**
         * @param string $text JavaScript code.
         * @param string $modelId the content model name
        /**
         * @param string $text JavaScript code.
         * @param string $modelId the content model name


@@ -73,4 +78,46 @@ class JavaScriptContent extends TextContent {
                return $html;
        }
 
                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';
        }
        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 );
         */
        protected function getContent( $titleText ) {
                $title = Title::newFromText( $titleText );

-               if ( !$title || $title->isRedirect() ) {
+               if ( !$title ) {

                        return null;
                }
 
                        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
  * @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 {
 
  */
 class CssContentTest extends JavaScriptContentTest {
 


@@ -68,7 +70,28 @@ class CssContentTest extends JavaScriptContentTest {
                $this->assertEquals( CONTENT_MODEL_CSS, $content->getContentHandler()->getModelID() );
        }
 
                $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 ),
                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
 
        /**
         * @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" );
 
                $target = Title::newFromText( "testUpdateRedirect_target" );
 

-               $content = $this->newContent( "#REDIRECT [[Someplace]]" );
+               $content = new JavaScriptContent( $oldText );

                $newContent = $content->updateRedirect( $target );
 
                $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 ),
                );
        }
                        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");' ),
+               );
+       }

 }
 }