Page MenuHomePhabricator

Don't require that the blocker be a User for a SystemBlock
Closed, ResolvedPublic3 Estimate Story Points

Description

At the point when a system block is created, a blocker is usually set using the 'byText' constructor option. A User object is built from this in AbstractBlock::setBlocker, which requires the text either to be a local user name or to be present in $wgReservedUsernames.

Some system blocks use a translated message as the user name. One disadvantage of this is that the message key must be reserved (the reserved username is translation-dependent - see User::isUsableName). Another disadvantage is that the message might be translated before the User is initialized (see T227007). Others use the English string 'Mediawiki default', perhaps to avoid registering another reserved username. Using an English string is disadvantageous for localization.

Why does the blocker need to be a User? AbstractBlock::setBlocker sets the $blocker property. AbstractBlock::getBlocker is rarely used outside the block classes; within them, it is used by AbstractBlock::getBy and AbstractBlock::getByName which call the User methods to get the blocker's ID and name respectively. These two methods are used in a few places, often related to reporting block errors.

It may be better to stop requiring that the blocker of a SystemBlock be a User. The $blocker property could be moved to DatabaseBlock, while AbstractBlock::getBy and AbstractBlock::getByName could be overridden in SystemBlock to return 0 and whatever was passed in as the 'byText' option for backwards compatibility (this is the same as their current return values).

This would allow 'byText' to be a message key, which could be translated when needed.

Details

Related Gerrit Patches:

Event Timeline

Tchanders created this task.Jul 1 2019, 3:32 PM
Tchanders updated the task description. (Show Details)Jul 1 2019, 4:37 PM
Restricted Application added a subscriber: MGChecker. · View Herald TranscriptJul 31 2019, 3:49 PM
Niharika triaged this task as Medium priority.Jul 31 2019, 3:54 PM
Niharika set the point value for this task to 3.
dmaza claimed this task.Aug 19 2019, 7:06 PM
dmaza moved this task from Ready to In Progress on the Anti-Harassment (The Letter Song) board.

Change 531517 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[mediawiki/core@master] WIP Remove requirement for blocker on a SystemBlock to be a User

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

Change 540940 had a related patch set uploaded (by Dmaza; owner: Dmaza):
[mediawiki/core@master] [WIP] - remove blocker dependency for System and Composite blocks

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

Tchanders added a subscriber: dmaza.
Tchanders reassigned this task from Tchanders to dmaza.Oct 25 2019, 7:34 PM

@dom_walden Might want to test this along with T236814

Change 531517 abandoned by Tchanders:
WIP Remove requirement for blocker on a SystemBlock to be a User

Reason:
Done in I1b17c652b12d98

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

Tested:

  • Database blocks
  • Composite blocks
  • $wgProxyList System Blocks
  • $wgDnsBlacklistUrls System Blocks

Checking how the block message looks editing on desktop and mobile and API, and in different languages.

Briefly checked that, for database blocks, the blocker is correct in the block message and on Special:BlockList.

The mobile block message for system and composite blocks no longer includes a section for the blocker ("Blocked by").

For the API block message for system and composite blocks, the blockedby is an empty string (e.g. {'blockedbyid': 0, 'blockedby': '', 'blockid': None, ...).

Also tested extensions:

  • GlobalBlocking
  • CheckUser
  • CentralAuth
  • TorBlock

Checking the display of the system blocks they create and (in the case of GlobalBlocking, CheckUser and CentralAuth) the database blocks they create as well.

dbarratt closed this task as Resolved.Nov 4 2019, 10:51 PM