From: Kunal Mehta Date: Tue, 23 Sep 2014 22:41:03 +0000 (-0700) Subject: Support redirects in JavaScriptContent X-Git-Tag: 1.31.0-rc.0~10706 X-Git-Url: https://git.heureux-cyclage.org/?a=commitdiff_plain;h=ad9f14d662f9597e9ca6bb4d1b572331b8256831;p=lhc%2Fweb%2Fwiklou.git 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 5510d35913..f488aa2485 100644 --- 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 c0194c2e02..6d23656009 100644 --- 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 d22189711a..65e3a6f020 100644 --- 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 a4d94f8fd6..0023de2757 100644 --- 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 40484d3a0e..44427ba9b6 100644 --- 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 index 0000000000..0f41020f25 --- /dev/null +++ b/tests/phpunit/includes/content/JavaScriptContentHandlerTest.php @@ -0,0 +1,30 @@ +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 7193ec9fac..0ee2712928 100644 --- 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");' ), + ); + } }