From: Timo Tijhof Date: Wed, 26 Oct 2016 16:14:21 +0000 (+0100) Subject: Convert action=markpatrolled fallback interface to HTTP POST X-Git-Tag: 1.31.0-rc.0~4926^2 X-Git-Url: http://git.cyclocoop.org/%22.htmlspecialchars%28%24url_syndic%29.%22?a=commitdiff_plain;h=4ff510e4a7020ae0a9a3d2c283041effeee9ccda;p=lhc%2Fweb%2Fwiklou.git Convert action=markpatrolled fallback interface to HTTP POST The main interface already has javascript enhancement to use the API and mw.notify. This patch affects permalinks without tokens, and opening the link without javascript. This will match the current behaviour of action=watch. Bug: T130946 Change-Id: I6be2c07824c17b165e068fc4ac36ab192e12bc9d --- diff --git a/docs/hooks.txt b/docs/hooks.txt index 562d7b4b2f..6668834717 100644 --- a/docs/hooks.txt +++ b/docs/hooks.txt @@ -1188,7 +1188,6 @@ wrapped in a span element which has class="patrollink". $differenceEngine: DifferenceEngine object &$markAsPatrolledLink: The "mark as patrolled" link HTML (string) $rcid: Recent change ID (rc_id) for this change (int) -$token: Patrol token; $rcid is used in generating this variable 'DifferenceEngineMarkPatrolledRCID': Allows extensions to possibly change the rcid parameter. For example the rcid might be set to zero due to the user being the same as the diff --git a/includes/actions/MarkpatrolledAction.php b/includes/actions/MarkpatrolledAction.php index 85ea87cead..8df60445da 100644 --- a/includes/actions/MarkpatrolledAction.php +++ b/includes/actions/MarkpatrolledAction.php @@ -1,7 +1,5 @@ getRequest(); + public function getRestriction() { + return 'patrol'; + } - $rcId = $request->getInt( 'rcid' ); - $rc = RecentChange::newFromId( $rcId ); - if ( is_null( $rc ) ) { + protected function getRecentChange( $data = null ) { + $rc = null; + // Note: This works both on initial GET url and after submitting the form + $rcId = $data ? intval( $data['rcid'] ) : $this->getRequest()->getInt( 'rcid' ); + if ( $rcId ) { + $rc = RecentChange::newFromId( $rcId ); + } + if ( !$rc ) { throw new ErrorPageError( 'markedaspatrollederror', 'markedaspatrollederrortext' ); } + return $rc; + } - $user = $this->getUser(); - if ( !$user->matchEditToken( $request->getVal( 'token' ), $rcId ) ) { - throw new ErrorPageError( 'sessionfailure-title', 'sessionfailure' ); - } + protected function preText() { + $rc = $this->getRecentChange(); + $title = $rc->getTitle(); + + // Based on logentry-patrol-patrol (see PatrolLogFormatter) + $revId = $rc->getAttribute( 'rc_this_oldid' ); + $query = [ + 'curid' => $rc->getAttribute( 'rc_cur_id' ), + 'diff' => $revId, + 'oldid' => $rc->getAttribute( 'rc_last_oldid' ) + ]; + $revlink = Linker::link( $title, htmlspecialchars( $revId ), [], $query ); + $pagelink = Linker::link( $title, htmlspecialchars( $title->getPrefixedText() ) ); + return $this->msg( 'confirm-markpatrolled-top' )->params( + $title->getPrefixedText(), + // Provide pre-rendered link as parser would render [[:$1]] as bold non-link + Message::rawParam( $pagelink ), + Message::rawParam( $revlink ) + )->parse(); + } + + protected function alterForm( HTMLForm $form ) { + $form->addHiddenField( 'rcid', $this->getRequest()->getInt( 'rcid' ) ); + $form->setTokenSalt( 'patrol' ); + $form->setSubmitTextMsg( 'confirm-markpatrolled-button' ); + } + + /** + * @return bool|array True for success, false for didn't-try, array of errors on failure + */ + public function onSubmit( $data ) { + $user = $this->getUser(); + $rc = $this->getRecentChange( $data ); $errors = $rc->doMarkPatrolled( $user ); if ( in_array( [ 'rcpatroldisabled' ], $errors ) ) { throw new ErrorPageError( 'rcpatroldisabled', 'rcpatroldisabledtext' ); } - if ( in_array( [ 'hookaborted' ], $errors ) ) { - // The hook itself has handled any output - return; - } - - # It would be nice to see where the user had actually come from, but for now just guess + // Guess where the user came from + // TODO: Would be nice to see where the user actually came from if ( $rc->getAttribute( 'rc_type' ) == RC_NEW ) { $returnTo = 'Newpages'; } elseif ( $rc->getAttribute( 'rc_log_type' ) == 'upload' ) { @@ -76,18 +108,25 @@ class MarkpatrolledAction extends FormlessAction { $this->getOutput()->setPageTitle( $this->msg( 'markedaspatrollederror' ) ); $this->getOutput()->addWikiMsg( 'markedaspatrollederror-noautopatrol' ); $this->getOutput()->returnToMain( null, $return ); - - return; + return true; } - if ( count( $errors ) ) { - throw new PermissionsError( 'patrol', $errors ); + if ( $errors ) { + if ( !in_array( [ 'hookaborted' ], $errors ) ) { + throw new PermissionsError( 'patrol', $errors ); + } + // The hook itself has handled any output + return $errors; } - # Inform the user $this->getOutput()->setPageTitle( $this->msg( 'markedaspatrolled' ) ); $this->getOutput()->addWikiMsg( 'markedaspatrolledtext', $rc->getTitle()->getPrefixedText() ); $this->getOutput()->returnToMain( null, $return ); + return true; + } + + public function onSuccess() { + // Required by parent class. Redundant as our onSubmit handles output already. } public function doesWrites() { diff --git a/includes/diff/DifferenceEngine.php b/includes/diff/DifferenceEngine.php index 16e9a443ff..b5a81eaa5f 100644 --- a/includes/diff/DifferenceEngine.php +++ b/includes/diff/DifferenceEngine.php @@ -496,12 +496,11 @@ class DifferenceEngine extends ContextSource { [ 'action' => 'markpatrolled', 'rcid' => $linkInfo['rcid'], - 'token' => $linkInfo['token'], ] ) . ']'; // Allow extensions to change the markpatrolled link Hooks::run( 'DifferenceEngineMarkPatrolledLink', [ $this, - &$this->mMarkPatrolledLink, $linkInfo['rcid'], $linkInfo['token'] ] ); + &$this->mMarkPatrolledLink, $linkInfo['rcid'] ] ); } } return $this->mMarkPatrolledLink; @@ -511,7 +510,7 @@ class DifferenceEngine extends ContextSource { * Returns an array of meta data needed to build a "mark as patrolled" link and * adds the mediawiki.page.patrol.ajax to the output. * - * @return array|false An array of meta data for a patrol link (rcid & token) + * @return array|false An array of meta data for a patrol link (rcid only) * or false if no link is needed */ protected function getMarkPatrolledLinkInfo() { @@ -561,10 +560,8 @@ class DifferenceEngine extends ContextSource { $this->getOutput()->addModules( 'mediawiki.page.patrol.ajax' ); } - $token = $user->getEditToken( $rcid ); return [ 'rcid' => $rcid, - 'token' => $token, ]; } } diff --git a/includes/page/Article.php b/includes/page/Article.php index 15f5238a9f..9a2a8e2b57 100644 --- a/includes/page/Article.php +++ b/includes/page/Article.php @@ -1184,10 +1184,6 @@ class Article implements Page { return false; } - $rcid = $rc->getAttribute( 'rc_id' ); - - $token = $user->getEditToken( $rcid ); - $outputPage->preventClickjacking(); if ( $wgEnableAPI && $wgEnableWriteAPI && $user->isAllowed( 'writeapi' ) ) { $outputPage->addModules( 'mediawiki.page.patrol.ajax' ); @@ -1199,8 +1195,7 @@ class Article implements Page { [], [ 'action' => 'markpatrolled', - 'rcid' => $rcid, - 'token' => $token, + 'rcid' => $rc->getAttribute( 'rc_id' ), ] ); diff --git a/languages/i18n/en.json b/languages/i18n/en.json index c7599845ab..9541176afc 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -2879,6 +2879,8 @@ "patrol-log-header": "This is a log of patrolled revisions.", "log-show-hide-patrol": "$1 patrol log", "log-show-hide-tag": "$1 tag log", + "confirm-markpatrolled-button": "OK", + "confirm-markpatrolled-top": "Mark revision $3 of $2 as patrolled?", "deletedrevision": "Deleted old revision $1", "filedeleteerror-short": "Error deleting file: $1", "filedeleteerror-long": "Errors were encountered while deleting the file:\n\n$1", diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index fe44a2b13e..1b40ac5075 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -3602,6 +3602,8 @@ "confirm-unwatch-top": "Used as confirmation message.", "confirm-rollback-button": "Used as Submit button text.\n{{Identical|OK}}", "confirm-rollback-top": "Used as confirmation message.", + "confirm-markpatrolled-button": "Used as Submit button text.\n{{Identical|OK}}", + "confirm-markpatrolled-top": "Confirmation message on interstitial form.\n\nParameters:\n* $1 - Target page title\n* $2 - Link to target page with page title as label\n* $3 - Link to recent change diff with revision ID as label", "semicolon-separator": "{{optional}}", "comma-separator": "{{optional}}\n\nWarning: languages have different usages of punctuation, and sometimes they are swapped (e.g. openining and closing quotation marks, or full stop and colon in Armenian), or change their form (the full stop in Chinese and Japanese, the prefered \"colon\" in Armenian used in fact as the regular full stop, the comma in Arabic, Armenian, and Chinese...)\n\nTheir spacing (before or after) may also vary across languages (for example French requires a non-breaking space, preferably narrow if the browser supports NNBSP, on the inner side of some punctuations like quotation/question/exclamation marks, colon, and semicolons).", "colon-separator": "{{optional}}\nChange it only if your language uses another character for ':' or it needs an extra space before the colon.",