Tpt has uploaded a new change for review. https://gerrit.wikimedia.org/r/92158
Change subject: Improves EditPage code ...................................................................... Improves EditPage code Removes getContent from ProofreadPagePage (used only on EditPage) Adds also support of validation for bot edits and injection of custom CSS in Page: pages Change-Id: Iac48aa97840facd51d687c03503c6cbe849713bd --- M ProofreadPage.body.php M ProofreadPage.php M includes/page/EditProofreadPagePage.php M includes/page/ProofreadPageContentHandler.php A includes/page/ProofreadPageEditAction.php M includes/page/ProofreadPagePage.php A includes/page/ProofreadPageSubmitAction.php M includes/page/ProofreadPageViewAction.php M tests/includes/page/ProofreadPagePageTest.php 9 files changed, 233 insertions(+), 134 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/ProofreadPage refs/changes/58/92158/1 diff --git a/ProofreadPage.body.php b/ProofreadPage.body.php index 242037b..bb524cc 100644 --- a/ProofreadPage.body.php +++ b/ProofreadPage.body.php @@ -128,10 +128,6 @@ $editor = new EditProofreadIndexPage( $article ); $editor->edit(); return false; - } elseif ( $article->getTitle()->inNamespace( self::getPageNamespaceId() ) ) { - $editor = new EditProofreadPagePage( $article ); - $editor->edit(); - return false; } else { return true; } @@ -684,6 +680,42 @@ } /** + * Make validation of the content in the edit API + * @param $editPage EditPage + * @param $text string + * @param $resultArr array + * @return bool + */ + public static function onAPIEditBeforeSave( EditPage $editPage, $text, array &$resultArr ) { + if ( $editPage->contentModel !== CONTENT_MODEL_PROOFREAD_PAGE ) { + return true; + } + + $article = $editPage->getArticle(); + $user = $article->getContext()->getUser(); + $oldContent = $article->getPage()->getContent( Revision::FOR_THIS_USER, $user ); + $newContent = ContentHandler::makeContent( $text, $editPage->getTitle(), $editPage->contentModel, $editPage->contentFormat ); + + if ( !$newContent->isValid() ) { + $resultArr['badpage'] = wfMessage( 'proofreadpage_badpagetext' )->text(); + return false; + } + + $oldLevel = $oldContent->getLevel(); + $newLevel = $newContent->getLevel(); + //if the user change the level, the change should be allowed and the new User should be the editing user + if ( + !$newLevel->equals( $oldLevel ) && + ( $newLevel->getUser()->getName() !== $user->getName() || !$oldLevel->isChangeAllowed( $newLevel ) ) + ) { + $resultArr['notallowed'] = wfMessage( 'proofreadpage_notallowedtext' )->text(); + return false; + } + + return true; + } + + /** * Add ProofreadPage preferences to the preferences menu * @param $user * @param $preferences array diff --git a/ProofreadPage.php b/ProofreadPage.php index c51d630..58d4cdd 100644 --- a/ProofreadPage.php +++ b/ProofreadPage.php @@ -59,6 +59,8 @@ $wgAutoloadClasses['ProofreadPageContent'] = $dir.'includes/page/ProofreadPageContent.php'; $wgAutoloadClasses['ProofreadPageLevel'] = $dir.'includes/page/ProofreadPageLevel.php'; $wgAutoloadClasses['ProofreadPageContentHandler'] = $dir.'includes/page/ProofreadPageContentHandler.php'; +$wgAutoloadClasses['ProofreadPageEditAction'] = $dir . 'includes/page/ProofreadPageEditAction.php'; +$wgAutoloadClasses['ProofreadPageSubmitAction'] = $dir . 'includes/page/ProofreadPageSubmitAction.php'; $wgAutoloadClasses['ProofreadPageViewAction'] = $dir . 'includes/page/ProofreadPageViewAction.php'; $wgExtensionCredits['other'][] = array( @@ -167,6 +169,7 @@ $wgHooks['CanonicalNamespaces'][] = 'ProofreadPage::addCanonicalNamespaces'; $wgHooks['SkinTemplateNavigation'][] = 'ProofreadPage::onSkinTemplateNavigation'; $wgHooks['ContentHandlerDefaultModelFor'][] = 'ProofreadPage::onContentHandlerDefaultModelFor'; +$wgHooks['APIEditBeforeSave'][] = 'ProofreadPage::onAPIEditBeforeSave'; /** diff --git a/includes/page/EditProofreadPagePage.php b/includes/page/EditProofreadPagePage.php index fe4178a..7a29259 100644 --- a/includes/page/EditProofreadPagePage.php +++ b/includes/page/EditProofreadPagePage.php @@ -22,40 +22,92 @@ class EditProofreadPagePage extends EditPage { /** - * @var ProofreadIndexPage stores the current revision of the page stored in the db + * @var ProofreadPagePage */ - protected $currentPage; + protected $pagePage; /** - * @var ProofreadPageContentHander ContentHandler for Page: pages + * @var Article $article + * @var ProofreadPagePage $pagePage + * @throw MWException */ - protected $contentHandler; - - public function __construct( Article $article ) { + public function __construct( Article $article, ProofreadPagePage $pagePage ) { parent::__construct( $article ); - $this->currentPage = ProofreadPagePage::newFromTitle( $this->mTitle ); + $this->pagePage = $pagePage; - $this->contentModel = CONTENT_MODEL_PROOFREAD_PAGE; - $this->contentHandler = ContentHandler::getForModelID( $this->contentModel ); - $this->contentFormat = $this->contentHandler->getDefaultFormat(); + if ( $this->contentModel !== CONTENT_MODEL_PROOFREAD_PAGE ) { + throw MWException( 'EditProofreadPagePage should only be called on ProofreadPageContent' ); + } } + /** + * @see EditPage::isSectionEditSupported + */ protected function isSectionEditSupported() { return false; // sections and forms don't mix } - protected function showContentForm() { - global $wgOut; + /** + * Load the content before edit + * + * @see EditPage::showContentForm + */ + protected function getContentObject( $def_content = null ) { + //preload content + if ( !$this->mTitle->exists() ) { + $index = $this->pagePage->getIndex(); + if ( $index ) { + $params = array( + 'pagenum' => $index->getDisplayedPageNumber( $this->getTitle() ) + ); + $header = $index->replaceVariablesWithIndexEntries( 'header', $params ); + $body = ''; + $footer = $index->replaceVariablesWithIndexEntries( 'footer', $params ); + //Extract text layer + $image = $index->getImage(); + $pageNumber = $this->pagePage->getPageNumber(); + if ( $image && $pageNumber !== null && $image->exists() ) { + $text = $image->getHandler()->getPageText( $image, $pageNumber ); + if ( $text ) { + $text = preg_replace( "/(\\\\n)/", "\n", $text ); + $body = preg_replace( "/(\\\\\d*)/", '', $text ); + } + } + + return new ProofreadPageContent( + new WikitextContent( $header ), + new WikitextContent( $body ), + new WikitextContent( $footer ), + new ProofreadPageLevel() + ); + } + } + + return parent::getContentObject( $def_content ); + } + + /** + * @see EditPage::showContentForm + */ + protected function showContentForm() { + $out = $this->mArticle->getContext()->getOutput(); $pageLang = $this->mTitle->getPageLanguage(); + + //custom CSS for preview + $css = $this->pagePage->getCustomCss(); + if ( $css !== '' ) { + $out->addInlineStyle( $css ); + } + $inputAttributes = array( 'lang' => $pageLang->getCode(), 'dir' => $pageLang->getDir(), 'cols' => '70' ); - if( wfReadOnly() ) { + if ( wfReadOnly() ) { $inputAttributes['readonly'] = ''; } @@ -77,12 +129,9 @@ 'tabindex' => '1' ); - $content = $this->contentHandler->unserializeContent( $this->textbox1 ); - $page = new ProofreadPagePage( $this->mTitle, $content ); - $content = $page->getContentForEdition(); - - $wgOut->addHTML( - $page->getPageContainerBegin() . + $content = $this->toEditContent( $this->textbox1 ); + $out->addHTML( + $this->pagePage->getPageContainerBegin() . Html::openElement( 'div', array( 'class' => 'prp-page-edit-header' ) ) . Html::element( 'label', array( 'for' => 'wpHeaderTextbox' ), wfMessage( 'proofreadpage_header' )->text() ) . Html::textarea( 'wpHeaderTextbox', $content->getHeader()->serialize(), $headerAttributes ) . @@ -95,29 +144,31 @@ Html::element( 'label', array( 'for' => 'wpFooterTextbox' ), wfMessage( 'proofreadpage_footer' )->text() ) . Html::textarea( 'wpFooterTextbox', $content->getFooter()->serialize(), $footerAttributes ) . Html::closeElement( 'div' ) . - $page->getPageContainerEnd() + $this->pagePage->getPageContainerEnd() ); - $wgOut->addModules( 'ext.proofreadpage.page.edit' ); + $out->addModules( 'ext.proofreadpage.page.edit' ); } /** * Sets the checkboxes for the proofreading status of the page. + * + * @see EditPage::getCheckBoxes */ function getCheckBoxes( &$tabindex, $checked ) { - global $wgUser; - $oldLevel = $this->currentPage->getContent()->getLevel(); + $oldLevel = $this->getCurrentContent()->getLevel(); - $content = $this->contentHandler->unserializeContent( $this->textbox1 ); + $content = $this->toEditContent( $this->textbox1 ); $currentLevel = $content->getLevel(); $qualityLevels = array( 0, 2, 1, 3, 4 ); $html = ''; $checkboxes = parent::getCheckBoxes( $tabindex, $checked ); + $user = $this->mArticle->getContext()->getUser(); foreach( $qualityLevels as $level ) { - $newLevel = new ProofreadPageLevel( $level, $wgUser ); + $newLevel = new ProofreadPageLevel( $level, $user ); if( !$oldLevel->isChangeAllowed( $newLevel ) ) { continue; } @@ -136,7 +187,7 @@ } $checkboxes['wpr-pageStatus'] = ''; - if ( $wgUser->isAllowed( 'pagequality' ) ) { + if ( $user->isAllowed( 'pagequality' ) ) { $checkboxes['wpr-pageStatus'] = Html::openElement( 'span', array( 'id' => 'wpQuality-container' ) ) . $html . @@ -149,7 +200,9 @@ return $checkboxes; } - + /** + * @see EditPage::getSummaryInput + */ function getSummaryInput( $summary = '', $labelText = null, $inputAttrs = null, $spanLabelAttrs = null ) { if ( !$this->mTitle->exists() ) { @@ -160,21 +213,16 @@ } /** - * Extract the page content data from the posted form - * - * @param $request WebRequest - * @todo Support edition by bots. + * @see EditPage::importContentFormData */ protected function importContentFormData( &$request ) { - global $wgUser; - $proofreadingLevel = $request->getInt( 'wpQuality' ); - $oldLevel = $this->currentPage->getContent()->getLevel(); + $oldLevel = $this->getCurrentContent()->getLevel(); $user = ( $oldLevel->getLevel() === $proofreadingLevel ) ? $oldLevel->getUser() - : $wgUser; - if( $oldLevel->getUser() === null ) { - $user = $wgUser; + : $this->mArticle->getContext()->getUser(); + if ( $oldLevel->getUser() === null ) { + $user = $this->mArticle->getContext()->getUser(); } $content = new ProofreadPageContent( @@ -189,13 +237,13 @@ /** * Check the validity of the page + * + * @see EditPage::internalAttemptSave */ public function internalAttemptSave( &$result, $bot = false ) { - global $wgOut; - $error = ''; - $oldContent = $this->currentPage->getContent(); - $newContent = $this->contentHandler->unserializeContent( $this->textbox1 ); + $oldContent = $this->getCurrentContent(); + $newContent = $this->toEditContent( $this->textbox1 ); if ( !$newContent->isValid() ) { $error = 'badpage'; @@ -204,9 +252,8 @@ } if ( $error !== '' ) { - $wgOut->showErrorPage( 'proofreadpage_notallowed', 'proofreadpage_notallowedtext' ); - $status = Status::newGood(); - $status->fatal( 'hookaborted' ); + $this->mArticle->getContext()->getOutput()->showErrorPage( 'proofreadpage_' . $error, 'proofreadpage_' . $error . 'text' ); + $status = Status::newFatal( 'hookaborted' ); $status->value = self::AS_HOOK_ERROR; return $status; } diff --git a/includes/page/ProofreadPageContentHandler.php b/includes/page/ProofreadPageContentHandler.php index f81b6d9..c6b16d1 100644 --- a/includes/page/ProofreadPageContentHandler.php +++ b/includes/page/ProofreadPageContentHandler.php @@ -114,6 +114,8 @@ */ public function getActionOverrides() { return array( + 'edit' => 'ProofreadPageEditAction', + 'submit' => 'ProofreadPageSubmitAction', 'view' => 'ProofreadPageViewAction' ); } diff --git a/includes/page/ProofreadPageEditAction.php b/includes/page/ProofreadPageEditAction.php new file mode 100644 index 0000000..ea3eb34 --- /dev/null +++ b/includes/page/ProofreadPageEditAction.php @@ -0,0 +1,35 @@ +<?php +/** + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * http://www.gnu.org/copyleft/gpl.html + * + * @file + * @ingroup ProofreadPage + */ + +/** + * EditAction for a Page: page + */ +class ProofreadPageEditAction extends EditAction { + + /** + * @see FormlessAction:show + */ + public function show() { + $pagePage = ProofreadPagePage::newFromTitle( $this->getTitle() ); + $editor = new EditProofreadPagePage( $this->page, $pagePage ); + $editor->edit(); + } +} \ No newline at end of file diff --git a/includes/page/ProofreadPagePage.php b/includes/page/ProofreadPagePage.php index 6e23962..452919f 100644 --- a/includes/page/ProofreadPagePage.php +++ b/includes/page/ProofreadPagePage.php @@ -33,12 +33,6 @@ * @var Title */ protected $title; - - /** - * @var ProofreadPageContent content of the page - */ - protected $content; - /** * @var ProofreadIndexPage|null index related to the page */ @@ -51,13 +45,11 @@ /** * Constructor - * @param $title Title Reference to a Title object. - * @param $content ProofreadPageContent content of the page. Warning: only done for EditProofreadPagePage use. - * @param $index ProofreadIndexPage index related to the page. + * @param $title Title Reference to a Title object + * @param $index ProofreadIndexPage index related to the page */ - public function __construct( Title $title, ProofreadPageContent $content = null, ProofreadIndexPage $index = null ) { + public function __construct( Title $title, ProofreadIndexPage $index = null ) { $this->title = $title; - $this->content = $content; $this->index = $index; } @@ -67,8 +59,9 @@ * @return ProofreadPagePage */ public static function newFromTitle( Title $title ) { - return new self( $title, null, null ); + return new self( $title ); } + /** * Returns Title of the index page * @return Title @@ -154,66 +147,6 @@ } /** - * Return content of the page - * @return ProofreadPageContent - */ - public function getContent() { - if ( $this->content === null ) { - $contentHandler = ContentHandler::getForModelID( CONTENT_MODEL_PROOFREAD_PAGE ); - - $rev = Revision::newFromTitle( $this->title ); - if ( $rev === null ) { - $this->content = $contentHandler->makeEmptyContent(); - } else { - $this->content = $rev->getContent(); - if( $this->content->getModel() !== CONTENT_MODEL_PROOFREAD_PAGE ) { - $this->content = $contentHandler->unserializeContent( $this->content->serialize() ); - } - } - } - return $this->content; - } - - /** - * Return content of the page initialised for edition - * @return ProofreadPageContent - */ - public function getContentForEdition() { - $content = $this->getContent(); - - if ( $content->isEmpty() && !$this->title->exists() ) { - $index = $this->getIndex(); - if ( $index ) { - $params = array( - 'pagenum' => $index->getDisplayedPageNumber( $this->getTitle() ) - ); - $header = $index->replaceVariablesWithIndexEntries( 'header', $params ); - $body = ''; - $footer = $index->replaceVariablesWithIndexEntries( 'footer', $params ); - - //Extract text layer - $image = $index->getImage(); - $pageNumber = $this->getPageNumber(); - if ( $image && $pageNumber !== null && $image->exists() ) { - $text = $image->getHandler()->getPageText( $image, $pageNumber ); - if ( $text ) { - $text = preg_replace( "/(\\\\n)/", "\n", $text ); - $body = preg_replace( "/(\\\\\d*)/", '', $text ); - } - } - $content = new ProofreadPageContent( - new WikitextContent( $header ), - new WikitextContent( $body ), - new WikitextContent( $footer ), - new ProofreadPageLevel() - ); - } - } - return $content; - } - - - /** * Return the scan image width for display * @return integer */ @@ -234,15 +167,18 @@ /** * Return custom CSS for the page + * Is protected against XSS * @return string - * @todo use it with ParserOutput::addInlineStyle but fix possible XSS issue */ public function getCustomCss() { $index = $this->getIndex(); if ( $index ) { $css = $index->getIndexEntry( 'css' ); if ( $css !== null ) { - return trim( $css->getStringValue() ); + return strtr( $css->getStringValue(), array( + '&' => '&', + '<' => '<' + ) ); } } diff --git a/includes/page/ProofreadPageSubmitAction.php b/includes/page/ProofreadPageSubmitAction.php new file mode 100644 index 0000000..a1ded79 --- /dev/null +++ b/includes/page/ProofreadPageSubmitAction.php @@ -0,0 +1,40 @@ +<?php +/** + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * http://www.gnu.org/copyleft/gpl.html + * + * @file + * @ingroup ProofreadPage + */ + +/** + * EditAction for a Page: page + */ +class ProofreadPageSubmitAction extends SubmitAction { + + /** + * @see FormlessAction:show + */ + public function show() { + if ( session_id() == '' ) { + // Send a cookie so anons get talk message notifications + wfSetupSession(); + } + + $pagePage = ProofreadPagePage::newFromTitle( $this->getTitle() ); + $editor = new EditProofreadPagePage( $this->page, $pagePage ); + $editor->edit(); + } +} \ No newline at end of file diff --git a/includes/page/ProofreadPageViewAction.php b/includes/page/ProofreadPageViewAction.php index b489fc2..efb5aba 100644 --- a/includes/page/ProofreadPageViewAction.php +++ b/includes/page/ProofreadPageViewAction.php @@ -47,7 +47,7 @@ $this->page->view(); return; } - $page = new ProofreadPagePage( $wikiPage->getTitle(), $content ); + $page = ProofreadPagePage::newFromTitle( $wikiPage->getTitle() ); $out = $this->getOutput(); //render HTML @@ -64,5 +64,11 @@ $out->addJsConfigVars( array( 'prpPageQuality' => $content->getLevel()->getLevel() ) ); + + //custom CSS + $css = $page->getCustomCss(); + if ( $css !== '' ) { + $out->addInlineStyle( $css ); + } } } \ No newline at end of file diff --git a/tests/includes/page/ProofreadPagePageTest.php b/tests/includes/page/ProofreadPagePageTest.php index eed372f..012a4ad 100644 --- a/tests/includes/page/ProofreadPagePageTest.php +++ b/tests/includes/page/ProofreadPagePageTest.php @@ -12,11 +12,11 @@ * @param $index ProofreadIndexPage|null * @return ProofreadPagePage */ - public static function newPagePage( $title = 'test.jpg', ProofreadPageContent $content = null, ProofreadIndexPage $index = null ) { + public static function newPagePage( $title = 'test.jpg', ProofreadIndexPage $index = null ) { if ( is_string( $title ) ) { $title = Title::makeTitle( 250, $title ); } - return new ProofreadPagePage( $title, $content, $index ); + return new ProofreadPagePage( $title, $index ); } public function testGetTitle() { @@ -33,24 +33,22 @@ public function testGetIndex() { $index = ProofreadIndexPageTest::newIndexPage(); - $this->assertEquals( $index, self::newPagePage( 'Test.jpg', null, $index )->getIndex() ); - } - - public function testGetContent() { - $pageContent = ProofreadPageContentTest::newContent( '', 'aaa' ); - $this->assertEquals( $pageContent, self::newPagePage( 'Test.jpg', $pageContent )->getContent() ); + $this->assertEquals( $index, self::newPagePage( 'Test.jpg', $index )->getIndex() ); } public function testGetImageWidth() { $index = ProofreadIndexPageTest::newIndexPage( 'Test', "{{\n|width= 500 \n}}" ); - $this->assertEquals( 500, self::newPagePage( 'Test.jpg', null, $index )->getImageWidth() ); + $this->assertEquals( 500, self::newPagePage( 'Test.jpg', $index )->getImageWidth() ); $index = ProofreadIndexPageTest::newIndexPage( 'Test', "{{\n|title=500\n}}" ); - $this->assertEquals( ProofreadPagePage::DEFAULT_IMAGE_WIDTH, self::newPagePage( 'Test.jpg', null, $index )->getImageWidth() ); + $this->assertEquals( ProofreadPagePage::DEFAULT_IMAGE_WIDTH, self::newPagePage( 'Test.jpg', $index )->getImageWidth() ); } public function testGetCustomCss() { $index = ProofreadIndexPageTest::newIndexPage( 'Test', "{{\n|CSS= width:300px; \n}}" ); - $this->assertEquals( 'width:300px;', self::newPagePage( 'Test.jpg', null, $index )->getCustomCss() ); + $this->assertEquals( 'width:300px;', self::newPagePage( 'Test.jpg', $index )->getCustomCss() ); + + $index = ProofreadIndexPageTest::newIndexPage( 'Test', "{{\n|CSS= width:300px;<style> \n}}" ); + $this->assertEquals( 'width:300px;<style>', self::newPagePage( 'Test.jpg', $index )->getCustomCss() ); } } \ No newline at end of file -- To view, visit https://gerrit.wikimedia.org/r/92158 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iac48aa97840facd51d687c03503c6cbe849713bd Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/ProofreadPage Gerrit-Branch: pagePagesRefactoring Gerrit-Owner: Tpt <thoma...@hotmail.fr> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits