From 48d5a67d41fc04a919ed208a6737972805b0d7ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Dziewo=C5=84ski?= Date: Fri, 25 Sep 2020 19:08:08 +0200 Subject: [PATCH] ReplyWidget: Pass initial config values to #setup, not to constructor The same ReplyWidget instance can be re-used when the user cancels replying and then starts replying again to the same comment. Previously, the initial values passed to the constructor (when switching modes) would still persist in this case, even if they were incorrect. Bug: T263061 Change-Id: I3dff007456bfd4f3149c718ae72fbf373a2e2913 --- modules/CommentController.js | 32 ++++++++++++++++++------------ modules/dt.ui.ReplyWidget.js | 16 ++++++++------- modules/dt.ui.ReplyWidgetPlain.js | 8 +++++--- modules/dt.ui.ReplyWidgetVisual.js | 11 +++++----- 4 files changed, 39 insertions(+), 28 deletions(-) diff --git a/modules/CommentController.js b/modules/CommentController.js index ac3df482c..6226a7676 100644 --- a/modules/CommentController.js +++ b/modules/CommentController.js @@ -197,7 +197,7 @@ CommentController.prototype.setup = function ( mode ) { } $( commentController.newListItem ).empty().append( replyWidget.$element ); - commentController.setupReplyWidget( replyWidget, null, true ); + commentController.setupReplyWidget( replyWidget, {}, true ); logger( { action: 'ready' } ); logger( { action: 'loaded' } ); @@ -222,10 +222,10 @@ CommentController.prototype.createReplyWidget = function ( comment, pageName, ol } ); }; -CommentController.prototype.setupReplyWidget = function ( replyWidget, initialValue, scrollIntoView ) { +CommentController.prototype.setupReplyWidget = function ( replyWidget, data, scrollIntoView ) { replyWidget.connect( this, { teardown: 'teardown' } ); - replyWidget.setup( initialValue ); + replyWidget.setup( data ); if ( scrollIntoView ) { replyWidget.scrollElementIntoView( { padding: scrollPadding } ); } @@ -323,6 +323,8 @@ CommentController.prototype.switchToWikitext = function () { var wikitextPromise, oldWidget = this.replyWidget, target = oldWidget.replyBodyWidget.target, + oldShowAdvanced = oldWidget.showAdvanced, + oldEditSummary = oldWidget.getEditSummary(), previewDeferred = $.Deferred(), commentController = this; @@ -332,10 +334,7 @@ CommentController.prototype.switchToWikitext = function () { oldWidget.comment, oldWidget.pageName, oldWidget.oldId, - { - showAdvanced: oldWidget.showAdvanced, - editSummary: oldWidget.getEditSummary() - }, + {}, false ); @@ -355,7 +354,11 @@ CommentController.prototype.switchToWikitext = function () { oldWidget.disconnect( commentController ); oldWidget.teardown(); - commentController.setupReplyWidget( replyWidget, wikitext ); + commentController.setupReplyWidget( replyWidget, { + value: wikitext, + showAdvanced: oldShowAdvanced, + editSummary: oldEditSummary + } ); } ); } ); }; @@ -363,6 +366,8 @@ CommentController.prototype.switchToWikitext = function () { CommentController.prototype.switchToVisual = function () { var parsePromise, oldWidget = this.replyWidget, + oldShowAdvanced = oldWidget.showAdvanced, + oldEditSummary = oldWidget.getEditSummary(), wikitext = oldWidget.getValue(), commentController = this; @@ -398,10 +403,7 @@ CommentController.prototype.switchToVisual = function () { oldWidget.comment, oldWidget.pageName, oldWidget.oldId, - { - showAdvanced: oldWidget.showAdvanced, - editSummary: oldWidget.getEditSummary() - }, + {}, true ); @@ -474,7 +476,11 @@ CommentController.prototype.switchToVisual = function () { oldWidget.disconnect( commentController ); oldWidget.teardown(); - commentController.setupReplyWidget( replyWidget, doc ); + commentController.setupReplyWidget( replyWidget, { + value: doc, + showAdvanced: oldShowAdvanced, + editSummary: oldEditSummary + } ); } ); }; diff --git a/modules/dt.ui.ReplyWidget.js b/modules/dt.ui.ReplyWidget.js index f19a7c9b7..589fa69ae 100644 --- a/modules/dt.ui.ReplyWidget.js +++ b/modules/dt.ui.ReplyWidget.js @@ -20,7 +20,6 @@ var controller = require( 'ext.discussionTools.init' ).controller, * @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 - * @param {Object} [config.editSummary] Initial edit summary */ function ReplyWidget( commentController, comment, pageName, oldId, config ) { var returnTo, contextNode, inputConfig, @@ -34,8 +33,6 @@ function ReplyWidget( commentController, comment, pageName, oldId, config ) { this.pending = false; this.commentController = commentController; this.comment = comment; - this.initialEditSummary = config.editSummary; - this.initialShowAdvanced = !!config.showAdvanced; this.summaryPrefixLength = null; this.pageName = pageName; this.oldId = oldId; @@ -365,20 +362,25 @@ ReplyWidget.prototype.onModeTabSelectChoose = function ( option ) { /** * Setup the widget * - * @param {Mixed} initialValue Initial value + * @param {Object} [data] Initial data + * @param {Mixed} [data.value] Initial value + * @param {string} [data.showAdvanced] Whether the "Advanced" menu is initially visible + * @param {string} [data.editSummary] Initial edit summary * @chainable * @return {ReplyWidget} */ -ReplyWidget.prototype.setup = function () { +ReplyWidget.prototype.setup = function ( data ) { var heading, summary, summaryPrefixLength; + data = data || {}; + this.bindBeforeUnloadHandler(); if ( this.modeTabSelect ) { this.modeTabSelect.selectItemByData( this.getMode() ); this.saveEditMode( this.getMode() ); } - summary = this.storage.get( this.storagePrefix + '/summary' ) || this.initialEditSummary; + summary = this.storage.get( this.storagePrefix + '/summary' ) || data.editSummary; if ( !summary ) { heading = this.comment.getHeading(); @@ -392,7 +394,7 @@ ReplyWidget.prototype.setup = function () { summary += mw.msg( 'discussiontools-defaultsummary-reply' ); } - this.toggleAdvanced( !!this.storage.get( this.storagePrefix + '/showAdvanced' ) || this.initialShowAdvanced ); + this.toggleAdvanced( !!this.storage.get( this.storagePrefix + '/showAdvanced' ) || !!data.showAdvanced ); this.editSummaryInput.setValue( summary ); // Store after change event handler is triggered diff --git a/modules/dt.ui.ReplyWidgetPlain.js b/modules/dt.ui.ReplyWidgetPlain.js index 1c8c88c8d..0fb24e25c 100644 --- a/modules/dt.ui.ReplyWidgetPlain.js +++ b/modules/dt.ui.ReplyWidgetPlain.js @@ -78,16 +78,18 @@ ReplyWidgetPlain.prototype.onInputChange = function () { this.storage.set( this.storagePrefix + '/body', wikitext ); }; -ReplyWidgetPlain.prototype.setup = function ( initialValue ) { +ReplyWidgetPlain.prototype.setup = function ( data ) { var autosaveValue = this.storage.get( this.storagePrefix + '/body' ); + data = data || {}; + // Parent method - ReplyWidgetPlain.super.prototype.setup.call( this ); + ReplyWidgetPlain.super.prototype.setup.apply( this, arguments ); // Events this.replyBodyWidget.connect( this, { change: this.onInputChangeThrottled } ); - this.replyBodyWidget.setValue( initialValue || autosaveValue ); + this.replyBodyWidget.setValue( data.value || autosaveValue ); // needs to bind after the initial setValue: this.replyBodyWidget.once( 'change', this.onFirstTransaction.bind( this ) ); diff --git a/modules/dt.ui.ReplyWidgetVisual.js b/modules/dt.ui.ReplyWidgetVisual.js index 605830040..ea0c7db5b 100644 --- a/modules/dt.ui.ReplyWidgetVisual.js +++ b/modules/dt.ui.ReplyWidgetVisual.js @@ -28,7 +28,6 @@ require( './dt-ve/dt.ce.PingNode.js' ); function ReplyWidgetVisual() { // TODO: Support 2017 wikitext editor this.defaultMode = 'visual'; - this.initialValue = null; // Parent constructor ReplyWidgetVisual.super.apply( this, arguments ); @@ -77,20 +76,22 @@ ReplyWidgetVisual.prototype.getMode = function () { this.defaultMode; }; -ReplyWidgetVisual.prototype.setup = function ( initialValue ) { +ReplyWidgetVisual.prototype.setup = function ( data ) { var htmlOrDoc, widget = this, target = this.replyBodyWidget.target; + data = data || {}; + if ( this.storage.get( this.storagePrefix + '/saveable' ) ) { htmlOrDoc = this.storage.get( this.storagePrefix + '/ve-dochtml' ); target.recovered = true; } else { - htmlOrDoc = initialValue || '

'; + htmlOrDoc = data.value || '

'; } target.originalHtml = htmlOrDoc instanceof HTMLDocument ? ve.properInnerHtml( htmlOrDoc.body ) : htmlOrDoc; - target.fromEditedState = !!initialValue; + target.fromEditedState = !!data.value; this.replyBodyWidget.setDocument( htmlOrDoc ); @@ -104,7 +105,7 @@ ReplyWidgetVisual.prototype.setup = function ( initialValue ) { } ); // Parent method - ReplyWidgetVisual.super.prototype.setup.call( this ); + ReplyWidgetVisual.super.prototype.setup.apply( this, arguments ); // Events this.replyBodyWidget.connect( this, {