[MediaWiki-commits] [Gerrit] Also render attributes of the form html/i-j/attrName in CE - change (mediawiki...VisualEditor)

2013-05-08 Thread Catrope (Code Review)
Catrope has uploaded a new change for review.

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


Change subject: Also render attributes of the form html/i-j/attrName in CE
..

Also render attributes of the form html/i-j/attrName in CE

Factored the parsing of html/* attributes out into a static function.
Factored attribute (re)rendering out into ce.View, attribute updates
are much simpler now.

Change-Id: I4caa6d5e1e2c21c28ddff61c3c864e47f66cc6b2
---
M modules/ve/ce/ve.ce.Node.js
M modules/ve/ce/ve.ce.View.js
M modules/ve/dm/ve.dm.Converter.js
3 files changed, 49 insertions(+), 33 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor 
refs/changes/87/62787/1

diff --git a/modules/ve/ce/ve.ce.Node.js b/modules/ve/ce/ve.ce.Node.js
index 16cb9e7..2a5bf96 100644
--- a/modules/ve/ce/ve.ce.Node.js
+++ b/modules/ve/ce/ve.ce.Node.js
@@ -63,19 +63,7 @@
  * @param {string} to New value
  */
 ve.ce.Node.prototype.onAttributeChange = function ( key, from, to ) {
-   var htmlKey = key.substr( 7 ).toLowerCase();
-   if (
-   this.constructor.static.renderHtmlAttributes 
-   key.indexOf( 'html/0/' ) === 0 
-   htmlKey.length 
-   this.constructor.static.domAttributeWhitelist.indexOf( htmlKey 
) !== -1
-   ) {
-   if ( to === undefined ) {
-   this.$.removeAttr( htmlKey );
-   } else {
-   this.$.attr( htmlKey, to );
-   }
-   }
+   this.renderAttributes( { key: to } );
 };
 
 /**
diff --git a/modules/ve/ce/ve.ce.View.js b/modules/ve/ce/ve.ce.View.js
index 188ac6d..891bd8a 100644
--- a/modules/ve/ce/ve.ce.View.js
+++ b/modules/ve/ce/ve.ce.View.js
@@ -26,13 +26,7 @@
 
// Initialization
this.$.data( 'view', this );
-   if ( this.constructor.static.renderHtmlAttributes ) {
-   ve.setDomAttributes(
-   this.$[0],
-   this.model.getAttributes( 'html/0/' ),
-   this.constructor.static.domAttributeWhitelist
-   );
-   }
+   this.renderAttributes( this.model.getAttributes() );
 };
 
 /* Inheritance */
@@ -116,3 +110,21 @@
this.live = live;
this.emit( 'live' );
 };
+
+ve.ce.View.prototype.renderAttributes = function ( attributes ) {
+   var key, parsed,
+   whitelist = this.constructor.static.domAttributeWhitelist;
+   if ( !this.constructor.static.renderHtmlAttributes ) {
+   return;
+   }
+   for ( key in attributes ) {
+   parsed = ve.dm.Converter.parseHtmlAttribute( key, this.$ );
+   if ( parsed  whitelist.indexOf( parsed.attribute ) !== -1 ) {
+   if ( attributes[key] === undefined ) {
+   parsed.domElement.removeAttribute( 
parsed.attribute );
+   } else {
+   parsed.domElement.setAttribute( 
parsed.attribute, attributes[key] );
+   }
+   }
+   }
+};
\ No newline at end of file
diff --git a/modules/ve/dm/ve.dm.Converter.js b/modules/ve/dm/ve.dm.Converter.js
index b7258dd..8f4ebaa 100644
--- a/modules/ve/dm/ve.dm.Converter.js
+++ b/modules/ve/dm/ve.dm.Converter.js
@@ -97,6 +97,31 @@
}
 };
 
+/**
+ * Parse a linear model attribute name of the form html/i-j-k/attrName.
+ *
+ * @param {string} attribute Name of a linear model attribute
+ * @param {HTMLElement[]|jQuery} domElements DOM elements array that the 
attribute indexes into
+ * @returns {Object|null} Object with domElement and attribute keys, or null
+ */
+ve.dm.Converter.parseHtmlAttribute = function ( attribute, domElements ) {
+   var i, ilen, indexes, child,
+   /*jshint regexp:false */
+   matches = attribute.match( /^html\/((?:\d+\-)*\d)\/(.*)$/ );
+   if ( !matches ) {
+   return null;
+   }
+   indexes = matches[1].split( '-' ); // matches[1] like '1-2-3'
+   child = domElements[indexes[0]];
+   for ( i = 1, ilen = indexes.length; i  ilen; i++ ) {
+   child = child  child.childNodes[indexes[i]];
+   }
+   if ( !child ) {
+   return null;
+   }
+   return { 'domElement': child, 'attribute': matches[2] };
+};
+
 /* Methods */
 
 /**
@@ -198,7 +223,7 @@
  * @returns {HTMLElement|boolean} DOM element, or false if the element cannot 
be converted
  */
 ve.dm.Converter.prototype.getDomElementsFromDataElement = function ( 
dataElements, doc ) {
-   var domElements, dataElementAttributes, key, matches, indexes, i, ilen, 
child,
+   var domElements, dataElementAttributes, key, parsed,
dataElement = ve.isArray( dataElements ) ? dataElements[0] : 
dataElements,
nodeClass = this.modelRegistry.lookup( dataElement.type );
if ( !nodeClass ) {
@@ -214,18 +239,9 @@
 

[MediaWiki-commits] [Gerrit] Also render attributes of the form html/i-j/attrName in CE - change (mediawiki...VisualEditor)

2013-05-08 Thread jenkins-bot (Code Review)
jenkins-bot has submitted this change and it was merged.

Change subject: Also render attributes of the form html/i-j/attrName in CE
..


Also render attributes of the form html/i-j/attrName in CE

Factored the parsing of html/* attributes out into a static function.
Factored attribute (re)rendering out into ce.View, attribute updates
are much simpler now.

Change-Id: I4caa6d5e1e2c21c28ddff61c3c864e47f66cc6b2
---
M modules/ve/ce/ve.ce.Node.js
M modules/ve/ce/ve.ce.View.js
M modules/ve/dm/ve.dm.Converter.js
3 files changed, 49 insertions(+), 33 deletions(-)

Approvals:
  Trevor Parscal: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/modules/ve/ce/ve.ce.Node.js b/modules/ve/ce/ve.ce.Node.js
index 16cb9e7..2a5bf96 100644
--- a/modules/ve/ce/ve.ce.Node.js
+++ b/modules/ve/ce/ve.ce.Node.js
@@ -63,19 +63,7 @@
  * @param {string} to New value
  */
 ve.ce.Node.prototype.onAttributeChange = function ( key, from, to ) {
-   var htmlKey = key.substr( 7 ).toLowerCase();
-   if (
-   this.constructor.static.renderHtmlAttributes 
-   key.indexOf( 'html/0/' ) === 0 
-   htmlKey.length 
-   this.constructor.static.domAttributeWhitelist.indexOf( htmlKey 
) !== -1
-   ) {
-   if ( to === undefined ) {
-   this.$.removeAttr( htmlKey );
-   } else {
-   this.$.attr( htmlKey, to );
-   }
-   }
+   this.renderAttributes( { key: to } );
 };
 
 /**
diff --git a/modules/ve/ce/ve.ce.View.js b/modules/ve/ce/ve.ce.View.js
index 188ac6d..891bd8a 100644
--- a/modules/ve/ce/ve.ce.View.js
+++ b/modules/ve/ce/ve.ce.View.js
@@ -26,13 +26,7 @@
 
// Initialization
this.$.data( 'view', this );
-   if ( this.constructor.static.renderHtmlAttributes ) {
-   ve.setDomAttributes(
-   this.$[0],
-   this.model.getAttributes( 'html/0/' ),
-   this.constructor.static.domAttributeWhitelist
-   );
-   }
+   this.renderAttributes( this.model.getAttributes() );
 };
 
 /* Inheritance */
@@ -116,3 +110,21 @@
this.live = live;
this.emit( 'live' );
 };
+
+ve.ce.View.prototype.renderAttributes = function ( attributes ) {
+   var key, parsed,
+   whitelist = this.constructor.static.domAttributeWhitelist;
+   if ( !this.constructor.static.renderHtmlAttributes ) {
+   return;
+   }
+   for ( key in attributes ) {
+   parsed = ve.dm.Converter.parseHtmlAttribute( key, this.$ );
+   if ( parsed  whitelist.indexOf( parsed.attribute ) !== -1 ) {
+   if ( attributes[key] === undefined ) {
+   parsed.domElement.removeAttribute( 
parsed.attribute );
+   } else {
+   parsed.domElement.setAttribute( 
parsed.attribute, attributes[key] );
+   }
+   }
+   }
+};
\ No newline at end of file
diff --git a/modules/ve/dm/ve.dm.Converter.js b/modules/ve/dm/ve.dm.Converter.js
index ff1af08..dd16935 100644
--- a/modules/ve/dm/ve.dm.Converter.js
+++ b/modules/ve/dm/ve.dm.Converter.js
@@ -100,6 +100,31 @@
}
 };
 
+/**
+ * Parse a linear model attribute name of the form html/i-j-k/attrName.
+ *
+ * @param {string} attribute Name of a linear model attribute
+ * @param {HTMLElement[]|jQuery} domElements DOM elements array that the 
attribute indexes into
+ * @returns {Object|null} Object with domElement and attribute keys, or null
+ */
+ve.dm.Converter.parseHtmlAttribute = function ( attribute, domElements ) {
+   var i, ilen, indexes, child,
+   /*jshint regexp:false */
+   matches = attribute.match( /^html\/((?:\d+\-)*\d)\/(.*)$/ );
+   if ( !matches ) {
+   return null;
+   }
+   indexes = matches[1].split( '-' ); // matches[1] like '1-2-3'
+   child = domElements[indexes[0]];
+   for ( i = 1, ilen = indexes.length; i  ilen; i++ ) {
+   child = child  child.childNodes[indexes[i]];
+   }
+   if ( !child ) {
+   return null;
+   }
+   return { 'domElement': child, 'attribute': matches[2] };
+};
+
 /* Methods */
 
 /**
@@ -201,7 +226,7 @@
  * @returns {HTMLElement|boolean} DOM element, or false if the element cannot 
be converted
  */
 ve.dm.Converter.prototype.getDomElementsFromDataElement = function ( 
dataElements, doc ) {
-   var domElements, dataElementAttributes, key, matches, indexes, i, ilen, 
child,
+   var domElements, dataElementAttributes, key, parsed,
dataElement = ve.isArray( dataElements ) ? dataElements[0] : 
dataElements,
nodeClass = this.modelRegistry.lookup( dataElement.type );
 
@@ -218,18 +243,9 @@
dataElementAttributes = dataElement.attributes;
if (