Ori.livneh has uploaded a new change for review.
https://gerrit.wikimedia.org/r/191287
Change subject: Replace ve.dm.Model#matchesAttributeSpec
......................................................................
Replace ve.dm.Model#matchesAttributeSpec
Attribute matcher specifications can take any form from the list below:
* Object with an array 'whitelist' property, containing any mix of booleans,
regular expressions, and strings.
* Object with an array 'blacklist' property (same composition as above).
* Object with both 'whitelist' and 'blacklist' properties (ditto).
* Array (ditto).
* Regular expression.
* Boolean.
This flexibility is purchased at the cost of a lot of branching in a very hot
code path (ve.dm.Model#matchesAttributeSpec gets called for every property of
every element).
This patch allows attribute specifications to be either a boolean value or a
function which takes an attribute name and returns a boolean value. This allows
for dynamic and complex specifications without incurring costs for the common
cases.
Change-Id: Ia0e0fe4fc2385dab67c816148cfc362a39413dac
---
M src/ce/ve.ce.View.js
M src/dm/nodes/ve.dm.BlockImageNode.js
M src/dm/nodes/ve.dm.TableCellNode.js
M src/dm/ve.dm.Converter.js
M src/dm/ve.dm.Model.js
5 files changed, 29 insertions(+), 65 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor
refs/changes/87/191287/1
diff --git a/src/ce/ve.ce.View.js b/src/ce/ve.ce.View.js
index d69bb81..3bd2e35 100644
--- a/src/ce/ve.ce.View.js
+++ b/src/ce/ve.ce.View.js
@@ -79,14 +79,16 @@
* @property {boolean|string|RegExp|Array|Object}
* @inheritable
*/
-ve.ce.View.static.renderHtmlAttributes = [
- 'abbr', 'about', 'align', 'alt', 'axis', 'bgcolor', 'border',
'cellpadding', 'cellspacing',
- 'char', 'charoff', 'cite', 'class', 'clear', 'color', 'colspan',
'datatype', 'datetime',
- 'dir', 'face', 'frame', 'headers', 'height', 'href', 'id', 'itemid',
'itemprop', 'itemref',
- 'itemscope', 'itemtype', 'lang', 'noshade', 'nowrap', 'property',
'rbspan', 'rel',
- 'resource', 'rev', 'rowspan', 'rules', 'scope', 'size', 'span', 'src',
'start', 'style',
- 'summary', 'title', 'type', 'typeof', 'valign', 'value', 'width'
-];
+ve.ce.View.static.renderHtmlAttributes = function ( attribute ) {
+ return ve.indexOf( attribute, [
+ 'abbr', 'about', 'align', 'alt', 'axis', 'bgcolor', 'border',
'cellpadding', 'cellspacing',
+ 'char', 'charoff', 'cite', 'class', 'clear', 'color',
'colspan', 'datatype', 'datetime',
+ 'dir', 'face', 'frame', 'headers', 'height', 'href', 'id',
'itemid', 'itemprop', 'itemref',
+ 'itemscope', 'itemtype', 'lang', 'noshade', 'nowrap',
'property', 'rbspan', 'rel',
+ 'resource', 'rev', 'rowspan', 'rules', 'scope', 'size', 'span',
'src', 'start', 'style',
+ 'summary', 'title', 'type', 'typeof', 'valign', 'value', 'width'
+ ] ) === -1;
+};
/* Methods */
diff --git a/src/dm/nodes/ve.dm.BlockImageNode.js
b/src/dm/nodes/ve.dm.BlockImageNode.js
index f675066..bd73bcd 100644
--- a/src/dm/nodes/ve.dm.BlockImageNode.js
+++ b/src/dm/nodes/ve.dm.BlockImageNode.js
@@ -37,8 +37,8 @@
ve.dm.BlockImageNode.static.name = 'blockImage';
-ve.dm.BlockImageNode.static.storeHtmlAttributes = {
- blacklist: [ 'src', 'width', 'height', 'href' ]
+ve.dm.BlockImageNode.static.storeHtmlAttributes = function ( attribute ) {
+ return ve.indexOf( attribute, [ 'src', 'width', 'height', 'href' ] )
=== -1;
};
ve.dm.BlockImageNode.static.handlesOwnChildren = true;
diff --git a/src/dm/nodes/ve.dm.TableCellNode.js
b/src/dm/nodes/ve.dm.TableCellNode.js
index 53b3d7c..a017a51 100644
--- a/src/dm/nodes/ve.dm.TableCellNode.js
+++ b/src/dm/nodes/ve.dm.TableCellNode.js
@@ -39,8 +39,8 @@
ve.dm.TableCellNode.static.matchTagNames = [ 'td', 'th' ];
// Blacklisting 'colspan' and 'rowspan' as they are managed explicitly
-ve.dm.TableCellNode.static.storeHtmlAttributes = {
- blacklist: ['colspan', 'rowspan']
+ve.dm.TableCellNode.static.storeHtmlAttributes = function ( attribute ) {
+ return attribute !== 'colspan' && attribute !== 'rowspan';
};
/* Static Methods */
diff --git a/src/dm/ve.dm.Converter.js b/src/dm/ve.dm.Converter.js
index 686739a..6eca453 100644
--- a/src/dm/ve.dm.Converter.js
+++ b/src/dm/ve.dm.Converter.js
@@ -143,21 +143,21 @@
* @param {Object[]} [attributeList] Existing attribute list to populate; used
for recursion
* @returns {Object[]|undefined} Attribute list, or undefined if empty
*/
-ve.dm.Converter.buildHtmlAttributeList = function ( domElements, spec, deep,
attributeList ) {
+ve.dm.Converter.buildHtmlAttributeList = function ( domElements, filter, deep,
attributeList ) {
var i, ilen, j, jlen, domAttributes, childList, attrName,
empty = true;
attributeList = attributeList || [];
for ( i = 0, ilen = domElements.length; i < ilen; i++ ) {
+ attributeList.push( { values: {}, computed: {} } );
+ if ( !filter ) {
+ continue;
+ }
domAttributes = domElements[i].attributes || [];
- attributeList[i] = { values: {} };
for ( j = 0, jlen = domAttributes.length; j < jlen; j++ ) {
attrName = domAttributes[j].name;
- if ( ve.dm.Model.matchesAttributeSpec( attrName, spec )
) {
+ if ( filter === true || filter( attrName ) ) {
attributeList[i].values[attrName] =
domAttributes[j].value;
if ( ve.indexOf( attrName,
this.computedAttributes ) !== -1 ) {
- if ( !attributeList[i].computed ) {
- attributeList[i].computed = {};
- }
attributeList[i].computed[attrName] =
domElements[i][attrName];
}
empty = false;
@@ -169,7 +169,7 @@
// Use .children rather than .childNodes so we
don't mess around with things that
// can't have attributes anyway. Unfortunately,
non-element nodes have .children
// set to undefined so we have to coerce it to
an array in that case.
- domElements[i].children || [], spec, deep,
attributeList[i].children
+ domElements[i].children || [], filter, deep,
attributeList[i].children
);
if ( childList ) {
empty = false;
@@ -190,16 +190,16 @@
* @static
* @param {Object[]} attributeList Attribute list, see buildHtmlAttributeList()
* @param {HTMLElement[]} domElements Array of DOM elements to render onto
- * @param {boolean|string|RegExp|Array|Object} [spec=true] Attribute
specification, see ve.dm.Model
+ * @param {boolean|Function} [filter=true] Attribute filter
* @param {boolean} [computed=false] If true, use the computed values of
attributes where available
* @param {boolean} [overwrite=false] If true, overwrite attributes that are
already set
*/
-ve.dm.Converter.renderHtmlAttributeList = function ( attributeList,
domElements, spec, computed, overwrite ) {
+ve.dm.Converter.renderHtmlAttributeList = function ( attributeList,
domElements, filter, computed, overwrite ) {
var i, ilen, key, values, value;
- if ( spec === undefined ) {
- spec = true;
+ if ( filter === undefined ) {
+ filter = true;
}
- if ( spec === false ) {
+ if ( filter === false ) {
return;
}
for ( i = 0, ilen = attributeList.length; i < ilen; i++ ) {
@@ -208,7 +208,7 @@
}
values = attributeList[i].values;
for ( key in values ) {
- if ( ve.dm.Model.matchesAttributeSpec( key, spec ) ) {
+ if ( filter === true || filter( key ) ) {
value = computed && attributeList[i].computed
&& attributeList[i].computed[key] || values[key];
if ( value === undefined ) {
domElements[i].removeAttribute( key );
@@ -219,7 +219,7 @@
}
if ( attributeList[i].children ) {
ve.dm.Converter.renderHtmlAttributeList(
- attributeList[i].children,
domElements[i].children, spec, computed, overwrite
+ attributeList[i].children,
domElements[i].children, filter, computed, overwrite
);
}
}
@@ -783,7 +783,7 @@
!this.nodeFactory.doesNodeHandleOwnChildren( childDataElements[0].type )
) {
htmlAttributes =
ve.dm.Converter.buildHtmlAttributeList(
- childNodes,
modelClass.static.storeHtmlAttributes
+ childNodes,
modelClass.static.storeHtmlAttributes, false
);
if ( htmlAttributes ) {
childDataElements[0].htmlAttributes = htmlAttributes;
diff --git a/src/dm/ve.dm.Model.js b/src/dm/ve.dm.Model.js
index 08fc63d..fe4b9fd 100644
--- a/src/dm/ve.dm.Model.js
+++ b/src/dm/ve.dm.Model.js
@@ -205,44 +205,6 @@
/* 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 #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 ( !Array.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 );
-};
-
-/**
* Get hash object of a linear model data element.
*
* @static
--
To view, visit https://gerrit.wikimedia.org/r/191287
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia0e0fe4fc2385dab67c816148cfc362a39413dac
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Ori.livneh <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
