From 0a840327110951c64eac4b50a192a9b1d80e468b Mon Sep 17 00:00:00 2001 From: Tim Eulitz Date: Mon, 18 Feb 2019 14:32:05 +0100 Subject: [PATCH] Implement non-JS RollbackAction with form This change prepares a form for the RollbackAction to allow rollbacks to be triggered via POST while also ensuring users are always prompted with a request to confirm the rollback if the rollback confirmation prompt is enabled. Bug: T215303 Change-Id: Iaf7e095b3bb34072eea6bcac76ba29358b14cc09 --- includes/Linker.php | 6 +- includes/actions/RollbackAction.php | 123 ++++++++++++++---- languages/i18n/en.json | 2 +- languages/i18n/qqq.json | 2 +- .../src/mediawiki.rollback.confirmation.js | 18 ++- tests/selenium/pageobjects/history.page.js | 3 + tests/selenium/specs/rollback.js | 27 +++- 7 files changed, 147 insertions(+), 34 deletions(-) diff --git a/includes/Linker.php b/includes/Linker.php index 6f11c1c8b5..711b9dc782 100644 --- a/includes/Linker.php +++ b/includes/Linker.php @@ -1865,21 +1865,25 @@ class Linker { } $title = $rev->getTitle(); + $query = [ 'action' => 'rollback', 'from' => $rev->getUserText(), 'token' => $context->getUser()->getEditToken( 'rollback' ), ]; + $attrs = [ 'data-mw' => 'interface', 'title' => $context->msg( 'tooltip-rollback' )->text(), 'data-rollback-count' => (int)$editCount ]; + $options = [ 'known', 'noclasses' ]; if ( $context->getRequest()->getBool( 'bot' ) ) { + //T17999 + $query['hidediff'] = '1'; $query['bot'] = '1'; - $query['hidediff'] = '1'; // T17999 } $disableRollbackEditCount = false; diff --git a/includes/actions/RollbackAction.php b/includes/actions/RollbackAction.php index 03a5bc840a..e2fc265f96 100644 --- a/includes/actions/RollbackAction.php +++ b/includes/actions/RollbackAction.php @@ -25,7 +25,7 @@ * * @ingroup Actions */ -class RollbackAction extends FormlessAction { +class RollbackAction extends FormAction { public function getName() { return 'rollback'; @@ -35,21 +35,59 @@ class RollbackAction extends FormlessAction { return 'rollback'; } - /** - * Temporarily unused message keys due to T88044/T136375: - * - confirm-rollback-top - * - confirm-rollback-button - * - rollbackfailed - * - rollback-missingparam - * - rollback-success-notify - */ + protected function usesOOUI() { + return true; + } + + protected function getDescription() { + return ''; + } + + public function doesWrites() { + return true; + } + + public function onSuccess() { + return false; + } + + public function onSubmit( $data ) { + return false; + } + + protected function alterForm( HTMLForm $form ) { + $form->setWrapperLegendMsg( 'confirm-rollback-top' ); + $form->setSubmitTextMsg( 'confirm-rollback-button' ); + $form->setTokenSalt( 'rollback' ); + + $from = $this->getRequest()->getVal( 'from' ); + if ( $from === null ) { + throw new BadRequestError( 'rollbackfailed', 'rollback-missingparam' ); + } + foreach ( [ 'from', 'bot', 'hidediff', 'summary', 'token' ] as $param ) { + $val = $this->getRequest()->getVal( $param ); + if ( $val !== null ) { + $form->addHiddenField( $param, $val ); + } + } + } /** * @throws ErrorPageError + * @throws ReadOnlyError + * @throws ThrottledError */ - public function onView() { - // TODO: use $this->useTransactionalTimeLimit(); when POST only - wfTransactionalTimeLimit(); + public function show() { + if ( $this->getUser()->getOption( 'showrollbackconfirmation' ) == false || + $this->getRequest()->wasPosted() ) { + $this->handleRollbackRequest(); + } else { + $this->showRollbackConfirmationForm(); + } + } + + public function handleRollbackRequest() { + $this->enableTransactionalTimelimit(); $request = $this->getRequest(); $user = $this->getUser(); @@ -69,15 +107,6 @@ class RollbackAction extends FormlessAction { ] ); } - // @TODO: remove this hack once rollback uses POST (T88044) - $fname = __METHOD__; - $trxLimits = $this->context->getConfig()->get( 'TrxProfilerLimits' ); - $trxProfiler = Profiler::instance()->getTransactionProfiler(); - $trxProfiler->redefineExpectations( $trxLimits['POST'], $fname ); - DeferredUpdates::addCallableUpdate( function () use ( $trxProfiler, $trxLimits, $fname ) { - $trxProfiler->redefineExpectations( $trxLimits['PostSend-POST'], $fname ); - } ); - $data = null; $errors = $this->page->doRollback( $from, @@ -92,9 +121,7 @@ class RollbackAction extends FormlessAction { throw new ThrottledError; } - if ( isset( $errors[0][0] ) && - ( $errors[0][0] == 'alreadyrolled' || $errors[0][0] == 'cantrollback' ) - ) { + if ( $this->hasRollbackRelatedErrors( $errors ) ) { $this->getOutput()->setPageTitle( $this->msg( 'rollbackfailed' ) ); $errArray = $errors[0]; $errMsg = array_shift( $errArray ); @@ -166,11 +193,51 @@ class RollbackAction extends FormlessAction { } } - protected function getDescription() { - return ''; + /** + * Enables transactional time limit for POST and GET requests to RollbackAction + * @throws ConfigException + */ + private function enableTransactionalTimelimit() { + // If Rollbacks are made POST-only, use $this->useTransactionalTimeLimit() + wfTransactionalTimeLimit(); + if ( !$this->getRequest()->wasPosted() ) { + /** + * We apply the higher POST limits on GET requests + * to prevent logstash.wikimedia.org from being spammed + */ + $fname = __METHOD__; + $trxLimits = $this->context->getConfig()->get( 'TrxProfilerLimits' ); + $trxProfiler = Profiler::instance()->getTransactionProfiler(); + $trxProfiler->redefineExpectations( $trxLimits['POST'], $fname ); + DeferredUpdates::addCallableUpdate( function () use ( $trxProfiler, $trxLimits, $fname + ) { + $trxProfiler->redefineExpectations( $trxLimits['PostSend-POST'], $fname ); + } ); + } } - public function doesWrites() { - return true; + private function showRollbackConfirmationForm() { + $form = $this->getForm(); + if ( $form->show() ) { + $this->onSuccess(); + } + } + + protected function getFormFields() { + return [ + 'intro' => [ + 'type' => 'info', + 'vertical-label' => true, + 'raw' => true, + 'default' => $this->msg( 'confirm-rollback-bottom' )->parse() + ] + ]; + } + + private function hasRollbackRelatedErrors( array $errors ) { + return isset( $errors[0][0] ) && + ( $errors[0][0] == 'alreadyrolled' || + $errors[0][0] == 'cantrollback' + ); } } diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 3b0c524f30..db247b90af 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -2404,7 +2404,6 @@ "revertpage": "Reverted edits by [[Special:Contributions/$2|$2]] ([[User talk:$2|talk]]) to last revision by [[User:$1|$1]]", "revertpage-nouser": "Reverted edits by a hidden user to last revision by {{GENDER:$1|[[User:$1|$1]]}}", "rollback-success": "Reverted edits by {{GENDER:$3|$1}};\nchanged back to last revision by {{GENDER:$4|$2}}.", - "rollback-success-notify": "Reverted edits by $1;\nchanged back to last revision by $2. [$3 Show changes]", "sessionfailure-title": "Session failure", "sessionfailure": "There seems to be a problem with your login session;\nthis action has been canceled as a precaution against session hijacking.\nPlease resubmit the form.", "changecontentmodel" : "Change content model of a page", @@ -3329,6 +3328,7 @@ "confirm-unwatch-top": "Remove this page from your watchlist?", "confirm-rollback-button": "OK", "confirm-rollback-top": "Revert edits to this page?", + "confirm-rollback-bottom": "This action will instantly rollback the selected changes to this page.", "confirm-mcrrestore-title": "Restore a revision", "confirm-mcrundo-title": "Undo a change", "mcrundofailed": "Undo failed", diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index b9b59dae1c..109bba8359 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -2610,7 +2610,6 @@ "revertpage": "Parameters:\n* $1 - username 1\n* $2 - username 2\n* $3 - (Optional) revision ID of the revision reverted to\n* $4 - (Optional) timestamp of the revision reverted to\n* $5 - (Optional) revision ID of the revision reverted from\n* $6 - (Optional) timestamp of the revision reverted from\nSee also:\n* {{msg-mw|Revertpage-nouser}}\n{{Identical|Revert}}", "revertpage-nouser": "This is a confirmation message a user sees after reverting, when the username of the version is hidden with RevisionDelete.\n\nIn other cases the message {{msg-mw|Revertpage}} is used.\n\nParameters:\n* $1 - username 1, can be used for GENDER\n* $2 - (Optional) username 2\n* $3 - (Optional) revision ID of the revision reverted to\n* $4 - (Optional) timestamp of the revision reverted to\n* $5 - (Optional) revision ID of the revision reverted from\n* $6 - (Optional) timestamp of the revision reverted from", "rollback-success": "This message shows up on screen after successful revert (generally visible only to admins). Parameters:\n* $1 - user whose changes have been reverted\n* $2 - user who produced version, which replaces reverted version\n* $3 - the first user's name, can be used for GENDER\n* $4 - the second user's name, can be used for GENDER\n{{Identical|Revert}}\n{{Identical|Rollback}}", - "rollback-success-notify": "Notification shown after a successful revert.\n* $1 - User whose changes have been reverted\n* $2 - User that made the edit that was restored\n* $3 - Url to the diff of the rollback\nSee also:\n* {{msg-mw|showdiff}}\n{{related|rollback-success}}\n{{Format|jquerymsg}}", "sessionfailure-title": "Used as title of the error message {{msg-mw|Sessionfailure}}.", "sessionfailure": "Used as error message.\n\nThe title for this error message is {{msg-mw|Sessionfailure-title}}.", "changecontentmodel": "Title of the change content model special page", @@ -3535,6 +3534,7 @@ "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-rollback-bottom": "Used to describe the rollback action to the user.", "confirm-mcrrestore-title": "Title for the editless restore form.", "confirm-mcrundo-title": "Title for the editless undo form.", "mcrundofailed": "Title of the error page when an editless undo fails.", diff --git a/resources/src/mediawiki.rollback.confirmation.js b/resources/src/mediawiki.rollback.confirmation.js index 55d78d5415..8bf6786fa9 100644 --- a/resources/src/mediawiki.rollback.confirmation.js +++ b/resources/src/mediawiki.rollback.confirmation.js @@ -1,14 +1,28 @@ /*! * JavaScript for rollback confirmation prompt */ -$( function () { +( function () { + + var postRollback = function ( url ) { + var $form = $( '
', { + action: url, + method: 'post' + } ); + $form.appendTo( 'body' ).trigger( 'submit' ); + }; + $( '.mw-rollback-link a' ).each( function () { $( this ).confirmable( { i18n: { confirm: mw.msg( 'rollback-confirmation-confirm', $( this ).data( 'rollback-count' ) ), yes: mw.msg( 'rollback-confirmation-yes' ), no: mw.msg( 'rollback-confirmation-no' ) + }, + handler: function ( e ) { + e.preventDefault(); + postRollback( $( this ).attr( 'href' ) ); } } ); } ); -} ); + +}() ); diff --git a/tests/selenium/pageobjects/history.page.js b/tests/selenium/pageobjects/history.page.js index 6b45e66014..da5e90961e 100644 --- a/tests/selenium/pageobjects/history.page.js +++ b/tests/selenium/pageobjects/history.page.js @@ -6,9 +6,12 @@ class HistoryPage extends Page { get headingText() { return browser.getText( '#firstHeading' ); } get comment() { return browser.element( '#pagehistory .comment' ); } get rollback() { return browser.element( '.mw-rollback-link' ); } + get rollbackLink() { return browser.element( '.mw-rollback-link a' ); } get rollbackConfirmable() { return browser.element( '.mw-rollback-link .jquery-confirmable-text' ); } get rollbackConfirmableYes() { return browser.element( '.mw-rollback-link .jquery-confirmable-button-yes' ); } get rollbackConfirmableNo() { return browser.element( '.mw-rollback-link .jquery-confirmable-button-no' ); } + get rollbackNonJsConfirmable() { return browser.element( '.mw-htmlform .oo-ui-fieldsetLayout-header .oo-ui-labelElement-label' ); } + get rollbackNonJsConfirmableYes() { return browser.element( '.mw-htmlform .mw-htmlform-submit-buttons button' ); } open( title ) { super.openTitle( title, { action: 'history' } ); diff --git a/tests/selenium/specs/rollback.js b/tests/selenium/specs/rollback.js index c52eca1210..9169064232 100644 --- a/tests/selenium/specs/rollback.js +++ b/tests/selenium/specs/rollback.js @@ -66,6 +66,22 @@ describe( 'Rollback with confirmation', function () { return browser.getText( '#firstHeading' ) === 'Action complete'; }, 5000, 'Expected rollback page to appear.' ); } ); + + it( 'should verify rollbacks via GET requests are confirmed on a follow-up page', function () { + var rollbackActionUrl = HistoryPage.rollbackLink.getAttribute( 'href' ); + browser.url( rollbackActionUrl ); + + browser.waitUntil( function () { + return HistoryPage.rollbackNonJsConfirmable.getText() === 'Revert edits to this page?'; + }, 5000, 'Expected rollback confirmation page to appear for GET-based rollbacks.' ); + + HistoryPage.rollbackNonJsConfirmableYes.click(); + + browser.waitUntil( function () { + return browser.getText( '#firstHeading' ) === 'Action complete'; + }, 5000, 'Expected rollback page to appear.' ); + } ); + } ); describe( 'Rollback without confirmation', function () { @@ -103,7 +119,7 @@ describe( 'Rollback without confirmation', function () { HistoryPage.open( name ); } ); - it( 'should perform rollback without asking the user to confirm', function () { + it( 'should perform rollback via POST request without asking the user to confirm', function () { HistoryPage.rollback.click(); // waitUntil indirectly asserts that the content we are looking for is present @@ -111,4 +127,13 @@ describe( 'Rollback without confirmation', function () { return HistoryPage.headingText === 'Action complete'; }, 5000, 'Expected rollback page to appear.' ); } ); + + it( 'should perform rollback via GET request without asking the user to confirm', function () { + var rollbackActionUrl = HistoryPage.rollbackLink.getAttribute( 'href' ); + browser.url( rollbackActionUrl ); + + browser.waitUntil( function () { + return browser.getText( '#firstHeading' ) === 'Action complete'; + }, 5000, 'Expected rollback page to appear.' ); + } ); } ); -- 2.20.1