From b706eac8bc1b245ef3fb1030f4b6d2d5e6fcfcbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Dziewo=C5=84ski?= Date: Wed, 19 Aug 2020 20:03:41 +0000 Subject: [PATCH] Re-apply new reply API patches (again) This reverts commit 4d7c98b97c4e8e2d4091f74435b53ce868338848. Change-Id: I4100521efb687ec324d25e273a9c986fd5dac0d0 --- extension.json | 4 + includes/ApiDiscussionTools.php | 109 ++++++++++++++ includes/ApiDiscussionToolsEdit.php | 179 ++++++++++++++++++++++ modules/CommentController.js | 115 +++++---------- modules/controller.js | 204 +++++++++++++++++--------- modules/dt.ui.ReplyWidget.js | 124 +++------------- modules/dt.ui.ReplyWidgetPlain.js | 12 +- modules/dt.ui.ReplyWidgetVisual.js | 12 +- modules/modifier.js | 7 + tests/phpunit/CommentModifierTest.php | 2 + 10 files changed, 512 insertions(+), 256 deletions(-) create mode 100644 includes/ApiDiscussionTools.php create mode 100644 includes/ApiDiscussionToolsEdit.php diff --git a/extension.json b/extension.json index 4a356363e..915a4ecf0 100644 --- a/extension.json +++ b/extension.json @@ -279,6 +279,10 @@ "TestAutoloadNamespaces": { "MediaWiki\\Extension\\DiscussionTools\\Tests\\": "tests/phpunit/" }, + "APIModules": { + "discussiontools": "MediaWiki\\Extension\\DiscussionTools\\ApiDiscussionTools", + "discussiontoolsedit": "MediaWiki\\Extension\\DiscussionTools\\ApiDiscussionToolsEdit" + }, "Hooks": { "BeforePageDisplay": "\\MediaWiki\\Extension\\DiscussionTools\\Hooks::onBeforePageDisplay", "ResourceLoaderGetConfigVars": "\\MediaWiki\\Extension\\DiscussionTools\\Hooks::onResourceLoaderGetConfigVars", diff --git a/includes/ApiDiscussionTools.php b/includes/ApiDiscussionTools.php new file mode 100644 index 000000000..a2831d2d3 --- /dev/null +++ b/includes/ApiDiscussionTools.php @@ -0,0 +1,109 @@ +extractRequestParams(); + $title = Title::newFromText( $params['page'] ); + $result = null; + + if ( !$title ) { + $this->dieWithError( [ 'apierror-invalidtitle', wfEscapeWikiText( $params['page'] ) ] ); + return; + } + + switch ( $params['paction'] ) { + case 'transcludedfrom': + $response = $this->requestRestbasePageHtml( + $this->getValidRevision( $title, $params['oldid'] ?? null ) + ); + + $doc = DOMUtils::parseHTML( $response['body'] ); + $container = $doc->getElementsByTagName( 'body' )->item( 0 ); + '@phan-var DOMElement $container'; + + CommentUtils::unwrapParsoidSections( $container ); + + $parser = CommentParser::newFromGlobalState( $container ); + $comments = $parser->getCommentItems(); + + $transcludedFrom = []; + foreach ( $comments as $comment ) { + $from = $comment->getTranscludedFrom(); + // 'false' is the most likely result, so don't bother sending it, + // the client can just assume it if the key is missing + if ( $from !== false ) { + $transcludedFrom[ $comment->getId() ] = $from; + } + } + + $result = $transcludedFrom; + break; + } + + $this->getResult()->addValue( null, $this->getModuleName(), $result ); + } + + /** + * @inheritDoc + */ + public function getAllowedParams() { + return [ + 'paction' => [ + ParamValidator::PARAM_REQUIRED => true, + ParamValidator::PARAM_TYPE => [ + 'transcludedfrom', + ], + ApiBase::PARAM_HELP_MSG => 'apihelp-visualeditoredit-param-paction', + ], + 'page' => [ + ParamValidator::PARAM_REQUIRED => true, + ApiBase::PARAM_HELP_MSG => 'apihelp-visualeditoredit-param-page', + ], + 'oldid' => null, + ]; + } + + /** + * @inheritDoc + */ + public function needsToken() { + return false; + } + + /** + * @inheritDoc + */ + public function isInternal() { + return true; + } + + /** + * @inheritDoc + */ + public function isWriteMode() { + return false; + } +} diff --git a/includes/ApiDiscussionToolsEdit.php b/includes/ApiDiscussionToolsEdit.php new file mode 100644 index 000000000..147769842 --- /dev/null +++ b/includes/ApiDiscussionToolsEdit.php @@ -0,0 +1,179 @@ +extractRequestParams(); + $title = Title::newFromText( $params['page'] ); + $result = null; + + if ( !$title ) { + $this->dieWithError( [ 'apierror-invalidtitle', wfEscapeWikiText( $params['page'] ) ] ); + return; + } + + switch ( $params['paction'] ) { + case 'addcomment': + // Fetch the latest revision + $revision = $this->getLatestRevision( $title ); + $oldid = $revision->getId(); + $response = $this->requestRestbasePageHtml( $revision ); + $headers = $response['headers']; + + $doc = DOMUtils::parseHTML( $response['body'] ); + $container = $doc->getElementsByTagName( 'body' )->item( 0 ); + '@phan-var DOMElement $container'; + + $commentId = $params['commentid'] ?? null; + + if ( !$commentId ) { + $this->dieWithError( [ 'apierror-missingparam', 'commentid' ] ); + } + + $parser = CommentParser::newFromGlobalState( $container ); + $comment = $parser->findCommentById( $commentId ); + if ( !$comment ) { + $this->dieWithError( [ 'apierror-discussiontools-commentid-notfound', $commentId ] ); + return; + } + + $this->requireOnlyOneParameter( $params, 'wikitext', 'html' ); + + if ( $params['wikitext'] !== null ) { + CommentModifier::addWikitextReply( $comment, $params['wikitext'] ); + } else { + CommentModifier::addHtmlReply( $comment, $params['html'] ); + } + + $heading = $comment->getHeading(); + if ( $heading->isPlaceholderHeading() ) { + // This comment is in 0th section, there's no section title for the edit summary + $summaryPrefix = ''; + } else { + $summaryPrefix = '/* ' . $heading->getRange()->startContainer->textContent . ' */ '; + } + $summary = $summaryPrefix . + $this->msg( 'discussiontools-defaultsummary-reply' )->inContentLanguage()->text(); + + $api = new ApiMain( + new DerivativeRequest( + $this->getRequest(), + [ + 'action' => 'visualeditoredit', + 'paction' => 'save', + 'page' => $params['page'], + 'token' => $params['token'], + 'oldid' => $oldid, + 'html' => DOMCompat::getOuterHTML( $doc->documentElement ), + 'summary' => $summary, + 'baserevid' => $revision->getId(), + 'starttimestamp' => wfTimestampNow(), + 'etag' => $headers['etag'], + 'watchlist' => $params['watchlist'], + 'captchaid' => $params['captchaid'], + 'captchaword' => $params['captchaword'] + ], + /* was posted? */ true + ), + /* enable write? */ true + ); + + $api->execute(); + + // TODO: Tags are only added by 'dttags' existing on the original request + // context (see Hook::onRecentChangeSave). What tags (if any) should be + // added in this API? + + $data = $api->getResult()->getResultData(); + $result = $data['visualeditoredit']; + break; + } + + $this->getResult()->addValue( null, $this->getModuleName(), $result ); + } + + /** + * @inheritDoc + */ + public function getAllowedParams() { + return [ + 'paction' => [ + ParamValidator::PARAM_REQUIRED => true, + ParamValidator::PARAM_TYPE => [ + 'addcomment', + ], + ApiBase::PARAM_HELP_MSG => 'apihelp-visualeditoredit-param-paction', + ], + 'page' => [ + ParamValidator::PARAM_REQUIRED => true, + ApiBase::PARAM_HELP_MSG => 'apihelp-visualeditoredit-param-page', + ], + 'token' => [ + ParamValidator::PARAM_REQUIRED => true, + ], + 'commentid' => null, + 'wikitext' => [ + ParamValidator::PARAM_TYPE => 'text', + ParamValidator::PARAM_DEFAULT => null, + ], + 'html' => [ + ParamValidator::PARAM_TYPE => 'text', + ParamValidator::PARAM_DEFAULT => null, + ], + 'watchlist' => [ + ApiBase::PARAM_HELP_MSG => 'apihelp-edit-param-watchlist', + ], + 'captchaid' => [ + ApiBase::PARAM_HELP_MSG => 'apihelp-visualeditoredit-param-captchaword', + ], + 'captchaword' => [ + ApiBase::PARAM_HELP_MSG => 'apihelp-visualeditoredit-param-captchaword', + ], + ]; + } + + /** + * @inheritDoc + */ + public function needsToken() { + return 'csrf'; + } + + /** + * @inheritDoc + */ + public function isInternal() { + return true; + } + + /** + * @inheritDoc + */ + public function isWriteMode() { + return true; + } +} diff --git a/modules/CommentController.js b/modules/CommentController.js index bbd56f961..1468b68bd 100644 --- a/modules/CommentController.js +++ b/modules/CommentController.js @@ -76,13 +76,13 @@ function getLatestRevId( pageName ) { } /** - * Like #getParsoidCommentData, but assumes the comment was found on the current page, + * Like #checkCommentOnPage, but assumes the comment was found on the current page, * and then follows transclusions to determine the source page where it is written. * - * @param {string} commentId Comment ID, from a comment parsed in the local document - * @return {jQuery.Promise} + * @param {string} commentId Comment ID + * @return {jQuery.Promise} Promise which resolves with pageName+oldId, or rejects with an error */ -function getParsoidTranscludedCommentData( commentId ) { +function getTranscludedFromSource( commentId ) { var promise, pageName = mw.config.get( 'wgRelevantPageName' ), oldId = mw.config.get( 'wgCurRevisionId' ); @@ -94,7 +94,7 @@ function getParsoidTranscludedCommentData( commentId ) { if ( errorData.follow && typeof errorData.transcludedFrom === 'string' ) { return getLatestRevId( errorData.transcludedFrom ).then( function ( latestRevId ) { // Fetch the transcluded page, until we cross the recursion limit - return controller.getParsoidCommentData( errorData.transcludedFrom, latestRevId, commentId ) + return controller.checkCommentOnPage( errorData.transcludedFrom, latestRevId, commentId ) .catch( followTransclusion.bind( null, recursionLimit - 1 ) ); } ); } @@ -104,7 +104,7 @@ function getParsoidTranscludedCommentData( commentId ) { // Arbitrary limit of 10 steps, which should be more than anyone could ever need // (there are reasonable use cases for at least 2) - promise = controller.getParsoidCommentData( pageName, oldId, commentId ) + promise = controller.checkCommentOnPage( pageName, oldId, commentId ) .catch( followTransclusion.bind( null, 10 ) ); return promise; @@ -127,7 +127,7 @@ CommentController.prototype.onReplyLinkClick = function ( e ) { * @param {string} [mode] Optionally force a mode, 'visual' or 'source' */ CommentController.prototype.setup = function ( mode ) { - var parsoidPromise, + var comment = this.comment, commentController = this; if ( mode === undefined ) { @@ -165,10 +165,8 @@ CommentController.prototype.setup = function ( mode ) { this.$replyLinkButtons.addClass( 'dt-init-replylink-active' ); if ( !this.replyWidgetPromise ) { - parsoidPromise = getParsoidTranscludedCommentData( this.comment.id ); - - this.replyWidgetPromise = parsoidPromise.then( function ( parsoidData ) { - return commentController.createReplyWidget( parsoidData, mode === 'visual' ); + this.replyWidgetPromise = getTranscludedFromSource( comment.id ).then( function ( pageData ) { + return commentController.createReplyWidget( comment, pageData.pageName, pageData.oldId, mode === 'visual' ); }, function ( code, data ) { commentController.teardown(); @@ -188,14 +186,14 @@ CommentController.prototype.setup = function ( mode ) { } ); // On first load, add a placeholder list item - commentController.newListItem = modifier.addListItem( commentController.comment ); + commentController.newListItem = modifier.addListItem( comment ); $( commentController.newListItem ).text( mw.msg( 'discussiontools-replywidget-loading' ) ); } commentController.replyWidgetPromise.then( function ( replyWidget ) { if ( !commentController.newListItem ) { // On subsequent loads, there's no list item yet, so create one now - commentController.newListItem = modifier.addListItem( commentController.comment ); + commentController.newListItem = modifier.addListItem( comment ); } $( commentController.newListItem ).empty().append( replyWidget.$element ); @@ -216,11 +214,11 @@ CommentController.prototype.getReplyWidgetClass = function ( visual ) { } ); }; -CommentController.prototype.createReplyWidget = function ( parsoidData, visual ) { +CommentController.prototype.createReplyWidget = function ( comment, pageName, oldId, visual ) { var commentController = this; return this.getReplyWidgetClass( visual ).then( function ( ReplyWidget ) { - return new ReplyWidget( commentController, parsoidData ); + return new ReplyWidget( commentController, comment, pageName, oldId ); } ); }; @@ -255,52 +253,32 @@ CommentController.prototype.teardown = function ( abandoned ) { } }; -CommentController.prototype.postReply = function ( comment ) { - if ( this.replyWidget.getMode() === 'source' ) { - modifier.addWikitextReply( comment, this.replyWidget.getValue() ); - } else { - modifier.addHtmlReply( comment, this.replyWidget.getValue() ); - } -}; - -CommentController.prototype.save = function ( parsoidData ) { - var heading, summaryPrefix, summary, savePromise, - mode = this.replyWidget.getMode(), - comment = parsoidData.comment, - pageData = parsoidData.pageData, +CommentController.prototype.save = function ( comment, pageName ) { + var replyWidget = this.replyWidget, commentController = this; - // Update the Parsoid DOM - this.postReply( parsoidData.comment ); - - heading = comment.getHeading(); - if ( heading.placeholderHeading ) { - // This comment is in 0th section, there's no section title for the edit summary - summaryPrefix = ''; - } else { - summaryPrefix = '/* ' + heading.range.startContainer.innerText + ' */ '; - } - - summary = summaryPrefix + mw.msg( 'discussiontools-defaultsummary-reply' ); - return this.replyWidget.checkboxesPromise.then( function ( checkboxes ) { var captchaInput = commentController.replyWidget.captchaInput, data = { - page: pageData.pageName, - oldid: pageData.oldId, - summary: summary, - baserevid: pageData.oldId, - starttimestamp: pageData.startTimeStamp, - etag: pageData.etag, + action: 'discussiontoolsedit', + paction: 'addcomment', + page: pageName, + commentid: comment.id, assert: mw.user.isAnon() ? 'anon' : 'user', assertuser: mw.user.getName() || undefined, dttags: [ 'discussiontools', 'discussiontools-reply', - 'discussiontools-' + mode + 'discussiontools-' + replyWidget.getMode() ].join( ',' ) }; + if ( replyWidget.getMode() === 'source' ) { + data.wikitext = replyWidget.getValue(); + } else { + data.html = replyWidget.getValue(); + } + if ( captchaInput ) { data.captchaid = captchaInput.getCaptchaId(); data.captchaword = captchaInput.getCaptchaWord(); @@ -312,8 +290,7 @@ CommentController.prototype.save = function ( parsoidData ) { 'unwatch'; } - savePromise = mw.libs.ve.targetSaver.saveDoc( - parsoidData.doc, + return mw.libs.ve.targetSaver.postContent( data, { // No timeout. Huge talk pages take a long time to save, and falsely reporting an error can @@ -321,32 +298,17 @@ CommentController.prototype.save = function ( parsoidData ) { api: new mw.Api( { ajax: { timeout: 0 }, parameters: { formatversion: 2 } } ) } ).catch( function ( code, data ) { - // Handle edit conflicts. Load the latest revision of the page, then try again. If the parent - // comment has been deleted from the page, or if retry also fails for some other reason, the - // error is handled as normal below. - if ( code === 'editconflict' ) { - return getLatestRevId( pageData.pageName ).then( function ( latestRevId ) { - return controller.getParsoidCommentData( pageData.pageName, latestRevId, comment.id ).then( function ( parsoidData ) { - return commentController.save( parsoidData ); - } ); - } ); + // Better user-facing error message + if ( code === 'discussiontools-commentid-notfound' ) { + return $.Deferred().reject( 'discussiontools-commentid-notfound', { errors: [ { + code: 'discussiontools-commentid-notfound', + html: mw.message( 'discussiontools-error-comment-disappeared' ).parse() + } ] } ).promise(); } return $.Deferred().reject( code, data ).promise(); + } ).then( function ( data ) { + controller.update( data, comment, pageName, replyWidget ); } ); - savePromise.then( function () { - var watch; - // Update watch link to match 'watch checkbox' in save dialog. - // User logged in if module loaded. - if ( mw.loader.getState( 'mediawiki.page.watch.ajax' ) === 'ready' ) { - watch = require( 'mediawiki.page.watch.ajax' ); - watch.updateWatchLink( - // eslint-disable-next-line no-jquery/no-global-selector - $( '#ca-watch a, #ca-unwatch a' ), - data.watchlist === 'watch' ? 'unwatch' : 'watch' - ); - } - } ); - return savePromise; } ); }; @@ -359,7 +321,7 @@ CommentController.prototype.switchToWikitext = function () { // TODO: We may need to pass oldid/etag when editing is supported wikitextPromise = target.getWikitextFragment( target.getSurface().getModel().getDocument() ); - this.replyWidgetPromise = this.createReplyWidget( oldWidget.parsoidData, false ); + this.replyWidgetPromise = this.createReplyWidget( oldWidget.comment, oldWidget.pageName, oldWidget.oldId, false ); return $.when( wikitextPromise, this.replyWidgetPromise ).then( function ( wikitext, replyWidget ) { wikitext = modifier.sanitizeWikitextLinebreaks( wikitext ); @@ -386,7 +348,6 @@ CommentController.prototype.switchToVisual = function () { var parsePromise, oldWidget = this.replyWidget, wikitext = oldWidget.getValue(), - pageData = oldWidget.parsoidData.pageData, commentController = this; wikitext = modifier.sanitizeWikitextLinebreaks( wikitext ); @@ -408,7 +369,7 @@ CommentController.prototype.switchToVisual = function () { parsePromise = api.post( { action: 'visualeditor', paction: 'parsefragment', - page: pageData.pageName, + page: oldWidget.pageName, wikitext: wikitext, pst: true } ).then( function ( response ) { @@ -417,7 +378,7 @@ CommentController.prototype.switchToVisual = function () { } else { parsePromise = $.Deferred().resolve( '' ).promise(); } - this.replyWidgetPromise = this.createReplyWidget( oldWidget.parsoidData, true ); + this.replyWidgetPromise = this.createReplyWidget( oldWidget.comment, oldWidget.pageName, oldWidget.oldId, true ); return $.when( parsePromise, this.replyWidgetPromise ).then( function ( html, replyWidget ) { var doc, bodyChildren, type, $msg, diff --git a/modules/controller.js b/modules/controller.js index 3e6b87f8c..aad467bf6 100644 --- a/modules/controller.js +++ b/modules/controller.js @@ -4,6 +4,7 @@ var api = new mw.Api( { parameters: { formatversion: 2 } } ), $pageContainer, Parser = require( './Parser.js' ), + logger = require( './logger.js' ), pageDataCache = {}; mw.messages.set( require( './controller/contLangMessages.json' ) ); @@ -47,20 +48,17 @@ function highlight( comment ) { } /** - * Get the Parsoid document HTML and metadata needed to edit this page from the API. + * Get various pieces of page metadata. * * This method caches responses. If you call it again with the same parameters, you'll get the exact * same Promise object, and no API request will be made. * - * TODO: Resolve the naming conflict between this raw "pageData" from the API, and the - * plain object "pageData" that gets attached to parsoidData. - * * @param {string} pageName Page title * @param {number} oldId Revision ID * @return {jQuery.Promise} */ function getPageData( pageName, oldId ) { - var lintPromise; + var lintPromise, transcludedFromPromise, veMetadataPromise; pageDataCache[ pageName ] = pageDataCache[ pageName ] || {}; if ( pageDataCache[ pageName ][ oldId ] ) { return pageDataCache[ pageName ][ oldId ]; @@ -76,70 +74,57 @@ function getPageData( pageName, oldId ) { return OO.getProp( response, 'query', 'linterrors' ) || []; } ); - pageDataCache[ pageName ][ oldId ] = mw.loader.using( 'ext.visualEditor.targetLoader' ).then( function () { - var pageDataPromise = mw.libs.ve.targetLoader.requestPageData( - 'visual', pageName, { oldId: oldId } - ); - return $.when( lintPromise, pageDataPromise ).then( function ( linterrors, pageData ) { - pageData.linterrors = linterrors; - return pageData; - } ); - }, function () { - // Clear on failure - pageDataCache[ pageName ][ oldId ] = null; + transcludedFromPromise = api.get( { + action: 'discussiontools', + paction: 'transcludedfrom', + page: pageName, + oldid: oldId + } ).then( function ( response ) { + return OO.getProp( response, 'discussiontools' ) || []; } ); + + veMetadataPromise = api.get( { + action: 'visualeditor', + paction: 'metadata', + page: pageName + } ).then( function ( response ) { + return OO.getProp( response, 'visualeditor' ) || []; + } ); + + pageDataCache[ pageName ][ oldId ] = $.when( lintPromise, transcludedFromPromise, veMetadataPromise ) + .then( function ( linterrors, transcludedfrom, metadata ) { + return { + linterrors: linterrors, + transcludedfrom: transcludedfrom, + metadata: metadata + }; + }, function () { + // Clear on failure + pageDataCache[ pageName ][ oldId ] = null; + } ); return pageDataCache[ pageName ][ oldId ]; } /** - * Get the Parsoid document DOM, parse comments and threads, and find a specific comment in it. + * Check if a given comment on a page can be replied to * * @param {string} pageName Page title * @param {number} oldId Revision ID * @param {string} commentId Comment ID - * @return {jQuery.Promise} + * @return {jQuery.Promise} Resolves with the pageName+oldId if the comment appears on the page. + * Rejects with error data if the comment is transcluded, or there are lint errors on the page. */ -function getParsoidCommentData( pageName, oldId, commentId ) { - var parsoidPageData, parsoidDoc; - +function checkCommentOnPage( pageName, oldId, commentId ) { return getPageData( pageName, oldId ) .then( function ( response ) { - var data, comment, transcludedFrom, transcludedErrMsg, mwTitle, follow, - lintType, parser, - lintErrors = response.linterrors; + var isTranscludedFrom, transcludedErrMsg, mwTitle, follow, + lintType, + lintErrors = response.linterrors, + transcludedFrom = response.transcludedfrom; - data = response.visualeditor; - parsoidDoc = ve.parseXhtml( data.content ); - // Remove section wrappers, they interfere with transclusion handling - mw.libs.ve.unwrapParsoidSections( parsoidDoc.body ); - // Mirror VE's ve.init.mw.Target.prototype.fixBase behavior: - ve.fixBase( parsoidDoc, document, ve.resolveUrl( - // Don't replace $1 with the page name, because that'll break if - // the page name contains a slash - mw.config.get( 'wgArticlePath' ).replace( '$1', '' ), - document - ) ); - - parsoidPageData = { - pageName: pageName, - oldId: oldId, - startTimeStamp: data.starttimestamp, - etag: data.etag - }; - - parser = new Parser( parsoidDoc.body ); - comment = parser.findCommentById( commentId ); - - if ( !comment ) { - return $.Deferred().reject( 'comment-disappeared', { errors: [ { - code: 'comment-disappeared', - html: mw.message( 'discussiontools-error-comment-disappeared' ).parse() - } ] } ).promise(); - } - - transcludedFrom = comment.getTranscludedFrom(); - if ( transcludedFrom ) { - mwTitle = transcludedFrom === true ? null : mw.Title.newFromText( transcludedFrom ); + isTranscludedFrom = transcludedFrom[ commentId ]; + if ( isTranscludedFrom ) { + mwTitle = isTranscludedFrom === true ? null : mw.Title.newFromText( isTranscludedFrom ); // If this refers to a template rather than a subpage, we never want to edit it follow = mwTitle && mwTitle.getNamespaceId() !== mw.config.get( 'wgNamespaceIds' ).template; @@ -158,7 +143,7 @@ function getParsoidCommentData( pageName, oldId, commentId ) { return $.Deferred().reject( 'comment-is-transcluded', { errors: [ { data: { - transcludedFrom: transcludedFrom, + transcludedFrom: isTranscludedFrom, follow: follow }, code: 'comment-is-transcluded', @@ -180,19 +165,18 @@ function getParsoidCommentData( pageName, oldId, commentId ) { } return { - comment: comment, - doc: parsoidDoc, - pageData: parsoidPageData + pageName: pageName, + oldId: oldId }; } ); } -function getCheckboxesPromise( pageData ) { +function getCheckboxesPromise( pageName, oldId ) { return getPageData( - pageData.pageName, - pageData.oldId - ).then( function ( response ) { - var data = response.visualeditor, + pageName, + oldId + ).then( function ( pageData ) { + var data = pageData.metadata, checkboxesDef = {}; mw.messages.set( data.checkboxesMessages ); @@ -230,7 +214,7 @@ function init( $container, state ) { highlight( repliedToComment.replies[ repliedToComment.replies.length - 1 ] ); } - // Preload the Parsoid document. + // Preload page metadata. // TODO: Isn't this too early to load it? We will only need it if the user tries replying... getPageData( mw.config.get( 'wgRelevantPageName' ), @@ -238,8 +222,94 @@ function init( $container, state ) { ); } +function update( data, comment, pageName, replyWidget ) { + var watch, + pageUpdated = $.Deferred(); + + replyWidget.teardown(); + // TODO: Tell controller to teardown all other open widgets + + // Update page state + if ( pageName === mw.config.get( 'wgRelevantPageName' ) ) { + // We can use the result from the VisualEditor API + $pageContainer.html( data.content ); + mw.config.set( { + wgCurRevisionId: data.newrevid, + wgRevisionId: data.newrevid + } ); + mw.config.set( data.jsconfigvars ); + // Note: VE API merges 'modules' and 'modulestyles' + mw.loader.load( data.modules ); + // TODO update categories, displaytitle, lastmodified + // (see ve.init.mw.DesktopArticleTarget.prototype.replacePageContent) + + pageUpdated.resolve(); + + } else { + // We saved to another page, we must purge and then fetch the current page + api.post( { + action: 'purge', + titles: mw.config.get( 'wgRelevantPageName' ) + } ).then( function () { + return api.get( { + action: 'parse', + prop: [ 'text', 'modules', 'jsconfigvars' ], + page: mw.config.get( 'wgRelevantPageName' ) + } ); + } ).then( function ( parseResp ) { + $pageContainer.html( parseResp.parse.text ); + mw.config.set( parseResp.parse.jsconfigvars ); + mw.loader.load( parseResp.parse.modulestyles ); + mw.loader.load( parseResp.parse.modules ); + // TODO update categories, displaytitle, lastmodified + // We may not be able to use prop=displaytitle without making changes in the action=parse API, + // VE API has some confusing code that changes the HTML escaping on it before returning??? + + pageUpdated.resolve(); + + } ).catch( function () { + // We saved the reply, but couldn't purge or fetch the updated page. Seems difficult to + // explain this problem. Redirect to the page where the user can at least see their reply… + window.location = mw.util.getUrl( pageName ); + } ); + } + + // Update watch link to match 'watch checkbox' in save dialog. + // User logged in if module loaded. + if ( mw.loader.getState( 'mediawiki.page.watch.ajax' ) === 'ready' ) { + watch = require( 'mediawiki.page.watch.ajax' ); + watch.updateWatchLink( + // eslint-disable-next-line no-jquery/no-global-selector + $( '#ca-watch a, #ca-unwatch a' ), + data.watchlist === 'watch' ? 'unwatch' : 'watch' + ); + } + + pageUpdated.then( function () { + // Re-initialize and highlight the new reply. + mw.dt.initState.repliedTo = comment.id; + + // We need our init code to run after everyone else's handlers for this hook, + // so that all changes to the page layout have been completed (e.g. collapsible elements), + // and we can measure things and display the highlight in the right place. + mw.hook( 'wikipage.content' ).remove( mw.dt.init ); + mw.hook( 'wikipage.content' ).fire( $pageContainer ); + // The hooks have "memory" so calling add() after fire() actually fires the handler, + // and calling add() before fire() would actually fire it twice. + mw.hook( 'wikipage.content' ).add( mw.dt.init ); + + logger( { + action: 'saveSuccess', + // eslint-disable-next-line camelcase + revision_id: data.newrevid + } ); + } ); + +} + module.exports = { init: init, - getParsoidCommentData: getParsoidCommentData, + update: update, + checkCommentOnPage: checkCommentOnPage, getCheckboxesPromise: getCheckboxesPromise }; diff --git a/modules/dt.ui.ReplyWidget.js b/modules/dt.ui.ReplyWidget.js index 6788bbe66..39dc12b0a 100644 --- a/modules/dt.ui.ReplyWidget.js +++ b/modules/dt.ui.ReplyWidget.js @@ -5,6 +5,7 @@ var controller = require( 'ext.discussionTools.init' ).controller, /** * @external CommentController + * @external CommentItem */ /** @@ -14,15 +15,15 @@ var controller = require( 'ext.discussionTools.init' ).controller, * @extends OO.ui.Widget * @constructor * @param {CommentController} commentController Comment controller - * @param {Object} parsoidData Result from controller#getParsoidCommentData + * @param {CommentItem} comment Comment item + * @param {string} pageName Page name the reply is being saved to + * @param {number} oldId Revision ID of page at time of editing * @param {Object} [config] Configuration options * @param {Object} [config.input] Configuration options for the comment input widget */ -function ReplyWidget( commentController, parsoidData, config ) { +function ReplyWidget( commentController, comment, pageName, oldId, config ) { var returnTo, contextNode, inputConfig, - widget = this, - pageData = parsoidData.pageData, - comment = parsoidData.comment; + widget = this; config = config || {}; @@ -31,7 +32,9 @@ function ReplyWidget( commentController, parsoidData, config ) { this.pending = false; this.commentController = commentController; - this.parsoidData = parsoidData; + this.comment = comment; + this.pageName = pageName; + this.oldId = oldId; contextNode = utils.closestElement( comment.range.endContainer, [ 'dl', 'ul', 'ol' ] ); this.context = contextNode ? contextNode.nodeName.toLowerCase() : 'dl'; // TODO: Should storagePrefix include pageName? @@ -92,9 +95,9 @@ function ReplyWidget( commentController, parsoidData, config ) { this.replyButton.$element ); this.$footer = $( '
' ).addClass( 'dt-ui-replyWidget-footer' ); - if ( pageData.pageName !== mw.config.get( 'wgRelevantPageName' ) ) { + if ( this.pageName !== mw.config.get( 'wgRelevantPageName' ) ) { this.$footer.append( $( '

' ).append( - mw.message( 'discussiontools-replywidget-transcluded', pageData.pageName ).parseDom() + mw.message( 'discussiontools-replywidget-transcluded', this.pageName ).parseDom() ) ); } this.$footer.append( @@ -156,7 +159,7 @@ function ReplyWidget( commentController, parsoidData, config ) { this.$actionsWrapper.detach(); } - this.checkboxesPromise = controller.getCheckboxesPromise( this.parsoidData.pageData ); + this.checkboxesPromise = controller.getCheckboxesPromise( this.pageName, this.oldId ); this.checkboxesPromise.then( function ( checkboxes ) { var name; function trackCheckbox( name ) { @@ -207,6 +210,9 @@ ReplyWidget.prototype.clear = function () { if ( this.errorMessage ) { this.errorMessage.$element.remove(); } + this.$preview.empty(); + this.storage.remove( this.storagePrefix + '/mode' ); + this.storage.remove( this.storagePrefix + '/saveable' ); }; ReplyWidget.prototype.setPending = function ( pending ) { @@ -334,9 +340,6 @@ ReplyWidget.prototype.tryTeardown = function () { ReplyWidget.prototype.teardown = function ( abandoned ) { this.unbindBeforeUnloadHandler(); this.clear(); - this.storage.remove( this.storagePrefix + '/mode' ); - this.storage.remove( this.storagePrefix + '/saveable' ); - this.$preview.empty(); this.emit( 'teardown', abandoned ); return this; }; @@ -400,7 +403,7 @@ ReplyWidget.prototype.preparePreview = function ( wikitext ) { text: wikitext, pst: true, prop: [ 'text', 'modules', 'jsconfigvars' ], - title: mw.config.get( 'wgPageName' ) + title: this.pageName } ); } // TODO: Add list context @@ -474,8 +477,8 @@ ReplyWidget.prototype.onUnload = function () { ReplyWidget.prototype.onReplyClick = function () { var widget = this, - pageData = this.parsoidData.pageData, - comment = this.parsoidData.comment; + pageName = this.pageName, + comment = this.comment; if ( this.pending || this.isEmpty() ) { return; @@ -490,91 +493,8 @@ ReplyWidget.prototype.onReplyClick = function () { logger( { action: 'saveIntent' } ); // TODO: When editing a transcluded page, VE API returning the page HTML is a waste, since we won't use it - - // We must get a new copy of the document every time, otherwise any unsaved replies will pile up - // TODO: Move most of this logic to the CommentController - controller.getParsoidCommentData( - pageData.pageName, - pageData.oldId, - comment.id - ).then( function ( parsoidData ) { - logger( { action: 'saveAttempt' } ); - return widget.commentController.save( parsoidData ); - } ).then( function ( data ) { - var - pageUpdated = $.Deferred(), - // eslint-disable-next-line no-jquery/no-global-selector - $container = $( '#mw-content-text' ); - - widget.teardown(); - // TODO: Tell controller to teardown all other open widgets - - // Update page state - if ( pageData.pageName === mw.config.get( 'wgRelevantPageName' ) ) { - // We can use the result from the VisualEditor API - $container.html( data.content ); - mw.config.set( { - wgCurRevisionId: data.newrevid, - wgRevisionId: data.newrevid - } ); - mw.config.set( data.jsconfigvars ); - // Note: VE API merges 'modules' and 'modulestyles' - mw.loader.load( data.modules ); - // TODO update categories, displaytitle, lastmodified - // (see ve.init.mw.DesktopArticleTarget.prototype.replacePageContent) - - pageUpdated.resolve(); - - } else { - // We saved to another page, we must purge and then fetch the current page - widget.api.post( { - action: 'purge', - titles: mw.config.get( 'wgRelevantPageName' ) - } ).then( function () { - return widget.api.get( { - action: 'parse', - prop: [ 'text', 'modules', 'jsconfigvars' ], - page: mw.config.get( 'wgRelevantPageName' ) - } ); - } ).then( function ( parseResp ) { - $container.html( parseResp.parse.text ); - mw.config.set( parseResp.parse.jsconfigvars ); - mw.loader.load( parseResp.parse.modulestyles ); - mw.loader.load( parseResp.parse.modules ); - // TODO update categories, displaytitle, lastmodified - // We may not be able to use prop=displaytitle without making changes in the action=parse API, - // VE API has some confusing code that changes the HTML escaping on it before returning??? - - pageUpdated.resolve(); - - } ).catch( function () { - // We saved the reply, but couldn't purge or fetch the updated page. Seems difficult to - // explain this problem. Redirect to the page where the user can at least see their reply… - window.location = mw.util.getUrl( pageData.pageName ); - } ); - } - - pageUpdated.then( function () { - // Re-initialize and highlight the new reply. - mw.dt.initState.repliedTo = comment.id; - - // We need our init code to run after everyone else's handlers for this hook, - // so that all changes to the page layout have been completed (e.g. collapsible elements), - // and we can measure things and display the highlight in the right place. - mw.hook( 'wikipage.content' ).remove( mw.dt.init ); - mw.hook( 'wikipage.content' ).fire( $container ); - // The hooks have "memory" so calling add() after fire() actually fires the handler, - // and calling add() before fire() would actually fire it twice. - mw.hook( 'wikipage.content' ).add( mw.dt.init ); - - logger( { - action: 'saveSuccess', - // eslint-disable-next-line camelcase - revision_id: data.newrevid - } ); - } ); - - }, function ( code, data ) { + logger( { action: 'saveAttempt' } ); + widget.commentController.save( comment, pageName ).fail( function ( code, data ) { var typeMap = { // Compare to ve.init.mw.ArticleTargetEvents.js in VisualEditor. editconflict: 'editConflict', @@ -598,11 +518,11 @@ ReplyWidget.prototype.onReplyClick = function () { } widget.captchaInput = undefined; - if ( OO.getProp( data, 'visualeditoredit', 'edit', 'captcha' ) ) { + if ( OO.getProp( data, 'discussiontoolsedit', 'edit', 'captcha' ) ) { code = 'captcha'; widget.captchaInput = new mw.libs.confirmEdit.CaptchaInputWidget( - OO.getProp( data, 'visualeditoredit', 'edit', 'captcha' ) + OO.getProp( data, 'discussiontoolsedit', 'edit', 'captcha' ) ); // Save when pressing 'Enter' in captcha field as it is single line. widget.captchaInput.on( 'enter', function () { diff --git a/modules/dt.ui.ReplyWidgetPlain.js b/modules/dt.ui.ReplyWidgetPlain.js index 0a438505b..842b231da 100644 --- a/modules/dt.ui.ReplyWidgetPlain.js +++ b/modules/dt.ui.ReplyWidgetPlain.js @@ -5,7 +5,9 @@ * @extends mw.dt.ReplyWidget * @constructor * @param {Object} commentController - * @param {Object} parsoidData + * @param {string} commentId + * @param {string} pageName + * @param {number} oldId * @param {Object} [config] Configuration options */ function ReplyWidgetPlain() { @@ -45,10 +47,12 @@ ReplyWidgetPlain.prototype.focus = function () { }; ReplyWidgetPlain.prototype.clear = function () { + this.replyBodyWidget.setValue( '' ); + + this.storage.remove( this.storagePrefix + '/body' ); + // Parent method ReplyWidgetPlain.super.prototype.clear.apply( this, arguments ); - - this.replyBodyWidget.setValue( '' ); }; ReplyWidgetPlain.prototype.isEmpty = function () { @@ -92,8 +96,6 @@ ReplyWidgetPlain.prototype.teardown = function () { this.replyBodyWidget.disconnect( this ); this.replyBodyWidget.off( 'change' ); - this.storage.remove( this.storagePrefix + '/body' ); - // Parent method return ReplyWidgetPlain.super.prototype.teardown.call( this ); }; diff --git a/modules/dt.ui.ReplyWidgetVisual.js b/modules/dt.ui.ReplyWidgetVisual.js index af912a69d..d3f9da61f 100644 --- a/modules/dt.ui.ReplyWidgetVisual.js +++ b/modules/dt.ui.ReplyWidgetVisual.js @@ -15,7 +15,9 @@ require( './dt-ve/dt.ce.PingNode.js' ); * @extends mw.dt.ReplyWidget * @constructor * @param {Object} commentController - * @param {Object} parsoidData + * @param {string} commentId + * @param {string} pageName + * @param {number} oldId * @param {Object} [config] Configuration options */ function ReplyWidgetVisual() { @@ -51,10 +53,12 @@ ReplyWidgetVisual.prototype.getValue = function () { }; ReplyWidgetVisual.prototype.clear = function () { + this.replyBodyWidget.clear(); + + this.replyBodyWidget.target.clearDocState(); + // Parent method ReplyWidgetVisual.super.prototype.clear.apply( this, arguments ); - - this.replyBodyWidget.clear(); }; ReplyWidgetVisual.prototype.isEmpty = function () { @@ -109,8 +113,6 @@ ReplyWidgetVisual.prototype.setup = function ( initialValue ) { ReplyWidgetVisual.prototype.teardown = function () { this.replyBodyWidget.disconnect( this ); this.replyBodyWidget.off( 'change' ); - // TODO: Just teardown the whole target? - this.replyBodyWidget.target.clearDocState(); // Parent method return ReplyWidgetVisual.super.prototype.teardown.call( this ); diff --git a/modules/modifier.js b/modules/modifier.js index 1e077aa4e..9e2c6b326 100644 --- a/modules/modifier.js +++ b/modules/modifier.js @@ -343,6 +343,7 @@ function addSiblingListItem( previousItem ) { return listItem; } +// TODO: No longer used in the client function createWikitextNode( doc, wt ) { var span = doc.createElement( 'span' ); @@ -420,6 +421,8 @@ function appendSignature( container ) { /** * Add a reply to a specific comment * + * TODO: No longer used in the client + * * @param {CommentItem} comment Comment being replied to * @param {HTMLElement} container Container of comment DOM nodes */ @@ -444,6 +447,8 @@ function addReply( comment, container ) { /** * Create a container of comment DOM nodes from wikitext * + * TODO: No longer used in the client + * * @param {CommentItem} comment Comment being replied to * @param {string} wikitext Wikitext */ @@ -469,6 +474,8 @@ function addWikitextReply( comment, wikitext ) { /** * Create a container of comment DOM nodes from HTML * + * TODO: No longer used in the client + * * @param {CommentItem} comment Comment being replied to * @param {string} html HTML */ diff --git a/tests/phpunit/CommentModifierTest.php b/tests/phpunit/CommentModifierTest.php index 977cf8680..99f50d519 100644 --- a/tests/phpunit/CommentModifierTest.php +++ b/tests/phpunit/CommentModifierTest.php @@ -46,6 +46,7 @@ class CommentModifierTest extends CommentTestCase { // Uncomment this to write updated content to the "modified HTML" files: // self::overwriteHtmlFile( $expectedPath, $doc, $origPath ); + // saveHtml is not dirty-diff safe, but for testing it is probably faster than DOMCompat::getOuterHTML self::assertEquals( $expectedDoc->saveHtml(), $doc->saveHtml(), $name ); // removeAddedListItem is not implemented on the server @@ -89,6 +90,7 @@ class CommentModifierTest extends CommentTestCase { // Uncomment this to write updated content to the "reply HTML" files: // self::overwriteHtmlFile( $expectedPath, $doc, $origPath ); + // saveHtml is not dirty-diff safe, but for testing it is probably faster than DOMCompat::getOuterHTML self::assertEquals( $expectedDoc->saveHtml(), $doc->saveHtml(), $name ); }