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

diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index 87468bd..8ec6d35 100644 (file)
--- 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 8d11d90..f9840ad 100644 (file)
--- 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 c1db302..0bc980a 100644 (file)
--- 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 cc6fb11..429de4e 100644 (file)
--- 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 );