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="{&quot;id&quot;:&quot;mwt1&quot;,&quot;target&quot;:{&quot;wt&quot;:&quot;Inline&quot;},&quot;params&quot;:{&quot;1&quot;:{&quot;wt&quot;:&quot;5,678&quot;}}}"><span
 about="#mwt1">Foo</span>',
        'pairOne': '<p about="#mwt1" typeof="mw:Transclusion" 
data-mw="{&quot;params&quot;:{&quot;1&quot;:{&quot;wt&quot;:&quot;foo&quot;}}}" 
data-parsoid="1">foo</p>',
-       'pairTwo': '<p about="#mwt2" typeof="mw:Transclusion" 
data-mw="{&quot;params&quot;:{&quot;1&quot;:{&quot;wt&quot;:&quot;foo&quot;}}}" 
data-parsoid="2">foo</p>'
+       'pairTwo': '<p about="#mwt2" typeof="mw:Transclusion" 
data-mw="{&quot;params&quot;:{&quot;1&quot;:{&quot;wt&quot;:&quot;foo&quot;}}}" 
data-parsoid="2">foo</p>',
+       'meta':
+               '<link rel="mw:WikiLink/Category" href="./Category:Page" 
about="#mwt1" typeof="mw:Transclusion" ' +
+                       
'data-mw="{&quot;target&quot;:{&quot;wt&quot;:&quot;Template:Echo&quot;,&quot;href&quot;:&quot;./Template:Echo&quot;},&quot;params&quot;:{&quot;1&quot;:{&quot;wt&quot;:&quot;[[Category:Page]]\\n[[Category:Book]]&quot;}},&quot;i&quot;: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

Reply via email to