From 5b0fd153dfc3732a320355bd9b8ab7fd737f0434 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Gerg=C5=91=20Tisza?= <tgr.huwiki@gmail.com>
Date: Wed, 12 Jun 2019 13:41:15 +0200
Subject: [PATCH] Do move options checks before the move

This is both more correct conceptually (if the move-subpages
permission check depends on the page content, we want that to be
the content that is getting moved) and hopefully more robust
(whereas doing permission checks on a title just after having
moved it might run into issues with replication lag).

Also, permission checks can be expensive so skip the move-subpage
check when the user did not request moving subpages anyway. And
replace the deprecated method.

Bug: T225366
Change-Id: I9809b2a5bbae4006d5c5389dfd7c04f20f7da8fd
---
 includes/specials/SpecialMovepage.php | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/includes/specials/SpecialMovepage.php b/includes/specials/SpecialMovepage.php
index 15b7c632ae..252df5be09 100644
--- a/includes/specials/SpecialMovepage.php
+++ b/includes/specials/SpecialMovepage.php
@@ -597,7 +597,15 @@ class MovePageForm extends UnlistedSpecialPage {
 		# Do the actual move.
 		$mp = new MovePage( $ot, $nt );
 
+		# check whether the requested actions are permitted / possible
 		$userPermitted = $mp->checkPermissions( $user, $this->reason )->isOK();
+		if ( $ot->isTalkPage() || $nt->isTalkPage() ) {
+			$this->moveTalk = false;
+		}
+		if ( $this->moveSubpages ) {
+			$permissionManager = MediaWikiServices::getInstance()->getPermissionManager();
+			$this->moveSubpages = $permissionManager->userCan( 'move-subpages', $user, $ot );
+		}
 
 		$status = $mp->moveIfAllowed( $user, $this->reason, $createRedirect );
 		if ( !$status->isOK() ) {
@@ -646,19 +654,11 @@ class MovePageForm extends UnlistedSpecialPage {
 		$movePage = $this;
 		Hooks::run( 'SpecialMovepageAfterMove', [ &$movePage, &$ot, &$nt ] );
 
-		# Now we move extra pages we've been asked to move: subpages and talk
-		# pages.  First, if the old page or the new page is a talk page, we
-		# can't move any talk pages: cancel that.
-		if ( $ot->isTalkPage() || $nt->isTalkPage() ) {
-			$this->moveTalk = false;
-		}
-
-		if ( count( $ot->getUserPermissionsErrors( 'move-subpages', $user ) ) ) {
-			$this->moveSubpages = false;
-		}
-
-		/**
-		 * Next make a list of id's.  This might be marginally less efficient
+		/*
+		 * Now we move extra pages we've been asked to move: subpages and talk
+		 * pages.
+		 *
+		 * First, make a list of id's.  This might be marginally less efficient
 		 * than a more direct method, but this is not a highly performance-cri-
 		 * tical code path and readable code is more important here.
 		 *
-- 
2.20.1