jenkins-bot has submitted this change and it was merged. Change subject: Handle meta-only transclusions as meta items ......................................................................
Handle meta-only transclusions as meta items To achieve this we need to evaluate the DOM contents of transclusion nodes to see if it consists solely of meta items and whitespace. To check for meta items we do a model registry match, but with an additional parameter to exclude mwTransclusion types as a possible result (as the first item may be a meta tag, but with a mw:Transclusion typeof attribute). Bug: 51322 Change-Id: I89a220350fb7e10e15f3682d21438539196a5846 --- M VisualEditor.php A modules/ve-mw/dm/metaitems/ve.dm.MWTransclusionMetaItem.js M modules/ve-mw/dm/nodes/ve.dm.MWTransclusionNode.js M modules/ve-mw/test/dm/ve.dm.mwExample.js M modules/ve-mw/test/index.php M modules/ve/dm/ve.dm.Converter.js M modules/ve/dm/ve.dm.ModelRegistry.js M modules/ve/test/ve.test.utils.js 8 files changed, 128 insertions(+), 3 deletions(-) Approvals: Catrope: Looks good to me, approved jenkins-bot: Verified diff --git a/VisualEditor.php b/VisualEditor.php index 1ea12f7..4b1d88b 100644 --- a/VisualEditor.php +++ b/VisualEditor.php @@ -365,6 +365,7 @@ 've-mw/dm/metaitems/ve.dm.MWCategoryMetaItem.js', 've-mw/dm/metaitems/ve.dm.MWDefaultSortMetaItem.js', 've-mw/dm/metaitems/ve.dm.MWLanguageMetaItem.js', + 've-mw/dm/metaitems/ve.dm.MWTransclusionMetaItem.js', 've-mw/dm/models/ve.dm.MWTransclusionModel.js', 've-mw/dm/models/ve.dm.MWTransclusionPartModel.js', diff --git a/modules/ve-mw/dm/metaitems/ve.dm.MWTransclusionMetaItem.js b/modules/ve-mw/dm/metaitems/ve.dm.MWTransclusionMetaItem.js new file mode 100644 index 0000000..f55b5fa --- /dev/null +++ b/modules/ve-mw/dm/metaitems/ve.dm.MWTransclusionMetaItem.js @@ -0,0 +1,37 @@ +/*! + * VisualEditor DataModel MWTransclusionMetaItem class. + * + * @copyright 2011-2013 VisualEditor Team and others; see AUTHORS.txt + * @license The MIT License (MIT); see LICENSE.txt + */ + +/** + * DataModel MW-specific meta item. + * + * @class + * @abstract + * @extends ve.dm.AlienMetaItem + * @constructor + * @param {Object} element Reference to element in meta-linmod + */ +ve.dm.MWTransclusionMetaItem = function VeDmMWTransclusionMetaItem( element ) { + // Parent constructor + ve.dm.AlienMetaItem.call( this, element ); +}; + +/* Inheritance */ + +ve.inheritClass( ve.dm.MWTransclusionMetaItem, ve.dm.AlienMetaItem ); + +/* Static Properties */ + +ve.dm.MWTransclusionMetaItem.static.name = 'mwTransclusionMeta'; + +ve.dm.MWTransclusionMetaItem.static.matchTagNames = []; + +// mwTransclusionMetaItems are generated by ve.dm.MWTransclusionNode#toDataElement when +// all of the transclusions contents are considered to be metadata or whitespace + +/* Registration */ + +ve.dm.modelRegistry.register( ve.dm.MWTransclusionMetaItem ); diff --git a/modules/ve-mw/dm/nodes/ve.dm.MWTransclusionNode.js b/modules/ve-mw/dm/nodes/ve.dm.MWTransclusionNode.js index 620ee76..6ffbf8f 100644 --- a/modules/ve-mw/dm/nodes/ve.dm.MWTransclusionNode.js +++ b/modules/ve-mw/dm/nodes/ve.dm.MWTransclusionNode.js @@ -61,6 +61,10 @@ }; ve.dm.MWTransclusionNode.static.toDataElement = function ( domElements, converter ) { + if ( converter.isDomAllMetaOrWhitespace( domElements, ['mwTransclusion', 'mwTransclusionInline', 'mwTransclusionBlock'] ) ) { + return ve.dm.MWTransclusionMetaItem.static.toDataElement( domElements, converter ); + } + var dataElement, index, mwDataJSON = domElements[0].getAttribute( 'data-mw' ), mwData = mwDataJSON ? JSON.parse( mwDataJSON ) : {}, diff --git a/modules/ve-mw/test/dm/ve.dm.mwExample.js b/modules/ve-mw/test/dm/ve.dm.mwExample.js index ec10cbb..073c2d0 100644 --- a/modules/ve-mw/test/dm/ve.dm.mwExample.js +++ b/modules/ve-mw/test/dm/ve.dm.mwExample.js @@ -27,7 +27,12 @@ 'inlineClose': '</span>', 'mixed': '<link about="#mwt1" rel="mw:WikiLink/Category" typeof="mw:Transclusion" data-mw="{"id":"mwt1","target":{"wt":"Inline"},"params":{"1":{"wt":"5,678"}}}"><span about="#mwt1">Foo</span>', 'pairOne': '<p about="#mwt1" typeof="mw:Transclusion" data-mw="{"params":{"1":{"wt":"foo"}}}" data-parsoid="1">foo</p>', - 'pairTwo': '<p about="#mwt2" typeof="mw:Transclusion" data-mw="{"params":{"1":{"wt":"foo"}}}" data-parsoid="2">foo</p>' + 'pairTwo': '<p about="#mwt2" typeof="mw:Transclusion" data-mw="{"params":{"1":{"wt":"foo"}}}" data-parsoid="2">foo</p>', + 'meta': + '<link rel="mw:WikiLink/Category" href="./Category:Page" about="#mwt1" typeof="mw:Transclusion" ' + + 'data-mw="{"target":{"wt":"Template:Echo","href":"./Template:Echo"},"params":{"1":{"wt":"[[Category:Page]]\\n[[Category:Book]]"}},"i":0}">' + + '<span about="#mwt1" data-parsoid="{}">\n</span>' + + '<link rel="mw:WikiLink/Category" href="./Category:Book" about="#mwt1">' }; ve.dm.mwExample.MWTransclusion.blockData = { 'type': 'mwTransclusionBlock', @@ -800,6 +805,23 @@ } ] }, + 'mw:Transclusion containing only meta data': { + 'html': + '<body>' + ve.dm.mwExample.MWTransclusion.meta + '</body>', + 'data': [ + { + 'type': 'mwTransclusionMeta', + 'attributes': { + 'domElements': $( ve.dm.mwExample.MWTransclusion.meta ).toArray() + } + }, + { 'type': '/mwTransclusionMeta' }, + { 'type': 'paragraph', 'internal': { 'generated': 'empty' } }, + { 'type': '/paragraph' }, + { 'type': 'internalList' }, + { 'type': '/internalList' } + ] + }, 'mw:Reference': { 'html': '<body>' + diff --git a/modules/ve-mw/test/index.php b/modules/ve-mw/test/index.php index a333b29..12485ff 100644 --- a/modules/ve-mw/test/index.php +++ b/modules/ve-mw/test/index.php @@ -135,6 +135,7 @@ <script src="../../ve-mw/dm/metaitems/ve.dm.MWCategoryMetaItem.js"></script> <script src="../../ve-mw/dm/metaitems/ve.dm.MWDefaultSortMetaItem.js"></script> <script src="../../ve-mw/dm/metaitems/ve.dm.MWLanguageMetaItem.js"></script> + <script src="../../ve-mw/dm/metaitems/ve.dm.MWTransclusionMetaItem.js"></script> <script src="../../ve-mw/dm/models/ve.dm.MWTransclusionModel.js"></script> <script src="../../ve-mw/dm/models/ve.dm.MWTransclusionPartModel.js"></script> <script src="../../ve-mw/dm/models/ve.dm.MWTransclusionContentModel.js"></script> diff --git a/modules/ve/dm/ve.dm.Converter.js b/modules/ve/dm/ve.dm.Converter.js index ad3e522..5e2a520 100644 --- a/modules/ve/dm/ve.dm.Converter.js +++ b/modules/ve/dm/ve.dm.Converter.js @@ -661,6 +661,8 @@ processNextWhitespace( childDataElements[0] ); prevElement = childDataElements[0]; } + // In case we consumed multiple childDomElements, adjust i accordingly + i += childDomElements.length - 1; break; } @@ -937,6 +939,54 @@ }; /** + * Check if all the domElements provided are metadata or whitespace. + * + * A list of model names to exclude when matching can optionally be passed. + * + * @param {HTMLElement[]} domElements DOM elements to check + * @param {string[]} [excludeTypes] Model names to exclude when matching DOM elements + * @returns {boolean} All the elements are metadata or whitespace + */ +ve.dm.Converter.prototype.isDomAllMetaOrWhitespace = function ( domElements, excludeTypes ) { + var i, childDomElement, modelName, modelClass; + + for ( i = 0; i < domElements.length; i++ ) { + childDomElement = domElements[i]; + switch ( childDomElement.nodeType ) { + case Node.ELEMENT_NODE: + modelName = this.modelRegistry.matchElement( childDomElement, false, excludeTypes ); + modelClass = this.modelRegistry.lookup( modelName ) || ve.dm.AlienNode; + if ( + !( modelClass.prototype instanceof ve.dm.Annotation ) && + !( modelClass.prototype instanceof ve.dm.MetaItem ) + ) { + // If the element not meta or an annotation, then we must have content + return false; + } + // Recursively check children + if ( + childDomElement.childNodes.length && + !this.isDomAllMetaOrWhitespace( childDomElement.childNodes, excludeTypes ) + ) { + return false; + } + continue; + case Node.TEXT_NODE: + // Check for whitespace-only + if ( !childDomElement.data.match( /\S/ ) ) { + continue; + } + break; + case Node.COMMENT_NODE: + // Comments are always meta + continue; + } + return false; + } + return true; +}; + +/** * Convert linear model data to an HTML DOM * * @method diff --git a/modules/ve/dm/ve.dm.ModelRegistry.js b/modules/ve/dm/ve.dm.ModelRegistry.js index 0a90a48..ab87da6 100644 --- a/modules/ve/dm/ve.dm.ModelRegistry.js +++ b/modules/ve/dm/ve.dm.ModelRegistry.js @@ -176,9 +176,10 @@ * * @param {HTMLElement} element Element to match * @param {boolean} [forceAboutGrouping] If true, only match models with about grouping enabled + * @param {string[]} [excludeTypes] Model names to exclude when matching * @returns {string|null} Model type, or null if none found */ -ve.dm.ModelRegistry.prototype.matchElement = function ( element, forceAboutGrouping ) { +ve.dm.ModelRegistry.prototype.matchElement = function ( element, forceAboutGrouping, excludeTypes ) { var i, name, model, matches, winner, types, elementExtSpecificTypes, matchTypes, hasExtSpecificTypes, tag = element.nodeName.toLowerCase(), @@ -191,6 +192,9 @@ function matchTypeRegExps( type, tag, withFunc ) { var i, j, types, matches = [], models = reg.modelsWithTypeRegExps[+!!withFunc]; for ( i = 0; i < models.length; i++ ) { + if ( excludeTypes && ve.indexOf( models[i], excludeTypes ) !== -1 ) { + continue; + } types = reg.registry[models[i]].static.matchRdfaTypes; for ( j = 0; j < types.length; j++ ) { if ( @@ -237,6 +241,9 @@ for ( i = 0; i < types.length; i++ ) { // Queue string matches and regexp matches separately queue = queue.concat( ve.getProp( reg.modelsByTypeAndTag, 1, types[i], tag ) || [] ); + if ( excludeTypes ) { + queue = ve.simpleArrayDifference( queue, excludeTypes ); + } queue2 = queue2.concat( matchTypeRegExps( types[i], tag, true ) ); } if ( mustMatchAll ) { @@ -274,6 +281,9 @@ for ( i = 0; i < types.length; i++ ) { // Queue string and regexp matches separately queue = queue.concat( ve.getProp( reg.modelsByTypeAndTag, 0, types[i], tag ) || [] ); + if ( excludeTypes ) { + queue = ve.simpleArrayDifference( queue, excludeTypes ); + } queue2 = queue2.concat( matchTypeRegExps( types[i], tag, false ) ); } if ( mustMatchAll ) { diff --git a/modules/ve/test/ve.test.utils.js b/modules/ve/test/ve.test.utils.js index 97039b2..42d9033 100644 --- a/modules/ve/test/ve.test.utils.js +++ b/modules/ve/test/ve.test.utils.js @@ -78,7 +78,7 @@ for ( i = 0, length = cases[msg].storeItems.length; i < length; i++ ) { hash = cases[msg].storeItems[i].hash || ve.getHash( cases[msg].storeItems[i].value ); assert.deepEqualWithDomElements( - store.value( store.indexOfHash( hash ) ), + store.value( store.indexOfHash( hash ) ) || {}, cases[msg].storeItems[i].value, msg + ': store item ' + i + ' found' ); -- To view, visit https://gerrit.wikimedia.org/r/75331 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I89a220350fb7e10e15f3682d21438539196a5846 Gerrit-PatchSet: 5 Gerrit-Project: mediawiki/extensions/VisualEditor Gerrit-Branch: master Gerrit-Owner: Esanders <esand...@wikimedia.org> Gerrit-Reviewer: Catrope <roan.katt...@gmail.com> Gerrit-Reviewer: Esanders <esand...@wikimedia.org> Gerrit-Reviewer: Jforrester <jforres...@wikimedia.org> Gerrit-Reviewer: Krinkle <krinklem...@gmail.com> Gerrit-Reviewer: jenkins-bot _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits