From 3d4a81653c2d412cb41f996bffc34ea3f6541bd7 Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Thu, 21 Mar 2013 09:36:03 -0400
Subject: [PATCH] (bug 46405) Fix errors in mw.title.new( pageid )

mw.title.new( pageid ) should not throw an error for an nonexisting
pageid, just return nil. Similarly, it should always return nil for 0,
rather than returning the last non-existent title created.

Change-Id: I3cdbb24fc785aef0f8e75fba1feccd26ac5b7370
---
 engines/LuaCommon/TitleLibrary.php            | 13 +++++++------
 tests/engines/LuaCommon/TitleLibraryTest.php  |  2 +-
 tests/engines/LuaCommon/TitleLibraryTests.lua | 16 ++++++++++++++--
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/engines/LuaCommon/TitleLibrary.php b/engines/LuaCommon/TitleLibrary.php
index c35c9e64..51259203 100644
--- a/engines/LuaCommon/TitleLibrary.php
+++ b/engines/LuaCommon/TitleLibrary.php
@@ -6,7 +6,7 @@ class Scribunto_LuaTitleLibrary extends Scribunto_LuaLibraryBase {
 	// addition besides the one for the current page calls
 	// incrementExpensiveFunctionCount()
 	private $titleCache = array();
-	private $idCache = array();
+	private $idCache = array( 0 => null );
 
 	function register() {
 		$lib = array(
@@ -58,13 +58,11 @@ class Scribunto_LuaTitleLibrary extends Scribunto_LuaLibraryBase {
 	 * @return array Lua data
 	 */
 	private function returnTitleToLua( Title $title ) {
-		if ( !$title ) {
-			return array( null );
-		}
-
 		// Cache it
 		$this->titleCache[$title->getPrefixedDBkey()] = $title;
-		$this->idCache[$title->getArticleID()] = $title;
+		if ( $title->getArticleID() > 0 ) {
+			$this->idCache[$title->getArticleID()] = $title;
+		}
 
 		// Record a link
 		if ( $this->getParser() && !$title->equals( $this->getTitle() ) ) {
@@ -116,6 +114,9 @@ class Scribunto_LuaTitleLibrary extends Scribunto_LuaLibraryBase {
 				$title = Title::newFromID( $text_or_id );
 				$this->idCache[$text_or_id] = $title;
 			}
+			if ( !$title ) {
+				return array( null );
+			}
 		} elseif ( $type === 'string' ) {
 			$this->checkNamespace( 'title.new', 2, $defaultNamespace, NS_MAIN );
 
diff --git a/tests/engines/LuaCommon/TitleLibraryTest.php b/tests/engines/LuaCommon/TitleLibraryTest.php
index 72ec5af5..6eaed0bb 100644
--- a/tests/engines/LuaCommon/TitleLibraryTest.php
+++ b/tests/engines/LuaCommon/TitleLibraryTest.php
@@ -55,7 +55,7 @@ class Scribunto_LuaTitleLibraryTests extends Scribunto_LuaEngineTestBase {
 		// Indicate to the tests that it's safe to create the title objects
 		$interpreter = $this->getEngine()->getInterpreter();
 		$interpreter->callFunction(
-			$interpreter->loadString( 'mw.ok = true', 'fortest' )
+			$interpreter->loadString( "mw.title.testPageId = {$page->getId()}", 'fortest' )
 		);
 
 		$this->setMwGlobals( array(
diff --git a/tests/engines/LuaCommon/TitleLibraryTests.lua b/tests/engines/LuaCommon/TitleLibraryTests.lua
index bd1454ee..84b5465e 100644
--- a/tests/engines/LuaCommon/TitleLibraryTests.lua
+++ b/tests/engines/LuaCommon/TitleLibraryTests.lua
@@ -1,7 +1,7 @@
 local testframework = require 'Module:TestFramework'
 
-local title, title_copy, title2, title3, title4, title5, title4p
-if mw.ok then
+local title, title_copy, title2, title3, title4, title5, title6, title4p
+if mw.title.testPageId then
 	title = mw.title.getCurrentTitle()
 	title_copy = mw.title.getCurrentTitle()
 	title2 = mw.title.new( 'Module:TestFramework' )
@@ -119,6 +119,18 @@ local tests = {
 	  args = { '<bad title>' },
 	  expect = { nil }
 	},
+	{ name = 'title.new with nonexistent pageid', func = mw.title.new,
+	  args = { -1 },
+	  expect = { nil }
+	},
+	{ name = 'title.new with pageid 0', func = mw.title.new,
+	  args = { 0 },
+	  expect = { nil }
+	},
+	{ name = 'title.new with existing pageid', func = mw.title.new, type = 'ToString',
+	  args = { mw.title.testPageId },
+	  expect = { 'ScribuntoTestPage' }
+	},
 
 	{ name = 'title.makeTitle', func = mw.title.makeTitle, type = 'ToString',
 	  args = { 'Module', 'TestFramework' },