Esanders has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/205291

Change subject: Add removable flag to meta items
......................................................................

Add removable flag to meta items

Meta items which are marked as removable (to begin with, just empty
annotations) will get deleted by operations which otherwise would result
in them being merged to a different offset.

Bug: T96528
Change-Id: I71ece0cf20513defeee7ec5ecd395c666113b4d5
---
M build/modules.json
M demos/ve/desktop.html
M demos/ve/mobile.html
A src/dm/metaitems/ve.dm.RemovableAlienMetaItem.js
M src/dm/ve.dm.Converter.js
M src/dm/ve.dm.MetaItem.js
M src/dm/ve.dm.MetaItemFactory.js
M src/dm/ve.dm.Transaction.js
M tests/dm/ve.dm.Transaction.test.js
M tests/dm/ve.dm.example.js
M tests/index.html
11 files changed, 95 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor 
refs/changes/91/205291/1

diff --git a/build/modules.json b/build/modules.json
index 216c1d9..af8b6ea 100644
--- a/build/modules.json
+++ b/build/modules.json
@@ -293,6 +293,7 @@
                        "src/dm/annotations/ve.dm.UserInputAnnotation.js",
                        "src/dm/annotations/ve.dm.VariableAnnotation.js",
                        "src/dm/metaitems/ve.dm.AlienMetaItem.js",
+                       "src/dm/metaitems/ve.dm.RemovableAlienMetaItem.js",
                        "src/dm/metaitems/ve.dm.CommentMetaItem.js",
                        "src/dm/nodes/ve.dm.CommentNode.js",
                        "src/ce/ve.ce.js",
diff --git a/demos/ve/desktop.html b/demos/ve/desktop.html
index 5b89fa9..adcbef3 100644
--- a/demos/ve/desktop.html
+++ b/demos/ve/desktop.html
@@ -254,6 +254,7 @@
                <script 
src="../../src/dm/annotations/ve.dm.UserInputAnnotation.js"></script>
                <script 
src="../../src/dm/annotations/ve.dm.VariableAnnotation.js"></script>
                <script 
src="../../src/dm/metaitems/ve.dm.AlienMetaItem.js"></script>
+               <script 
src="../../src/dm/metaitems/ve.dm.RemovableAlienMetaItem.js"></script>
                <script 
src="../../src/dm/metaitems/ve.dm.CommentMetaItem.js"></script>
                <script src="../../src/dm/nodes/ve.dm.CommentNode.js"></script>
                <script src="../../src/ce/ve.ce.js"></script>
diff --git a/demos/ve/mobile.html b/demos/ve/mobile.html
index 68d39a8..80a4eac 100644
--- a/demos/ve/mobile.html
+++ b/demos/ve/mobile.html
@@ -255,6 +255,7 @@
                <script 
src="../../src/dm/annotations/ve.dm.UserInputAnnotation.js"></script>
                <script 
src="../../src/dm/annotations/ve.dm.VariableAnnotation.js"></script>
                <script 
src="../../src/dm/metaitems/ve.dm.AlienMetaItem.js"></script>
+               <script 
src="../../src/dm/metaitems/ve.dm.RemovableAlienMetaItem.js"></script>
                <script 
src="../../src/dm/metaitems/ve.dm.CommentMetaItem.js"></script>
                <script src="../../src/dm/nodes/ve.dm.CommentNode.js"></script>
                <script src="../../src/ce/ve.ce.js"></script>
diff --git a/src/dm/metaitems/ve.dm.RemovableAlienMetaItem.js 
b/src/dm/metaitems/ve.dm.RemovableAlienMetaItem.js
new file mode 100644
index 0000000..bc51c24
--- /dev/null
+++ b/src/dm/metaitems/ve.dm.RemovableAlienMetaItem.js
@@ -0,0 +1,34 @@
+/*!
+ * VisualEditor DataModel RemovableAlienMetaItem class.
+ *
+ * @copyright 2011-2015 VisualEditor Team and others; see 
http://ve.mit-license.org
+ */
+
+/**
+ * DataModel removable alien meta item.
+ *
+ * @class
+ * @extends ve.dm.AlienMetaItem
+ * @constructor
+ * @param {Object} element Reference to element in meta-linmod
+ */
+ve.dm.RemovableAlienMetaItem = function VeDmRemovableAlienMetaItem() {
+       // Parent constructor
+       ve.dm.RemovableAlienMetaItem.super.apply( this, arguments );
+};
+
+/* Inheritance */
+
+OO.inheritClass( ve.dm.RemovableAlienMetaItem, ve.dm.AlienMetaItem );
+
+/* Static Properties */
+
+ve.dm.RemovableAlienMetaItem.static.name = 'removableAlienMeta';
+
+ve.dm.RemovableAlienMetaItem.static.matchTagNames = [];
+
+ve.dm.RemovableAlienMetaItem.static.removable = true;
+
+/* Registration */
+
+ve.dm.modelRegistry.register( ve.dm.RemovableAlienMetaItem );
diff --git a/src/dm/ve.dm.Converter.js b/src/dm/ve.dm.Converter.js
index b039a8b..7ebfcb6 100644
--- a/src/dm/ve.dm.Converter.js
+++ b/src/dm/ve.dm.Converter.js
@@ -693,7 +693,11 @@
                                        childDataElements = 
this.getDataFromDomSubtree( childNode, undefined, childAnnotations );
                                        if ( !childDataElements.length || 
isAllInstanceOf( childDataElements, ve.dm.AlienMetaItem ) ) {
                                                // Empty annotation, create a 
meta item
-                                               childDataElements = 
this.createDataElements( ve.dm.AlienMetaItem, childNodes );
+                                               if ( !childDataElements.length 
|| isAllInstanceOf( childDataElements, ve.dm.RemovableAlienMetaItem ) ) {
+                                                       childDataElements = 
this.createDataElements( ve.dm.RemovableAlienMetaItem, childNodes );
+                                               } else {
+                                                       childDataElements = 
this.createDataElements( ve.dm.AlienMetaItem, childNodes );
+                                               }
                                                childDataElements.push( { type: 
'/' + childDataElements[0].type } );
                                                // Annotate meta item
                                                if ( 
!context.annotations.isEmpty() ) {
diff --git a/src/dm/ve.dm.MetaItem.js b/src/dm/ve.dm.MetaItem.js
index cd2f7d4..1ebf41a 100644
--- a/src/dm/ve.dm.MetaItem.js
+++ b/src/dm/ve.dm.MetaItem.js
@@ -40,11 +40,20 @@
  * Symbolic name for the group this meta item type will be grouped in in 
ve.dm.MetaList.
  *
  * @static
- * @property
+ * @property {string}
  * @inheritable
  */
 ve.dm.MetaItem.static.group = 'misc';
 
+/**
+ * If the metaitem can be removed by regular remove operations
+ *
+ * @static
+ * @property {boolean}
+ * @inheritable
+ */
+ve.dm.MetaItem.static.removable = false;
+
 /* Methods */
 
 /**
diff --git a/src/dm/ve.dm.MetaItemFactory.js b/src/dm/ve.dm.MetaItemFactory.js
index eda3e5f..f681e6e 100644
--- a/src/dm/ve.dm.MetaItemFactory.js
+++ b/src/dm/ve.dm.MetaItemFactory.js
@@ -37,6 +37,21 @@
        throw new Error( 'Unknown item type: ' + type );
 };
 
+/**
+ * Check if a given type is removable
+ *
+ * @method
+ * @param {string} type Meta item type
+ * @returns {string} The type is removable
+ * @throws {Error} Unknown item type
+ */
+ve.dm.MetaItemFactory.prototype.isRemovable = function ( type ) {
+       if ( Object.prototype.hasOwnProperty.call( this.registry, type ) ) {
+               return this.registry[type].static.removable;
+       }
+       throw new Error( 'Unknown item type: ' + type );
+};
+
 /* Initialization */
 
 ve.dm.metaItemFactory = new ve.dm.MetaItemFactory();
diff --git a/src/dm/ve.dm.Transaction.js b/src/dm/ve.dm.Transaction.js
index 48231e6..e5b8657 100644
--- a/src/dm/ve.dm.Transaction.js
+++ b/src/dm/ve.dm.Transaction.js
@@ -1184,7 +1184,7 @@
                return;
        }
 
-       var extraMetadata,
+       var extraMetadata, i, l,
                end = this.operations.length - 1,
                lastOp = end >= 0 ? this.operations[end] : null,
                penultOp = end >= 1 ? this.operations[ end - 1 ] : null,
@@ -1288,6 +1288,13 @@
        this.pushReplaceInternal( remove, insert, removeMetadata, 
insertMetadata, insertedDataOffset, insertedDataLength );
 
        if ( extraMetadata !== undefined ) {
+               for ( i = 0, l = extraMetadata.length; i < l; i++ ) {
+                       if ( ve.dm.metaItemFactory.isRemovable( 
extraMetadata[i].type ) ) {
+                               extraMetadata.splice( i, 1 );
+                               i--;
+                               l--;
+                       }
+               }
                this.pushReplaceMetadata( [], extraMetadata );
        }
 };
diff --git a/tests/dm/ve.dm.Transaction.test.js 
b/tests/dm/ve.dm.Transaction.test.js
index ab4a7b5..f2e3273 100644
--- a/tests/dm/ve.dm.Transaction.test.js
+++ b/tests/dm/ve.dm.Transaction.test.js
@@ -666,23 +666,24 @@
                                        { type: 'retain', length: 6 }
                                ]
                        },
-                       'removing content spanning metadata': {
-                               args: [metaDoc, new ve.Range( 7, 9 )],
+                       'removing content spanning metadata and removable 
metadata': {
+                               args: [metaDoc, new ve.Range( 7, 10 )],
                                ops: [
                                        { type: 'retain', length: 7 },
                                        {
                                                type: 'replace',
-                                               remove: ['B', 'a'],
+                                               remove: ['B', 'a', 'z'],
                                                insert: [],
-                                               removeMetadata: 
metaDoc.getMetadata().slice( 7, 9 ),
+                                               removeMetadata: 
metaDoc.getMetadata().slice( 7, 10 ),
                                                insertMetadata: []
                                        },
                                        {
                                                type: 'replaceMetadata',
                                                remove: [],
+                                               // Doesn't insert metadata at 
offset 10 as it is removable
                                                insert: 
ve.dm.MetaLinearData.static.merge( metaDoc.getMetadata().slice( 7, 9 ) )[0]
                                        },
-                                       { type: 'retain', length: 4 }
+                                       { type: 'retain', length: 3 }
                                ]
                        },
                        'selection including internal nodes doesn\'t remove 
them': {
diff --git a/tests/dm/ve.dm.example.js b/tests/dm/ve.dm.example.js
index 12bf23e..9e6fe16 100644
--- a/tests/dm/ve.dm.example.js
+++ b/tests/dm/ve.dm.example.js
@@ -565,10 +565,10 @@
        'B',
        'a',
        {
-               type: 'alienMeta',
-               originalDomElements: $( '<!-- inline -->' ).toArray()
+               type: 'removableAlienMeta',
+               originalDomElements: $( '<b></b>' ).toArray()
        },
-       { type: '/alienMeta' },
+       { type: '/removableAlienMeta' },
        'z',
        { type: '/paragraph' },
        {
@@ -642,8 +642,8 @@
        undefined,
        [
                {
-                       type: 'alienMeta',
-                       originalDomElements: $( '<!-- inline -->' ).toArray()
+                       type: 'removableAlienMeta',
+                       originalDomElements: $( '<b></b>' ).toArray()
                }
        ],
        undefined,
@@ -1759,10 +1759,10 @@
                        { type: 'paragraph' },
                        'F', 'o', 'o',
                        {
-                               type: 'alienMeta',
+                               type: 'removableAlienMeta',
                                originalDomElements: $( '<span 
id="anchorTarget"></span>' ).toArray()
                        },
-                       { type: '/alienMeta' },
+                       { type: '/removableAlienMeta' },
                        'B', 'a', 'r',
                        { type: '/paragraph' },
                        { type: 'internalList' },
@@ -1775,10 +1775,10 @@
                        { type: 'paragraph', internal: { generated: 'wrapper' } 
},
                        'F', 'o', 'o',
                        {
-                               type: 'alienMeta',
+                               type: 'removableAlienMeta',
                                originalDomElements: $( '<span 
id="anchorTarget"></span>' ).toArray()
                        },
-                       { type: '/alienMeta' },
+                       { type: '/removableAlienMeta' },
                        'B', 'a', 'r',
                        { type: '/paragraph' },
                        { type: 'internalList' },
@@ -1791,10 +1791,10 @@
                        { type: 'paragraph' },
                        'F', 'o', 'o',
                        {
-                               type: 'alienMeta',
+                               type: 'removableAlienMeta',
                                originalDomElements: $( '<i><b><u></u></b></i>' 
).toArray()
                        },
-                       { type: '/alienMeta' },
+                       { type: '/removableAlienMeta' },
                        'B', 'a', 'r',
                        { type: '/paragraph' },
                        { type: 'internalList' },
@@ -1809,11 +1809,11 @@
                        [ 'o', [ ve.dm.example.italic ] ],
                        [ 'o', [ ve.dm.example.italic ] ],
                        {
-                               type: 'alienMeta',
+                               type: 'removableAlienMeta',
                                originalDomElements: $( '<b></b>' ).toArray(),
                                annotations: [ ve.dm.example.italic ]
                        },
-                       { type: '/alienMeta' },
+                       { type: '/removableAlienMeta' },
                        { type: '/paragraph' },
                        { type: 'internalList' },
                        { type: '/internalList' }
diff --git a/tests/index.html b/tests/index.html
index ab28460..e7f0405 100644
--- a/tests/index.html
+++ b/tests/index.html
@@ -184,6 +184,7 @@
                <script 
src="../src/dm/annotations/ve.dm.UserInputAnnotation.js"></script>
                <script 
src="../src/dm/annotations/ve.dm.VariableAnnotation.js"></script>
                <script 
src="../src/dm/metaitems/ve.dm.AlienMetaItem.js"></script>
+               <script 
src="../src/dm/metaitems/ve.dm.RemovableAlienMetaItem.js"></script>
                <script 
src="../src/dm/metaitems/ve.dm.CommentMetaItem.js"></script>
                <script src="../src/dm/nodes/ve.dm.CommentNode.js"></script>
                <script src="../src/ce/ve.ce.js"></script>

-- 
To view, visit https://gerrit.wikimedia.org/r/205291
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I71ece0cf20513defeee7ec5ecd395c666113b4d5
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to