Convert action=markpatrolled fallback interface to HTTP POST
authorTimo Tijhof <krinklemail@gmail.com>
Wed, 26 Oct 2016 16:14:21 +0000 (17:14 +0100)
committerKrinkle <krinklemail@gmail.com>
Fri, 4 Nov 2016 23:57:26 +0000 (23:57 +0000)
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

docs/hooks.txt
includes/actions/MarkpatrolledAction.php
includes/diff/DifferenceEngine.php
includes/page/Article.php
languages/i18n/en.json
languages/i18n/qqq.json

index 562d7b4..6668834 100644 (file)
@@ -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
index 85ea87c..8df6044 100644 (file)
@@ -1,7 +1,5 @@
 <?php
 /**
- * Mark a revision as patrolled on a page
- *
  * Copyright © 2011 Alexandre Emsenhuber
  *
  * This program is free software; you can redistribute it and/or modify
  *
  * @ingroup Actions
  */
-class MarkpatrolledAction extends FormlessAction {
+class MarkpatrolledAction extends FormAction {
 
        public function getName() {
                return 'markpatrolled';
        }
 
        protected function getDescription() {
+               // Disable default header "subtitle"
                return '';
        }
 
-       public function onView() {
-               $request = $this->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() {
index 16e9a44..b5a81ea 100644 (file)
@@ -496,12 +496,11 @@ class DifferenceEngine extends ContextSource {
                                                [
                                                        'action' => 'markpatrolled',
                                                        'rcid' => $linkInfo['rcid'],
-                                                       'token' => $linkInfo['token'],
                                                ]
                                        ) . ']</span>';
                                // 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,
                                ];
                        }
                }
index 15f5238..9a2a8e2 100644 (file)
@@ -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' ),
                        ]
                );
 
index c759984..9541176 100644 (file)
        "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",
index fe44a2b..1b40ac5 100644 (file)
        "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.",