Make .static.storeHtmlAttributes more versatile

It now allows you to specify which attributes to preserve in various
ways rather than just setting true or false.

Removed unused factory methods that exposed the old value.

Change-Id: I914164adcf1f0e48fa3fa85277e68c72dbad393e

VisualEditor.hooks.php

@@ -115,6 +115,7 @@ class VisualEditorHooks {
 				'dm/ve.dm.SurfaceFragment.test.js',
 				'dm/ve.dm.ModelRegistry.test.js',
 				'dm/ve.dm.MetaList.test.js',
+				'dm/ve.dm.Model.test.js',
 				'dm/lineardata/ve.dm.ElementLinearData.test.js',
 				'dm/lineardata/ve.dm.MetaLinearData.test.js',
 				// VisualEditor ContentEditable Tests

modules/ve/dm/ve.dm.Converter.js

@@ -243,16 +243,21 @@ ve.dm.Converter.prototype.createDataElements = function ( modelClass, domElement
 	if ( !ve.isArray( dataElements ) ) {
 		dataElements = [ dataElements ];
 	}
-	if ( dataElements[0] && modelClass.static.storeHtmlAttributes ) {
+	if ( dataElements[0] && modelClass.static.storeHtmlAttributes ) { // Optimization: skip if false
 		for ( i = 0; i < domElements.length; i++ ) {
 			domElementAttributes = domElements[i].attributes;
 			if ( domElementAttributes && domElementAttributes.length ) {
 				dataElementAttributes = dataElements[0].attributes = dataElements[0].attributes || {};
-				// Include all attributes and prepend 'html/i/' to each attribute name
+				// Store attributes and prepend 'html/i/' to each attribute name
 				for ( j = 0; j < domElementAttributes.length; j++ ) {
 					domElementAttribute = domElementAttributes[j];
-					dataElementAttributes['html/' + i + '/' + domElementAttribute.name] =
-						domElementAttribute.value;
+					if (
+						ve.dm.Model.matchesAttributeSpec( domElementAttribute.name,
+							modelClass.static.storeHtmlAttributes )
+					) {
+						dataElementAttributes['html/' + i + '/' + domElementAttribute.name] =
+							domElementAttribute.value;
+					}
 				}
 			}
 		}

modules/ve/dm/ve.dm.MetaItemFactory.js

@@ -23,21 +23,6 @@ ve.inheritClass( ve.dm.MetaItemFactory, ve.NamedClassFactory );
 
 /* Methods */
 
-/**
- * Check if the item stores HTML attributes in the meta-linmod.
- *
- * @method
- * @param {string} type Meta item type
- * @returns {boolean} Whether the item stores HTML attributes
- * @throws {Error} Unknown item type
- */
-ve.dm.MetaItemFactory.prototype.doesItemStoreHtmlAttributes = function ( type ) {
-	if ( type in this.registry ) {
-		return this.registry[type].static.storeHtmlAttributes;
-	}
-	throw new Error( 'Unknown item type: ' + type );
-};
-
 /**
  * Get the group a given item type belongs to.
  *

modules/ve/dm/ve.dm.Model.js

@@ -165,20 +165,73 @@ ve.dm.Model.static.toDomElements = function ( /*dataElement, doc, converter*/ )
 ve.dm.Model.static.enableAboutGrouping = false;
 
 /**
- * Whether HTML attributes should be preserved for this model type. If true, the HTML attributes
- * of the DOM elements will be stored as linear model attributes. The attribute names will be
- * html/i/attrName, where i is the index of the DOM element in the domElements array, and attrName
- * is the name of the attribute.
+ * Which HTML attributes should be preserved for this model type. HTML attributes on the DOM
+ * elements that match this specification will be stored as attributes in the linear model. The
+ * attribute names will be html/i/attrName, where i is the index of the DOM element in the
+ * domElements array, and attrName is the name of the attribute. When converting back to DOM, these
+ * HTML attributes will be restored except for attributes that were already set by toDomElements().
  *
- * This should generally be enabled, except for model types that store their entire HTML in an
- * attribute.
+ * The value of this attribute can be one of the following:
+ * - true, to preserve all attributes (default)
+ * - false, to preserve none
+ * - a string, to preserve only that attribute
+ * - a regular expression matching attributes that should be preserved
+ * - an array of strings or regular expressions
+ * - an object with the following keys:
+ *   - 'blacklist': specification of attributes not to preserve (boolean|string|RegExp|Array)
+ *   - 'whitelist': specification of attributes to preserve
+ *
+ * If only a blacklist is specified, all attributes will be preserved except the ones matching
+ * the blacklist. If only a whitelist is specified, only those attributes matching the whitelist
+ * will be preserved. If both are specified, only attributes that both match the whitelist and
+ * do not match the blacklist will be preserved.
  *
  * @static
- * @property {boolean} static.storeHtmlAttributes
+ * @property {boolean|string|RegExp|Array|Object} static.storeHtmlAttributes
  * @inheritable
  */
 ve.dm.Model.static.storeHtmlAttributes = true;
 
+/* Static methods */
+
+/**
+ * Determine whether an attribute name matches an attribute specification.
+ *
+ * @param {string} attribute Attribute name
+ * @param {boolean|string|RegExp|Array|Object} spec Attribute specification, see ve.dm.Model.static.storeHtmlAttributes
+ * @returns {boolean} Attribute matches spec
+ */
+ve.dm.Model.matchesAttributeSpec = function ( attribute, spec ) {
+	function matches( subspec ) {
+		if ( subspec instanceof RegExp ) {
+			return !!subspec.exec( attribute );
+		}
+		if ( typeof subspec === 'boolean' ) {
+			return subspec;
+		}
+		return attribute === subspec;
+	}
+
+	function matchesArray( specArray ) {
+		var i, len;
+		if ( !ve.isArray( specArray ) ) {
+			specArray = [ specArray ];
+		}
+		for ( i = 0, len = specArray.length; i < len; i++ ) {
+			if  ( matches( specArray[i] ) ) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+	if ( spec.whitelist === undefined && spec.blacklist === undefined ) {
+		// Not an object, treat spec as a whitelist
+		return matchesArray( spec );
+	}
+	return matchesArray( spec.whitelist || true ) && !matchesArray( spec.blacklist || false );
+};
+
 /* Methods */
 
 /**

modules/ve/dm/ve.dm.NodeFactory.js

@@ -186,21 +186,6 @@ ve.dm.NodeFactory.prototype.doesNodeHaveSignificantWhitespace = function ( type
 	throw new Error( 'Unknown node type: ' + type );
 };
 
-/**
- * Check if the node stores HTML attributes in the linear model.
- *
- * @method
- * @param {string} type Node type
- * @returns {boolean} Whether the node stores HTML attributes
- * @throws {Error} Unknown node type
- */
-ve.dm.NodeFactory.prototype.doesNodeStoreHtmlAttributes = function ( type ) {
-	if ( type in this.registry ) {
-		return this.registry[type].static.storeHtmlAttributes;
-	}
-	throw new Error( 'Unknown node type: ' + type );
-};
-
 /**
  * Check if the node handles its own children.
  *

modules/ve/test/dm/ve.dm.Model.test.js

@@ -0,0 +1,122 @@
+/*!
+ * VisualEditor Model tests.
+ *
+ * @copyright 2011-2013 VisualEditor Team and others; see AUTHORS.txt
+ * @license The MIT License (MIT); see LICENSE.txt
+ */
+
+QUnit.module( 've.dm.Model' );
+
+QUnit.test( 'matchesAttributeSpec', function ( assert ) {
+	var i, cases = [
+		{
+			'spec': true,
+			'attr': 'foo',
+			'result': true,
+			'msg': 'true matches anything'
+		},
+		{
+			'spec': false,
+			'attr': 'foo',
+			'result': false,
+			'msg': 'false matches nothing'
+		},
+		{
+			'spec': 'foo',
+			'attr': 'foo',
+			'result': true,
+			'msg': 'string matches exact value'
+		},
+		{
+			'spec': 'foo',
+			'attr': 'bar',
+			'result': false,
+			'msg': 'string does not match anything else'
+		},
+		{
+			'spec': /^foo/,
+			'attr': 'foobar',
+			'result': true,
+			'msg': 'regex matches'
+		},
+		{
+			'spec': /^foo/,
+			'attr': 'barfoo',
+			'result': false,
+			'msg': 'regex does not match anything else'
+		},
+		{
+			'spec': [ 'foo', /^bar/ ],
+			'attr': 'foo',
+			'result': true,
+			'msg': 'array of string and regex matches string'
+		},
+		{
+			'spec': [ 'foo', /^bar/ ],
+			'attr': 'barbaz',
+			'result': true,
+			'msg': 'array of string and regex matches regex match'
+		},
+		{
+			'spec': {
+				'blacklist': 'foo'
+			},
+			'attr': 'foo',
+			'result': false,
+			'msg': 'blacklisted attribute does not match'
+		},
+		{
+			'spec': {
+				'blacklist': 'foo'
+			},
+			'attr': 'bar',
+			'result': true,
+			'msg': 'non-blacklisted attribute matches'
+		},
+		{
+			'spec': {
+				'whitelist': /^foo/,
+				'blacklist': [ 'foobar', 'foobaz' ]
+			},
+			'attr': 'foobar',
+			'result': false,
+			'msg': 'blacklist overrides whitelist'
+		},
+		{
+			'spec': {
+				'whitelist': /^foo/,
+				'blacklist':  [ 'foobar', 'foobaz' ],
+			},
+			'attr': 'foobaz',
+			'result': false,
+			'msg': 'blacklist overrides whitelist (2)'
+		},
+		{
+			'spec': {
+				'whitelist': /^foo/,
+				'blacklist': [ 'foobar', 'foobaz' ],
+			},
+			'attr': 'fooquux',
+			'result': true,
+			'msg': 'attribute on whitelist and not on blacklist matches'
+		},
+		{
+			'spec': {
+				'whitelist': /^foo/,
+				'blacklist': [ 'foobar', 'foobaz' ],
+			},
+			'attr': 'bar',
+			'result': false,
+			'msg': 'attribute on neither whitelist nor blacklist does not match'
+		}
+	];
+
+	QUnit.expect( cases.length );
+	for ( i = 0; i < cases.length; i++ ) {
+		assert.equal(
+			ve.dm.Model.matchesAttributeSpec( cases[i].attr, cases[i].spec ),
+			cases[i].result,
+			cases[i].msg
+		);
+	}
+} );

modules/ve/test/index.php

@@ -290,6 +290,7 @@
 		<script src="dm/ve.dm.SurfaceFragment.test.js"></script>
 		<script src="dm/ve.dm.ModelRegistry.test.js"></script>
 		<script src="dm/ve.dm.MetaList.test.js"></script>
+		<script src="dm/ve.dm.Model.test.js"></script>
 		<script src="dm/lineardata/ve.dm.ElementLinearData.test.js"></script>
 		<script src="dm/lineardata/ve.dm.MetaLinearData.test.js"></script>
 		<script src="ce/ve.ce.test.js"></script>