jenkins-bot has submitted this change and it was merged.
Change subject: Make .static.storeHtmlAttributes more versatile
......................................................................
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
---
M VisualEditor.hooks.php
M modules/ve/dm/ve.dm.Converter.js
M modules/ve/dm/ve.dm.MetaItemFactory.js
M modules/ve/dm/ve.dm.Model.js
M modules/ve/dm/ve.dm.NodeFactory.js
A modules/ve/test/dm/ve.dm.Model.test.js
M modules/ve/test/index.php
7 files changed, 193 insertions(+), 41 deletions(-)
Approvals:
Trevor Parscal: Looks good to me, approved
jenkins-bot: Verified
diff --git a/VisualEditor.hooks.php b/VisualEditor.hooks.php
index d29ca3b..6bb208a 100644
--- a/VisualEditor.hooks.php
+++ b/VisualEditor.hooks.php
@@ -115,6 +115,7 @@
'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
diff --git a/modules/ve/dm/ve.dm.Converter.js b/modules/ve/dm/ve.dm.Converter.js
index b00ba72..bcb3b91 100644
--- a/modules/ve/dm/ve.dm.Converter.js
+++ b/modules/ve/dm/ve.dm.Converter.js
@@ -243,16 +243,21 @@
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;
+ }
}
}
}
diff --git a/modules/ve/dm/ve.dm.MetaItemFactory.js
b/modules/ve/dm/ve.dm.MetaItemFactory.js
index 04aae66..7a8de93 100644
--- a/modules/ve/dm/ve.dm.MetaItemFactory.js
+++ b/modules/ve/dm/ve.dm.MetaItemFactory.js
@@ -24,21 +24,6 @@
/* 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.
*
* @method
diff --git a/modules/ve/dm/ve.dm.Model.js b/modules/ve/dm/ve.dm.Model.js
index 77043fa..dc86e1c 100644
--- a/modules/ve/dm/ve.dm.Model.js
+++ b/modules/ve/dm/ve.dm.Model.js
@@ -165,20 +165,73 @@
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 */
/**
diff --git a/modules/ve/dm/ve.dm.NodeFactory.js
b/modules/ve/dm/ve.dm.NodeFactory.js
index bcbbfc5..670d0fe 100644
--- a/modules/ve/dm/ve.dm.NodeFactory.js
+++ b/modules/ve/dm/ve.dm.NodeFactory.js
@@ -187,21 +187,6 @@
};
/**
- * 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.
*
* @method
diff --git a/modules/ve/test/dm/ve.dm.Model.test.js
b/modules/ve/test/dm/ve.dm.Model.test.js
new file mode 100644
index 0000000..94244ea
--- /dev/null
+++ b/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
+ );
+ }
+} );
diff --git a/modules/ve/test/index.php b/modules/ve/test/index.php
index 4a08966..f344e80 100644
--- a/modules/ve/test/index.php
+++ b/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>
--
To view, visit https://gerrit.wikimedia.org/r/62737
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I914164adcf1f0e48fa3fa85277e68c72dbad393e
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Trevor Parscal <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
