diff --git a/extension.json b/extension.json index 131a7228f..79f5043c4 100644 --- a/extension.json +++ b/extension.json @@ -78,7 +78,7 @@ "CommentItem.js", "HeadingItem.js", "CommentDetails.js", - "createMemoryStorage.js", + "MemoryStorage.js", "lib/moment-timezone/moment-timezone-with-data-1970-2030.js", { "name": "parser/data.json", diff --git a/modules/.eslintrc.json b/modules/.eslintrc.json index de76e4868..fb8a887aa 100644 --- a/modules/.eslintrc.json +++ b/modules/.eslintrc.json @@ -1,7 +1,7 @@ { "root": true, "extends": [ - "wikimedia/client-es5", + "wikimedia/client", "wikimedia/jquery", "wikimedia/mediawiki" ], @@ -11,6 +11,7 @@ }, "rules": { "no-implicit-globals": "off", + "no-var": "off", "max-len": "off" }, "settings": { diff --git a/modules/CommentController.js b/modules/CommentController.js index 3f928f913..e0bd4d31f 100644 --- a/modules/CommentController.js +++ b/modules/CommentController.js @@ -16,8 +16,9 @@ var * @param {jQuery} $pageContainer Page container * @param {ThreadItem} threadItem Thread item to attach new comment to * @param {ThreadItemSet} threadItemSet + * @param {MemoryStorage} storage Storage object for autosave */ -function CommentController( $pageContainer, threadItem, threadItemSet ) { +function CommentController( $pageContainer, threadItem, threadItemSet, storage ) { // Mixin constructors OO.EventEmitter.call( this ); @@ -25,6 +26,7 @@ function CommentController( $pageContainer, threadItem, threadItemSet ) { this.$pageContainer = $pageContainer; this.threadItem = threadItem; this.threadItemSet = threadItemSet; + this.storage = storage; this.newListItem = null; this.replyWidgetPromise = null; this.newComments = []; @@ -334,7 +336,7 @@ CommentController.prototype.setupReplyWidget = function ( replyWidget, data, sup CommentController.prototype.storeEditSummary = function () { if ( this.replyWidget ) { - this.replyWidget.storage.set( this.replyWidget.storagePrefix + '/summary', this.replyWidget.getEditSummary() ); + this.replyWidget.storage.set( 'summary', this.replyWidget.getEditSummary() ); } }; diff --git a/modules/MemoryStorage.js b/modules/MemoryStorage.js new file mode 100644 index 000000000..ae3bfb475 --- /dev/null +++ b/modules/MemoryStorage.js @@ -0,0 +1,75 @@ +/** + * MemoryStorage is a wrapper around mw.SafeStorage objects. + * + * It provides two mechanisms to improve their behavior when the underlying storage mechanism fails + * (e.g. quota exceeded): + * + * - Duplicating their contents in memory, so that the storage can be relied on before the page has + * been reloaded. + * - Storing all keys in a single underlying key, to make the stores are atomic: either all values + * are stored or none. + * + * This has two additional consequences which are convenient for our use case: + * + * - All keys share the same expiry time, removing the need to specify it repeatedly. + * - When multiple processes try to write the same keys, all keys are overwritten, so that we don't + * have to worry about inconsistent data. + * + * @example + * var sessionStorage = new MemoryStorage( mw.storage.session, 'myprefix' ); + * var localStorage = new MemoryStorage( mw.storage, 'myprefix' ); + */ +class MemoryStorage { + /** + * @param {mw.SafeStorage} backend + * @param {string} key Key name to store under + * @param {number} [expiry] Number of seconds after which this item can be deleted + */ + constructor( backend, key, expiry ) { + this.backend = backend; + this.key = key; + this.expiry = expiry; + + this.data = this.backend.getObject( this.key ) || {}; + } + + get( key ) { + if ( Object.prototype.hasOwnProperty.call( this.data, key ) ) { + return this.data[ key ]; + } + return null; + } + + set( key, value ) { + this.data[ key ] = value; + this.backend.setObject( this.key, this.data, this.expiry ); + return true; + } + + remove( key ) { + delete this.data[ key ]; + if ( Object.keys( this.data ).length === 0 ) { + this.backend.remove( this.key ); + } else { + this.backend.setObject( this.key, this.data, this.expiry ); + } + return true; + } + + clear() { + this.data = {}; + this.backend.remove( this.key ); + } + + // For compatibility with mw.SafeStorage API + getObject( key ) { + return this.get( key ); + } + + // For compatibility with mw.SafeStorage API + setObject( key, value ) { + return this.set( key, value ); + } +} + +module.exports = MemoryStorage; diff --git a/modules/NewTopicController.js b/modules/NewTopicController.js index 2afccdf2f..be4840337 100644 --- a/modules/NewTopicController.js +++ b/modules/NewTopicController.js @@ -8,8 +8,9 @@ var * @param {jQuery} $pageContainer Page container * @param {HeadingItem} threadItem * @param {ThreadItemSet} threadItemSet + * @param {MemoryStorage} storage Storage object for autosave */ -function NewTopicController( $pageContainer, threadItem, threadItemSet ) { +function NewTopicController( $pageContainer, threadItem, threadItemSet, storage ) { this.container = new OO.ui.PanelLayout( { classes: [ 'ext-discussiontools-ui-newTopic' ], expanded: false, @@ -39,7 +40,7 @@ function NewTopicController( $pageContainer, threadItem, threadItemSet ) { threadItem.range.endContainer = this.sectionTitleField.$element[ 0 ]; threadItem.range.endOffset = this.sectionTitleField.$element[ 0 ].childNodes.length; - NewTopicController.super.call( this, $pageContainer, threadItem, threadItemSet ); + NewTopicController.super.call( this, $pageContainer, threadItem, threadItemSet, storage ); } OO.inheritClass( NewTopicController, CommentController ); @@ -120,7 +121,7 @@ NewTopicController.prototype.setupReplyWidget = function ( replyWidget, data ) { } mw.hook( 'wikipage.content' ).fire( this.$notices ); - var title = this.replyWidget.storage.get( this.replyWidget.storagePrefix + '/title' ); + var title = this.replyWidget.storage.get( 'title' ); if ( title && !this.sectionTitle.getValue() ) { // Don't overwrite if the user has already typed something in while the widget was loading. // TODO This should happen immediately rather than waiting for the reply widget to load, @@ -128,12 +129,12 @@ NewTopicController.prototype.setupReplyWidget = function ( replyWidget, data ) { this.sectionTitle.setValue( title ); this.prevTitleText = title; - if ( this.replyWidget.storage.get( this.replyWidget.storagePrefix + '/summary' ) === null ) { + if ( this.replyWidget.storage.get( 'summary' ) === null ) { var generatedSummary = this.generateSummary( title ); this.replyWidget.editSummaryInput.setValue( generatedSummary ); } } - this.replyWidget.storage.set( this.replyWidget.storagePrefix + '/title', this.sectionTitle.getValue() ); + this.replyWidget.storage.set( 'title', this.sectionTitle.getValue() ); if ( this.replyWidget.modeTabSelect ) { // Start with the mode-select widget not-tabbable so focus will go from the title to the body @@ -173,7 +174,7 @@ NewTopicController.prototype.clear = function () { NewTopicController.prototype.clearStorage = function () { // This is going to get called as part of the teardown chain from replywidget if ( this.replyWidget ) { - this.replyWidget.storage.remove( this.replyWidget.storagePrefix + '/title' ); + this.replyWidget.storage.remove( 'title' ); } }; @@ -183,7 +184,7 @@ NewTopicController.prototype.storeEditSummary = function () { var generatedSummary = this.generateSummary( this.sectionTitle.getValue() ); if ( currentSummary === generatedSummary ) { // Do not store generated summaries (T315730) - this.replyWidget.storage.remove( this.replyWidget.storagePrefix + '/summary' ); + this.replyWidget.storage.remove( 'summary' ); return; } } @@ -289,7 +290,7 @@ NewTopicController.prototype.onSectionTitleChange = function () { var prevTitleText = this.prevTitleText; if ( prevTitleText !== titleText ) { - this.replyWidget.storage.set( this.replyWidget.storagePrefix + '/title', titleText ); + this.replyWidget.storage.set( 'title', titleText ); var generatedSummary = this.generateSummary( titleText ); var generatedPrevSummary = this.generateSummary( prevTitleText ); diff --git a/modules/controller.js b/modules/controller.js index 0e91fb9ca..3c4c059dd 100644 --- a/modules/controller.js +++ b/modules/controller.js @@ -5,8 +5,8 @@ var pageThreads, lastControllerScrollOffset, featuresEnabled = mw.config.get( 'wgDiscussionToolsFeaturesEnabled' ) || {}, - createMemoryStorage = require( './createMemoryStorage.js' ), - storage = createMemoryStorage( mw.storage ), + MemoryStorage = require( './MemoryStorage.js' ), + STORAGE_EXPIRY = 60 * 60 * 24 * 30, Parser = require( './Parser.js' ), ThreadItemSet = require( './ThreadItemSet.js' ), CommentDetails = require( './CommentDetails.js' ), @@ -355,18 +355,22 @@ function init( $container, state ) { * @param {string} [mode] Optionally force a mode, 'visual' or 'source' * @param {boolean} [hideErrors] Suppress errors, e.g. when restoring auto-save * @param {boolean} [suppressNotifications] Don't notify the user if recovering auto-save + * @param {MemoryStorage} [storage] Storage object for autosave */ - function setupController( comment, $link, mode, hideErrors, suppressNotifications ) { + function setupController( comment, $link, mode, hideErrors, suppressNotifications, storage ) { var commentController, $addSectionLink; + if ( !storage ) { + storage = new MemoryStorage( mw.storage, 'mw-ext-DiscussionTools-reply/' + comment.id, STORAGE_EXPIRY ); + } if ( comment.id === utils.NEW_TOPIC_COMMENT_ID ) { // eslint-disable-next-line no-jquery/no-global-selector $addSectionLink = $( '#ca-addsection' ).find( 'a' ); // When opening new topic tool using any link, always activate the link in page tabs too $link = $link.add( $addSectionLink ); - commentController = new NewTopicController( $pageContainer, comment, pageThreads ); + commentController = new NewTopicController( $pageContainer, comment, pageThreads, storage ); } else { - commentController = new CommentController( $pageContainer, comment, pageThreads ); + commentController = new CommentController( $pageContainer, comment, pageThreads, storage ); } activeCommentId = comment.id; @@ -473,13 +477,14 @@ function init( $container, state ) { return; } - var mode, $link; + var mode, $link, storage; for ( var i = 0; i < pageThreads.threadItems.length; i++ ) { var comment = pageThreads.threadItems[ i ]; - if ( storage.get( 'reply/' + comment.id + '/saveable' ) ) { - mode = storage.get( 'reply/' + comment.id + '/mode' ); + storage = new MemoryStorage( mw.storage, 'mw-ext-DiscussionTools-reply/' + comment.id, STORAGE_EXPIRY ); + if ( storage.get( 'saveable' ) ) { + mode = storage.get( 'mode' ); $link = $( commentNodes[ i ] ); - setupController( comment, $link, mode, true, !state.firstLoad ); + setupController( comment, $link, mode, true, !state.firstLoad, storage ); if ( OO.ui.isMobile() ) { var urlFragment = mw.util.escapeIdForLink( comment.id ); // Force the section to expand on mobile (T338920) @@ -488,12 +493,10 @@ function init( $container, state ) { break; } } - if ( - storage.get( 'reply/' + utils.NEW_TOPIC_COMMENT_ID + '/saveable' ) || - storage.get( 'reply/' + utils.NEW_TOPIC_COMMENT_ID + '/title' ) - ) { - mode = storage.get( 'reply/' + utils.NEW_TOPIC_COMMENT_ID + '/mode' ); - setupController( newTopicComment(), $( [] ), mode, true, !state.firstLoad ); + storage = new MemoryStorage( mw.storage, 'mw-ext-DiscussionTools-reply/' + utils.NEW_TOPIC_COMMENT_ID, STORAGE_EXPIRY ); + if ( storage.get( 'saveable' ) || storage.get( 'title' ) ) { + mode = storage.get( 'mode' ); + setupController( newTopicComment(), $( [] ), mode, true, !state.firstLoad, storage ); } else if ( mw.config.get( 'wgDiscussionToolsStartNewTopicTool' ) ) { var data = linksController.parseNewTopicLink( location.href ); setupController( newTopicComment( data ), $( [] ) ); @@ -765,7 +768,6 @@ module.exports = { checkThreadItemOnPage: checkThreadItemOnPage, getCheckboxesPromise: getCheckboxesPromise, getApi: getApi, - storage: storage, getReplyWidgetModules: getReplyWidgetModules, defaultVisual: defaultVisual, enable2017Wikitext: enable2017Wikitext diff --git a/modules/createMemoryStorage.js b/modules/createMemoryStorage.js deleted file mode 100644 index bb5874dee..000000000 --- a/modules/createMemoryStorage.js +++ /dev/null @@ -1,83 +0,0 @@ -/** - * createMemoryStorage creates a wrapper around mw.SafeStorage objects, duplicating - * their contents in memory, so that even if the underlying storage mechanism - * fails (e.g. quota exceeded), the storage can be relied on before the - * page has been reloaded. - * - * @example - * var sessionStorage = createMemoryStorage( mw.storage.session ); - * var localStorage = createMemoryStorage( mw.storage ); - * - * @param {mw.SafeStorage} storage - * @return {MemoryStorage} - */ -var createMemoryStorage = function ( storage ) { - - /** - * @class - * @extends mw.SafeStorage - * - * @constructor - * @param {Storage|undefined} store The Storage instance to wrap around - */ - function MemoryStorage( store ) { - this.data = {}; - - // Attempt to populate memory cache with existing data. - // Ignore any errors accessing native Storage object, as in mw.SafeStorage. - try { - for ( var i = 0, l = store.length; i < l; i++ ) { - var key = store.key( i ); - this.data[ key ] = store.getItem( key ); - } - } catch ( e ) {} - - // Parent constructor - MemoryStorage.super.apply( this, arguments ); - } - - /* Inheritance */ - - var ParentStorage = storage.constructor; - OO.inheritClass( MemoryStorage, ParentStorage ); - - /* Methods */ - - /** - * @inheritdoc - */ - MemoryStorage.prototype.get = function ( key ) { - if ( Object.prototype.hasOwnProperty.call( this.data, key ) ) { - return this.data[ key ]; - } else { - // Parent method - return MemoryStorage.super.prototype.get.apply( this, arguments ); - } - }; - - /** - * @inheritdoc - */ - MemoryStorage.prototype.set = function ( key, value ) { - // Parent method - MemoryStorage.super.prototype.set.apply( this, arguments ); - - this.data[ key ] = value; - return true; - }; - - /** - * @inheritdoc - */ - MemoryStorage.prototype.remove = function ( key ) { - // Parent method - MemoryStorage.super.prototype.remove.apply( this, arguments ); - - delete this.data[ key ]; - return true; - }; - - return new MemoryStorage( storage.store ); -}; - -module.exports = createMemoryStorage; diff --git a/modules/dt.ui.ReplyWidget.js b/modules/dt.ui.ReplyWidget.js index 6419ad2bf..3cecbf959 100644 --- a/modules/dt.ui.ReplyWidget.js +++ b/modules/dt.ui.ReplyWidget.js @@ -43,9 +43,7 @@ function ReplyWidget( commentController, commentDetails, config ) { this.pageExists = mw.config.get( 'wgRelevantArticleId', 0 ) !== 0; var contextNode = utils.closestElement( threadItem.range.endContainer, [ 'dl', 'ul', 'ol' ] ); this.context = contextNode ? contextNode.tagName.toLowerCase() : 'dl'; - // TODO: Should storagePrefix include pageName? - this.storagePrefix = 'reply/' + threadItem.id; - this.storage = controller.storage; + this.storage = commentController.storage; // eslint-disable-next-line no-jquery/no-global-selector this.contentDir = $( '#mw-content-text' ).css( 'direction' ); this.hideNewCommentsWarning = false; @@ -390,7 +388,7 @@ ReplyWidget.prototype.clear = function ( preserveStorage ) { if ( !preserveStorage ) { this.clearStorage(); } else { - this.storage.set( this.storagePrefix + '/saveable', '1' ); + this.storage.set( 'saveable', '1' ); } this.emit( 'clear' ); @@ -400,12 +398,7 @@ ReplyWidget.prototype.clear = function ( preserveStorage ) { * Remove any storage that the widget is using */ ReplyWidget.prototype.clearStorage = function () { - this.storage.remove( this.storagePrefix + '/mode' ); - this.storage.remove( this.storagePrefix + '/saveable' ); - this.storage.remove( this.storagePrefix + '/summary' ); - this.storage.remove( this.storagePrefix + '/showAdvanced' ); - this.storage.remove( this.storagePrefix + '/formToken' ); - + this.storage.clear(); this.emit( 'clearStorage' ); }; @@ -498,7 +491,7 @@ ReplyWidget.prototype.toggleAdvanced = function ( showAdvanced ) { this.advanced.toggle( !!this.showAdvanced ); this.advancedToggle.setIndicator( this.showAdvanced ? 'up' : 'down' ); - this.storage.set( this.storagePrefix + '/showAdvanced', this.showAdvanced ? '1' : '' ); + this.storage.set( 'showAdvanced', this.showAdvanced ? '1' : '' ); }; ReplyWidget.prototype.onEditSummaryChange = function () { @@ -585,7 +578,7 @@ ReplyWidget.prototype.setup = function ( data ) { } this.saveEditMode( this.getMode() ); - var summary = this.storage.get( this.storagePrefix + '/summary' ) || data.editSummary; + var summary = this.storage.get( 'summary' ) || data.editSummary; if ( !summary ) { if ( this.isNewTopic ) { @@ -600,7 +593,7 @@ ReplyWidget.prototype.setup = function ( data ) { } this.toggleAdvanced( - !!this.storage.get( this.storagePrefix + '/showAdvanced' ) || + !!this.storage.get( 'showAdvanced' ) || !!+mw.user.options.get( 'discussiontools-showadvanced' ) || !!data.showAdvanced ); @@ -631,7 +624,7 @@ ReplyWidget.prototype.afterSetup = function () { // Init preview and button state this.onInputChange(); // Autosave - this.storage.set( this.storagePrefix + '/mode', this.getMode() ); + this.storage.set( 'mode', this.getMode() ); }; /** @@ -640,12 +633,12 @@ ReplyWidget.prototype.afterSetup = function () { * @return {string} Form token */ ReplyWidget.prototype.getFormToken = function () { - var formToken = this.storage.get( this.storagePrefix + '/formToken' ); + var formToken = this.storage.get( 'formToken' ); if ( !formToken ) { // See ApiBase::PARAM_MAX_CHARS in ApiDiscussionToolsEdit.php var maxLength = 16; formToken = Math.random().toString( 36 ).slice( 2, maxLength + 2 ); - this.storage.set( this.storagePrefix + '/formToken', formToken ); + this.storage.set( 'formToken', formToken ); } return formToken; }; @@ -771,7 +764,7 @@ ReplyWidget.prototype.onInputChange = function () { } this.updateButtons(); - this.storage.set( this.storagePrefix + '/saveable', this.isEmpty() ? '' : '1' ); + this.storage.set( 'saveable', this.isEmpty() ? '' : '1' ); this.preparePreview(); }; diff --git a/modules/dt.ui.ReplyWidgetPlain.js b/modules/dt.ui.ReplyWidgetPlain.js index 1ac7cd24d..f1a7235d7 100644 --- a/modules/dt.ui.ReplyWidgetPlain.js +++ b/modules/dt.ui.ReplyWidgetPlain.js @@ -92,7 +92,7 @@ ReplyWidgetPlain.prototype.clear = function ( preserveStorage ) { this.replyBodyWidget.setValue( '' ); if ( !preserveStorage ) { - this.storage.remove( this.storagePrefix + '/body' ); + this.storage.remove( 'body' ); } // Parent method @@ -126,14 +126,14 @@ ReplyWidgetPlain.prototype.onInputChange = function () { ReplyWidgetPlain.super.prototype.onInputChange.apply( this, arguments ); var wikitext = this.getValue(); - this.storage.set( this.storagePrefix + '/body', wikitext ); + this.storage.set( 'body', wikitext ); }; /** * @inheritdoc */ ReplyWidgetPlain.prototype.setup = function ( data ) { - var autosaveValue = this.storage.get( this.storagePrefix + '/body' ); + var autosaveValue = this.storage.get( 'body' ); data = data || {}; diff --git a/modules/dt.ui.ReplyWidgetVisual.js b/modules/dt.ui.ReplyWidgetVisual.js index 41f777538..7ff3737a1 100644 --- a/modules/dt.ui.ReplyWidgetVisual.js +++ b/modules/dt.ui.ReplyWidgetVisual.js @@ -96,8 +96,8 @@ ReplyWidgetVisual.prototype.setup = function ( data, suppressNotifications ) { data = data || {}; var htmlOrDoc; - if ( this.storage.get( this.storagePrefix + '/saveable' ) ) { - htmlOrDoc = this.storage.get( this.storagePrefix + '/ve-dochtml' ); + if ( this.storage.get( 'saveable' ) ) { + htmlOrDoc = this.storage.get( 've-dochtml' ); target.recovered = true; } else { htmlOrDoc = data.value; @@ -115,16 +115,9 @@ ReplyWidgetVisual.prototype.setup = function ( data, suppressNotifications ) { focus: [ 'emit', 'bodyFocus' ] } ); - var listStorage = ve.init.platform.createConflictableStorage( widget.storage ); - // widget.storage is a MemoryStorage object. Copy over the .data cache so - // that listStorage reads from/writes to the same in-memory cache. - listStorage.data = widget.storage.data; - target.initAutosave( { suppressNotifications: suppressNotifications, - docId: widget.storagePrefix, - storage: listStorage, - storageExpiry: 60 * 60 * 24 * 30 + storage: widget.storage } ); widget.afterSetup();