From 236e3d1241c05dd4bfd0629702da797f18adc2e3 Mon Sep 17 00:00:00 2001 From: Ed Sanders Date: Sun, 15 May 2016 12:08:13 +0100 Subject: [PATCH] [BREAKING CHANGE] Evalute block/inline state when inserting a transclusion node Make some of the methods we currently use to render the node static so we can re-use them before inserting. We do the evaluation without inserting the node so as not to dirty the document and transcation history. In the unlikely case the request fails, just fallback to inline. This only handles insertions for now as type changes on edit will be very rare. This changes the signature of insertTransclusionNode, which is used in Cite and Citoid extensions. Bug: T51784 Change-Id: Ibc2fc66e6866084b0a4deeb082c8a1ca412febb2 --- .../ce/nodes/ve.ce.MWTransclusionNode.js | 46 ++++++++----- .../dm/models/ve.dm.MWTransclusionModel.js | 66 +++++++++++++++---- .../dm/nodes/ve.dm.MWTransclusionNode.js | 64 ++++++++++-------- .../ui/dialogs/ve.ui.MWTemplateDialog.js | 14 ++-- 4 files changed, 130 insertions(+), 60 deletions(-) diff --git a/modules/ve-mw/ce/nodes/ve.ce.MWTransclusionNode.js b/modules/ve-mw/ce/nodes/ve.ce.MWTransclusionNode.js index 29bdc04063..64eb73e07c 100644 --- a/modules/ve-mw/ce/nodes/ve.ce.MWTransclusionNode.js +++ b/modules/ve-mw/ce/nodes/ve.ce.MWTransclusionNode.js @@ -87,6 +87,33 @@ ve.ce.MWTransclusionNode.static.getDescription = function ( model ) { .join( ve.msg( 'comma-separator' ) ); }; +/** + * Filter rendering to remove auto-generated content and wrappers + * + * @static + * @param {HTMLElement[]} contentNodes Rendered nodes + * @return {HTMLElement[]} Filtered rendered nodes + */ +ve.ce.MWTransclusionNode.static.filterRendering = function ( contentNodes ) { + // Filter out auto-generated items, e.g. reference lists + contentNodes = contentNodes.filter( function ( node ) { + var dataMw = node.nodeType === Node.ELEMENT_NODE && + node.hasAttribute( 'data-mw' ) && + JSON.parse( node.getAttribute( 'data-mw' ) ); + if ( dataMw && dataMw.autoGenerated ) { + return false; + } + return true; + } ); + // HACK: if $content consists of a single paragraph, unwrap it. + // We have to do this because the parser wraps everything in

s, and inline templates + // will render strangely when wrapped in

s. + if ( contentNodes.length === 1 && contentNodes[ 0 ].nodeName.toLowerCase() === 'p' ) { + contentNodes = Array.prototype.slice.apply( contentNodes[ 0 ].childNodes ); + } + return contentNodes; +}; + /* Methods */ /** @@ -122,24 +149,7 @@ ve.ce.MWTransclusionNode.prototype.onParseSuccess = function ( deferred, respons // Work around https://github.com/jquery/jquery/issues/1997 contentNodes = $.parseHTML( response.visualeditor.content, this.getModelHtmlDocument() ) || []; - // Filter out auto-generated items, e.g. reference lists - contentNodes = contentNodes.filter( function ( node ) { - var dataMw = node.nodeType === Node.ELEMENT_NODE && - node.hasAttribute( 'data-mw' ) && - JSON.parse( node.getAttribute( 'data-mw' ) ); - if ( dataMw && dataMw.autoGenerated ) { - return false; - } - return true; - } ); - // HACK: if $content consists of a single paragraph, unwrap it. - // We have to do this because the parser wraps everything in

s, and inline templates - // will render strangely when wrapped in

s. - if ( contentNodes.length === 1 && contentNodes[ 0 ].nodeName.toLowerCase() === 'p' ) { - contentNodes = Array.prototype.slice.apply( contentNodes[ 0 ].childNodes ); - } - - deferred.resolve( contentNodes ); + deferred.resolve( this.constructor.static.filterRendering( contentNodes ) ); }; /** diff --git a/modules/ve-mw/dm/models/ve.dm.MWTransclusionModel.js b/modules/ve-mw/dm/models/ve.dm.MWTransclusionModel.js index e8a8aacf6e..5ac11e3b1f 100644 --- a/modules/ve-mw/dm/models/ve.dm.MWTransclusionModel.js +++ b/modules/ve-mw/dm/models/ve.dm.MWTransclusionModel.js @@ -50,19 +50,63 @@ /** * Insert transclusion at the end of a surface fragment. * - * @param {ve.dm.SurfaceFragment} surfaceFragment Surface fragment to insert at + * If forceType is not specified and this is used in async mode, users of this method + * should ensure the surface is not accessible while the type is being evaluated. + * + * @param {ve.dm.SurfaceFragment} surfaceFragment Surface fragment after which to insert. + * @param {boolean|undefined} [forceType] Force the type to 'inline' or 'block'. If not + * specified it will be evaluated asynchronously. + * @return {jQuery.Promise} Promise which resolves when the node has been inserted. If + * forceType was specified this will be instant. */ - ve.dm.MWTransclusionModel.prototype.insertTransclusionNode = function ( surfaceFragment ) { - surfaceFragment - .insertContent( [ - { - type: 'mwTransclusionInline', - attributes: { - mw: this.getPlainObject() + ve.dm.MWTransclusionModel.prototype.insertTransclusionNode = function ( surfaceFragment, forceType ) { + var model = this, + deferred = $.Deferred(), + nodeClass = ve.dm.MWTransclusionNode; + + function insertNode( isInline ) { + var type = isInline ? nodeClass.static.inlineType : nodeClass.static.blockType; + surfaceFragment + .insertContent( [ + { + type: type, + attributes: { + mw: model.getPlainObject() + } + }, + { type: '/' + type } + ] ); + deferred.resolve(); + } + + if ( forceType ) { + insertNode( forceType === 'inline' ); + } else { + new mw.Api().post( { + action: 'visualeditor', + paction: 'parsefragment', + page: mw.config.get( 'wgRelevantPageName' ), + wikitext: nodeClass.static.getWikitext( this.getPlainObject() ), + pst: 1 + } ) + .then( function ( response ) { + var contentNodes; + + if ( ve.getProp( response, 'visualeditor', 'result' ) === 'success' ) { + contentNodes = $.parseHTML( response.visualeditor.content, surfaceFragment.getDocument().getHtmlDocument() ) || []; + contentNodes = ve.ce.MWTransclusionNode.static.filterRendering( contentNodes ); + insertNode( + nodeClass.static.isHybridInline( contentNodes, ve.dm.converter ) + ); + } else { + // Request failed - just assume inline + insertNode( true ); } - }, - { type: '/mwTransclusionInline' } - ] ); + }, function () { + insertNode( true ); + } ); + } + return deferred.promise(); }; /** diff --git a/modules/ve-mw/dm/nodes/ve.dm.MWTransclusionNode.js b/modules/ve-mw/dm/nodes/ve.dm.MWTransclusionNode.js index bbe6b08555..fc2afef0cc 100644 --- a/modules/ve-mw/dm/nodes/ve.dm.MWTransclusionNode.js +++ b/modules/ve-mw/dm/nodes/ve.dm.MWTransclusionNode.js @@ -268,6 +268,41 @@ ve.dm.MWTransclusionNode.static.escapeParameter = function ( param ) { return output; }; +/** + * Get the wikitext for this transclusion. + * + * @static + * @param {Object} content MW data content + * @return {string} Wikitext like `{{foo|1=bar|baz=quux}}` + */ +ve.dm.MWTransclusionNode.static.getWikitext = function ( content ) { + var i, len, part, template, param, + wikitext = ''; + + // Normalize to multi template format + if ( content.params ) { + content = { parts: [ { template: content } ] }; + } + // Build wikitext from content + for ( i = 0, len = content.parts.length; i < len; i++ ) { + part = content.parts[ i ]; + if ( part.template ) { + // Template + template = part.template; + wikitext += '{{' + template.target.wt; + for ( param in template.params ) { + wikitext += '|' + param + '=' + + this.escapeParameter( template.params[ param ].wt ); + } + wikitext += '}}'; + } else { + // Plain wikitext + wikitext += part; + } + } + return wikitext; +}; + /* Methods */ /** @@ -350,38 +385,13 @@ ve.dm.MWTransclusionNode.prototype.getPartsList = function () { }; /** - * Get the wikitext for this transclusion. + * Wrapper for static method * * @method * @return {string} Wikitext like `{{foo|1=bar|baz=quux}}` */ ve.dm.MWTransclusionNode.prototype.getWikitext = function () { - var i, len, part, template, param, - content = this.getAttribute( 'mw' ), - wikitext = ''; - - // Normalize to multi template format - if ( content.params ) { - content = { parts: [ { template: content } ] }; - } - // Build wikitext from content - for ( i = 0, len = content.parts.length; i < len; i++ ) { - part = content.parts[ i ]; - if ( part.template ) { - // Template - template = part.template; - wikitext += '{{' + template.target.wt; - for ( param in template.params ) { - wikitext += '|' + param + '=' + - this.constructor.static.escapeParameter( template.params[ param ].wt ); - } - wikitext += '}}'; - } else { - // Plain wikitext - wikitext += part; - } - } - return wikitext; + return this.constructor.static.getWikitext( this.getAttribute( 'mw' ) ); }; /* Concrete subclasses */ diff --git a/modules/ve-mw/ui/dialogs/ve.ui.MWTemplateDialog.js b/modules/ve-mw/ui/dialogs/ve.ui.MWTemplateDialog.js index 4f80d96087..c5838e8a1b 100644 --- a/modules/ve-mw/ui/dialogs/ve.ui.MWTemplateDialog.js +++ b/modules/ve-mw/ui/dialogs/ve.ui.MWTemplateDialog.js @@ -414,19 +414,25 @@ ve.ui.MWTemplateDialog.prototype.getActionProcess = function ( action ) { return new OO.ui.Process( function () { var deferred = $.Deferred(); dialog.checkRequiredParameters().done( function () { - var surfaceModel = dialog.getFragment().getSurface(), + var modelPromise, + surfaceModel = dialog.getFragment().getSurface(), obj = dialog.transclusionModel.getPlainObject(); + dialog.pushPending(); + if ( dialog.selectedNode instanceof ve.dm.MWTransclusionNode ) { dialog.transclusionModel.updateTransclusionNode( surfaceModel, dialog.selectedNode ); + // TODO: updating the node could result in the inline/block state change + modelPromise = $.Deferred().resolve().promise(); } else if ( obj !== null ) { // Collapse returns a new fragment, so update dialog.fragment dialog.fragment = dialog.getFragment().collapseToEnd(); - dialog.transclusionModel.insertTransclusionNode( dialog.getFragment() ); + modelPromise = dialog.transclusionModel.insertTransclusionNode( dialog.getFragment() ); } - dialog.pushPending(); - dialog.close( { action: action } ).always( dialog.popPending.bind( dialog ) ); + return modelPromise.then( function () { + dialog.close( { action: action } ).always( dialog.popPending.bind( dialog ) ); + } ); } ).always( deferred.resolve ); return deferred;