As a long-term solution, this would probably be addressed by introducing page types T209924

As a more short term solution, we need to verify what the individual actions return from getName method, and figure out if there are valid cases when the returned name is different from the name which the action is registered under. In case they normally don't, we can deprecate Action::getName and start entirely relying on the names actions are registered under. This will allow getting the action name without constructing the action itself.

So, a todo now is to look over all the Action::getName overrides.

Elevator pitch of why it's important

Inside MediaWiki main class, the code flow related to Action instantiation looks somewhat like this:

$this->context = REquestContext::getMain;
$this->context->setTitle( $this->parseTitle );
$act = Action::getActionName
$action = Action::factory( $act )

The last two calls actually instantiate the Action and all it's dependencies twice. Given that Action bring in quite a few dependencies, that might be pretty heavy. Additionally, once we switch Action to depend on Article instead of WikiPage, the call to Action::getActionName would instantiate Article too early in the initialization flow, which might become a problem.

Additionally, note that Action::factory is called essentially with the result of Action->getName. ActionFactory does a lookup of an appropriate implementation class within wgActions, so if an action overrides the Action::getName and returns anything other then the name it is registered under - it will break.

So, the current situation is not only pointless, but probably has some performance impact and is extremely fragile.

Plan

I have made an overview of all the Action::getName overrides, and all of them are returning the same key the action is registered under.

One exception is SpecialPageAction, however the modifications it does were added as a workaround to support IE6 in T22966, thus it can be removed now. Flow overrides action to return a private field set in constructor, but that's just to support make it simpler to override different flow actions.

The action itself needs to know it's name sometimes, most heavily used in FormAction::getForm.

Proposal:

• Remove workaround for T22966 from Action::getActionName and SpecialPageAction::getName
• Under the Stable interface policy make Action non-newable, thus deprecate and remove access to the constructor from anything except subclasses
• Modify Action constructor signature to __construct( string $name, Article $article ) - note that we drop the context, since Article should already hold the context. This is not a part of this issue, but since we would be deprecating the constructor signature, we might as well do this. Have b/c support for old signature, deprecate it.
• Store the $name into a private field in the class, make Action::getName non-abstract, return the name passed in the constructor. Deprecate Action::getName for overriding in subclasses
• Remove Action subclasses getName overrides
• Make Action::getName final.

This plan is only needed for consistency, it's possible to get the correct action name without doing all this, demonstrated in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/585627 - some Action tests are failing, but those test cases actually seem misleading - if Action::getName would be overridden like this in production, it will actually break MediaWiki.

Change 858436 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] HistoryAction: Remove use of action=historysubmit hack

https://gerrit.wikimedia.org/r/858436

Change 858436 merged by jenkins-bot:

[mediawiki/core@master] HistoryAction: Remove use of action=historysubmit hack

https://gerrit.wikimedia.org/r/858436