From a90b7ea969b4332a6229be1c4160190a3ec79200 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thiemo=20M=C3=A4ttig?= Date: Thu, 17 Jul 2014 10:27:38 +0200 Subject: [PATCH] Make an empty "?action=" parameter default to "view" When no action is given, e.g. in https://en.wikipedia.org/wiki/URL the action defaults to "view". Just like you called https://en.wikipedia.org/wiki/URL?action=view But when the action is empty, e.g. https://en.wikipedia.org/wiki/URL?action= you get an error message telling you that "the action specified" can not be "recognized". Wait. I did not "specified" an action. That's why I left the parameter empty. It doesn't make much sense to have an empty action name that's different from the "view" default, right? From the users point of view I expect the empty string to behave like null/undefined. Change-Id: I331924d9223e597293bc8285f0c330edf08e429b --- includes/MediaWiki.php | 11 ++++---- includes/actions/Action.php | 2 +- includes/skins/SkinTemplate.php | 2 +- tests/phpunit/includes/actions/ActionTest.php | 27 ++++++++++++++----- 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php index 87468bd2bf..8ec6d35070 100644 --- a/includes/MediaWiki.php +++ b/includes/MediaWiki.php @@ -59,7 +59,7 @@ class MediaWiki { $request = $this->context->getRequest(); $curid = $request->getInt( 'curid' ); $title = $request->getVal( 'title' ); - $action = $request->getVal( 'action', 'view' ); + $action = $this->getAction(); if ( $request->getCheck( 'search' ) ) { // Compatibility with old search URLs which didn't use Special:Search @@ -229,7 +229,7 @@ class MediaWiki { throw new BadTitleError(); } // Redirect loops, no title in URL, $wgUsePathInfo URLs, and URLs with a variant - } elseif ( $request->getVal( 'action', 'view' ) == 'view' && !$request->wasPosted() + } elseif ( $this->getAction() === 'view' && !$request->wasPosted() && ( $request->getVal( 'title' ) === null || $title->getPrefixedDBkey() != $request->getVal( 'title' ) ) && !count( $request->getValueNames( array( 'action', 'title' ) ) ) @@ -330,7 +330,7 @@ class MediaWiki { // Namespace might change when using redirects // Check for redirects ... - $action = $request->getVal( 'action', 'view' ); + $action = $this->getAction(); $file = ( $title->getNamespace() == NS_FILE ) ? $article->getFile() : null; if ( ( $action == 'view' || $action == 'render' ) // ... for actions that show content && !$request->getVal( 'oldid' ) // ... and are not old revisions @@ -416,7 +416,7 @@ class MediaWiki { return; } - if ( wfRunHooks( 'UnknownAction', array( $request->getVal( 'action', 'view' ), $page ) ) ) { + if ( wfRunHooks( 'UnknownAction', array( $this->getAction(), $page ) ) ) { $output->setStatusCode( 404 ); $output->showErrorPage( 'nosuchaction', 'nosuchactiontext' ); } @@ -489,8 +489,7 @@ class MediaWiki { $request = $this->context->getRequest(); // Send Ajax requests to the Ajax dispatcher. - if ( $this->config->get( 'UseAjax' ) && $request->getVal( 'action', 'view' ) == 'ajax' ) { - + if ( $this->config->get( 'UseAjax' ) && $this->getAction() === 'ajax' ) { // Set a dummy title, because $wgTitle == null might break things $title = Title::makeTitle( NS_MAIN, 'AJAX' ); $this->context->setTitle( $title ); diff --git a/includes/actions/Action.php b/includes/actions/Action.php index 8d11d901a2..f9840ad87b 100644 --- a/includes/actions/Action.php +++ b/includes/actions/Action.php @@ -142,7 +142,7 @@ abstract class Action { // Trying to get a WikiPage for NS_SPECIAL etc. will result // in WikiPage::factory throwing "Invalid or virtual namespace -1 given." // For SpecialPages et al, default to action=view. - if ( !$context->canUseWikiPage() ) { + if ( $actionName === '' || !$context->canUseWikiPage() ) { return 'view'; } diff --git a/includes/skins/SkinTemplate.php b/includes/skins/SkinTemplate.php index c1db302ded..0bc980a956 100644 --- a/includes/skins/SkinTemplate.php +++ b/includes/skins/SkinTemplate.php @@ -844,7 +844,7 @@ class SkinTemplate extends Skin { ); // parameters - $action = $request->getVal( 'action', 'view' ); + $action = Action::getActionName( $this ); $userCanRead = $title->quickUserCan( 'read', $user ); diff --git a/tests/phpunit/includes/actions/ActionTest.php b/tests/phpunit/includes/actions/ActionTest.php index cc6fb11a6f..429de4e143 100644 --- a/tests/phpunit/includes/actions/ActionTest.php +++ b/tests/phpunit/includes/actions/ActionTest.php @@ -57,8 +57,6 @@ class ActionTest extends MediaWikiTestCase { // Null and non-existing values array( 'null', null ), array( 'undeclared', null ), - array( '', null ), - array( false, null ), ); } @@ -129,20 +127,37 @@ class ActionTest extends MediaWikiTestCase { $this->assertType( $expected ?: 'null', $action ); } - public function testNull_doesNotExist() { - $exists = Action::exists( null ); + public function emptyActionProvider() { + return array( + array( null ), + array( false ), + array( '' ), + ); + } + + /** + * @dataProvider emptyActionProvider + */ + public function testEmptyAction_doesNotExist( $requestedAction ) { + $exists = Action::exists( $requestedAction ); $this->assertFalse( $exists ); } - public function testNull_defaultsToView() { + /** + * @dataProvider emptyActionProvider + */ + public function testEmptyAction_defaultsToView() { $context = $this->getContext( null ); $actionName = Action::getActionName( $context ); $this->assertEquals( 'view', $actionName ); } - public function testNull_canNotBeInstantiated() { + /** + * @dataProvider emptyActionProvider + */ + public function testEmptyAction_canNotBeInstantiated() { $page = $this->getPage(); $action = Action::factory( null, $page ); -- 2.20.1