From 05828cc3f16ee10bce1cf232850c4c7b499c208b Mon Sep 17 00:00:00 2001 From: Catrope Date: Tue, 7 May 2013 21:25:55 -0700 Subject: [PATCH] Preserve HTML attributes recursively For nodes that handle their own children (as well as leaf nodes and meta items), store the first child's attributes in html/0-0/*, the second child's attributes in html/0-1/*, the second element's third child's fourth child's attributes in html/1-2-3/* , etc. This obsoletes the ad-hoc code that basically did the same thing in MWInlineImageNode. Change-Id: If5abd2d5d9c361b359617ff4b0f3d6ba4c9b0142 --- .../ve/dm/nodes/ve.dm.MWInlineImageNode.js | 27 +---- modules/ve/dm/ve.dm.Converter.js | 107 ++++++++++++------ modules/ve/dm/ve.dm.Model.js | 11 +- modules/ve/test/dm/ve.dm.example.js | 16 ++- 4 files changed, 97 insertions(+), 64 deletions(-) diff --git a/modules/ve/dm/nodes/ve.dm.MWInlineImageNode.js b/modules/ve/dm/nodes/ve.dm.MWInlineImageNode.js index 873e8f3845..69db15a108 100644 --- a/modules/ve/dm/nodes/ve.dm.MWInlineImageNode.js +++ b/modules/ve/dm/nodes/ve.dm.MWInlineImageNode.js @@ -34,22 +34,11 @@ ve.dm.MWInlineImageNode.static.generatedContent = true; ve.dm.MWInlineImageNode.static.matchRdfaTypes = [ 'mw:Image' ]; ve.dm.MWInlineImageNode.static.toDataElement = function ( domElements ) { - var i, j, childNode, children = Array.prototype.slice.call( domElements[0].children, 0 ), + var children = Array.prototype.slice.call( domElements[0].children, 0 ), parentResult = ve.dm.ImageNode.static.toDataElement.apply( this, [ children ].concat( Array.prototype.slice.call( arguments, 1 ) ) - ), - dataElement = ve.copyObject( parentResult ); - - // Preserve the child nodes' attributes in html/0-i/foo - for ( i = 0; i < domElements[0].childNodes.length; i++ ) { - childNode = domElements[0].childNodes[i]; - for ( j = 0; j < childNode.attributes.length; j++ ) { - dataElement.attributes['html/0-' + i + '/' + childNode.attributes[j].name] = - childNode.attributes[j].value; - } - } - - return ve.extendObject( true, dataElement, { + ); + return ve.extendObject( true, parentResult, { 'type': 'MWinlineimage', 'attributes': { 'isLinked': domElements[0].nodeName.toLowerCase() === 'a' @@ -58,17 +47,9 @@ ve.dm.MWInlineImageNode.static.toDataElement = function ( domElements ) { }; ve.dm.MWInlineImageNode.static.toDomElements = function ( dataElement, doc ) { - var k, wrapper = doc.createElement( dataElement.attributes.isLinked ? 'a' : 'span' ), + var wrapper = doc.createElement( dataElement.attributes.isLinked ? 'a' : 'span' ), imageDomElement = ve.dm.ImageNode.static.toDomElements.apply( this, arguments )[0]; - wrapper.appendChild( imageDomElement ); - // Restore attributes from html/0-0/* - for ( k in dataElement.attributes ) { - if ( k.indexOf( 'html/0-0/' ) === 0 ) { - imageDomElement.setAttribute( k.substr( 9 ), dataElement.attributes[k] ); - } - } - return [ wrapper ]; }; diff --git a/modules/ve/dm/ve.dm.Converter.js b/modules/ve/dm/ve.dm.Converter.js index bcb3b9137a..ff1af085a0 100644 --- a/modules/ve/dm/ve.dm.Converter.js +++ b/modules/ve/dm/ve.dm.Converter.js @@ -40,7 +40,9 @@ ve.dm.Converter = function VeDmConverter( modelRegistry, nodeFactory, annotation * @returns {Array} Linear model data, one element per character */ ve.dm.Converter.getDataContentFromText = function ( text, annotations ) { - var i, len, characters = text.split( '' ); + var i, len, + characters = text.split( '' ); + if ( !annotations || annotations.isEmpty() ) { return characters; } @@ -67,6 +69,7 @@ ve.dm.Converter.getDataContentFromText = function ( text, annotations ) { */ ve.dm.Converter.openAndCloseAnnotations = function ( currentSet, targetSet, open, close ) { var i, len, annotation, startClosingAt; + // Close annotations as needed // Go through annotationStack from bottom to top (low to high), // and find the first annotation that's not in annotations. @@ -198,9 +201,10 @@ ve.dm.Converter.prototype.canCloseWrapper = function () { * @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, + var domElements, dataElementAttributes, key, matches, indexes, i, ilen, child, dataElement = ve.isArray( dataElements ) ? dataElements[0] : dataElements, nodeClass = this.modelRegistry.lookup( dataElement.type ); + if ( !nodeClass ) { throw new Error( 'Attempting to convert unknown data element type ' + dataElement.type ); } @@ -214,12 +218,17 @@ ve.dm.Converter.prototype.getDomElementsFromDataElement = function ( dataElement dataElementAttributes = dataElement.attributes; if ( dataElementAttributes ) { for ( key in dataElementAttributes ) { - // Only include 'html/i/*' attributes and strip the 'html/i/' from the beginning of the name + // Only include 'html/*' attributes and strip the prefix /*jshint regexp:false */ - matches = key.match( /^html\/(\d+)\/(.*)$/ ); + matches = key.match( /^html\/((?:\d+\-)*\d)\/(.*)$/ ); if ( matches ) { - if ( domElements[matches[1]] && !domElements[matches[1]].hasAttribute( matches[2] ) ) { - domElements[matches[1]].setAttribute( matches[2], dataElementAttributes[key] ); + 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 && !child.hasAttribute( matches[2] ) ) { + child.setAttribute( matches[2], dataElementAttributes[key] ); } } } @@ -234,35 +243,51 @@ ve.dm.Converter.prototype.getDomElementsFromDataElement = function ( dataElement * @returns {Object|Array|null} Data element or array of linear model data, or null to alienate */ ve.dm.Converter.prototype.createDataElements = function ( modelClass, domElements ) { - var i, j, dataElements, dataElementAttributes, domElementAttributes, - domElementAttribute; - dataElements = modelClass.static.toDataElement( domElements, this ); + var dataElements = modelClass.static.toDataElement( domElements, this ); + if ( !dataElements ) { return null; } if ( !ve.isArray( dataElements ) ) { dataElements = [ dataElements ]; } - 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 || {}; - // Store attributes and prepend 'html/i/' to each attribute name - for ( j = 0; j < domElementAttributes.length; j++ ) { - domElementAttribute = domElementAttributes[j]; - if ( - ve.dm.Model.matchesAttributeSpec( domElementAttribute.name, - modelClass.static.storeHtmlAttributes ) - ) { - dataElementAttributes['html/' + i + '/' + domElementAttribute.name] = - domElementAttribute.value; - } + return dataElements; +}; + +/** + * Set the 'html/' attributes for HTML attribute preservation + * @param {Object} dataElement Linear model element to set the attributes on + * @param {HTMLElement[]} domElements DOM elements the linear model element was generated from + * @param {boolean} [deep=false] If true, descend into children and store their attributes too + */ +ve.dm.Converter.prototype.setHtmlAttributes = function ( dataElement, domElements, deep ) { + var i, len, + modelClass = this.modelRegistry.lookup( dataElement.type ), + spec = modelClass.static.storeHtmlAttributes; + + function processChild( child, prefix ) { + var i, len, + // text nodes don't have attributes + childAttributes = child.attributes || []; + + for ( i = 0, len = childAttributes.length; i < len; i++ ) { + if ( ve.dm.Model.matchesAttributeSpec( childAttributes[i].name, spec ) ) { + if ( !dataElement.attributes ) { + dataElement.attributes = {}; } + dataElement.attributes[prefix + '/' + childAttributes[i].name] = childAttributes[i].value; + } + } + if ( deep ) { + for ( i = 0, len = child.childNodes.length; i < len; i++ ) { + processChild( child.childNodes[i], prefix + '-' + i ); } } } - return dataElements; + + for ( i = 0, len = domElements.length; i < len; i++ ) { + processChild( domElements[i], 'html/' + i ); + } }; /** @@ -275,6 +300,7 @@ ve.dm.Converter.prototype.createDataElements = function ( modelClass, domElement ve.dm.Converter.prototype.getDomElementFromDataAnnotation = function ( dataAnnotation, doc ) { var htmlData = dataAnnotation.toHTML(), domElement = doc.createElement( htmlData.tag ); + ve.setDomAttributes( domElement, htmlData.attributes ); return domElement; }; @@ -288,6 +314,7 @@ ve.dm.Converter.prototype.getDomElementFromDataAnnotation = function ( dataAnnot */ ve.dm.Converter.prototype.getDataFromDom = function ( doc, store, internalList ) { var linearData, refData; + // Set up the converter state this.doc = doc; this.store = store; @@ -353,7 +380,9 @@ ve.dm.Converter.prototype.getDataFromDomRecursion = function ( domElement, wrapp } } function outputWrappedMetaItems( whitespaceTreatment ) { - var i, len, prev = wrappingParagraph; + var i, len, + prev = wrappingParagraph; + for ( i = 0, len = wrappedMetaItems.length; i < len; i++ ) { if ( wrappedMetaItems[i].type && wrappedMetaItems[i].type.charAt( 0 ) !== '/' ) { if ( wrappedMetaItems[i].internal && wrappedMetaItems[i].internal.whitespace ) { @@ -401,7 +430,10 @@ ve.dm.Converter.prototype.getDataFromDomRecursion = function ( domElement, wrapp context.expectingContent = context.originallyExpectingContent; } function getAboutGroup( el ) { - var textNodes = [], aboutGroup = [ el ], elAbout, node; + var elAbout, node, + textNodes = [], + aboutGroup = [ el ]; + if ( !el.getAttribute || el.getAttribute( 'about' ) === null ) { return aboutGroup; } @@ -433,7 +465,8 @@ ve.dm.Converter.prototype.getDataFromDomRecursion = function ( domElement, wrapp wrappedWhitespaceIndex, wrappedMetaItems = [], context = {}, - prevContext = this.contextStack.length ? this.contextStack[this.contextStack.length - 1] : null; + prevContext = this.contextStack.length ? + this.contextStack[this.contextStack.length - 1] : null; context.annotations = annotationSet || ( prevContext ? prevContext.annotations.clone() : new ve.dm.AnnotationSet( this.store ) @@ -461,13 +494,13 @@ ve.dm.Converter.prototype.getDataFromDomRecursion = function ( domElement, wrapp modelName = this.modelRegistry.matchElement( childDomElement, aboutGroup.length > 1 ); modelClass = this.modelRegistry.lookup( modelName ) || ve.dm.AlienNode; if ( modelClass.prototype instanceof ve.dm.Annotation ) { - childDataElements = this.createDataElements( modelClass, [ childDomElement ] ); + childDomElements = [ childDomElement ]; } else { // Node or meta item childDomElements = modelClass.static.enableAboutGrouping ? aboutGroup : [ childDomElement ]; - childDataElements = this.createDataElements( modelClass, childDomElements ); } + childDataElements = this.createDataElements( modelClass, childDomElements ); if ( !childDataElements ) { // Alienate @@ -482,6 +515,7 @@ ve.dm.Converter.prototype.getDataFromDomRecursion = function ( domElement, wrapp // Now take the appropriate action based on that if ( modelClass.prototype instanceof ve.dm.Annotation ) { + this.setHtmlAttributes( childDataElements[0], childDomElements, false ); annotation = this.annotationFactory.create( modelName, childDataElements[0] ); // Start wrapping if needed if ( !context.inWrapper && !context.expectingContent ) { @@ -499,6 +533,7 @@ ve.dm.Converter.prototype.getDataFromDomRecursion = function ( domElement, wrapp } else { // Node or meta item if ( modelClass.prototype instanceof ve.dm.MetaItem ) { + this.setHtmlAttributes( childDataElements[0], childDomElements, true ); // No additional processing needed // Write to data and continue if ( childDataElements.length === 1 ) { @@ -552,6 +587,7 @@ ve.dm.Converter.prototype.getDataFromDomRecursion = function ( domElement, wrapp !this.nodeFactory.doesNodeHandleOwnChildren( childDataElements[0].type ) ) { // Recursion + this.setHtmlAttributes( childDataElements[0], childDomElements, false ); // Opening and closing elements are added by the recursion too data = data.concat( this.getDataFromDomRecursion( childDomElement, childDataElements[0], @@ -562,6 +598,7 @@ ve.dm.Converter.prototype.getDataFromDomRecursion = function ( domElement, wrapp if ( childDataElements.length === 1 ) { childDataElements.push( { 'type': '/' + childDataElements[0].type } ); } + this.setHtmlAttributes( childDataElements[0], childDomElements, true ); // Write childDataElements directly data = data.concat( childDataElements ); } @@ -781,6 +818,7 @@ ve.dm.Converter.prototype.getDataFromDomRecursion = function ( domElement, wrapp */ ve.dm.Converter.prototype.getDomFromData = function ( documentData, store, internalList ) { var doc = ve.createDocumentFromHTML( '' ); + // Set up the converter state this.documentData = documentData; this.store = store; @@ -805,9 +843,8 @@ ve.dm.Converter.prototype.getDomFromData = function ( documentData, store, inter */ ve.dm.Converter.prototype.getDomSubtreeFromData = function ( data, container ) { var text, i, j, annotations, annotationElement, dataElement, dataElementOrSlice, - childDomElements, pre, ours, theirs, parentDomElement, lastChild, - isContentNode, sibling, previousSiblings, doUnwrap, - textNode, type, + childDomElements, pre, ours, theirs, parentDomElement, lastChild, isContentNode, sibling, + previousSiblings, doUnwrap, textNode, type, canContainContentStack = [], conv = this, doc = container.ownerDocument, @@ -839,7 +876,9 @@ ve.dm.Converter.prototype.getDomSubtreeFromData = function ( data, container ) { } function getDataElementOrSlice() { - var dataSlice, j, depth, handlesOwn = false; + var dataSlice, j, depth, + handlesOwn = false; + try { handlesOwn = ve.dm.nodeFactory.doesNodeHandleOwnChildren( data[i].type ); } catch ( e ) {} diff --git a/modules/ve/dm/ve.dm.Model.js b/modules/ve/dm/ve.dm.Model.js index dc86e1c5d9..df7cbfcdf5 100644 --- a/modules/ve/dm/ve.dm.Model.js +++ b/modules/ve/dm/ve.dm.Model.js @@ -168,10 +168,15 @@ ve.dm.Model.static.enableAboutGrouping = false; * 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(). + * domElements array, and attrName is the name of the attribute. If the DOM elements have + * children and the converter does not descend into them, the children's attributes will be stored + * in html/i-j/attrName (for domElements[i].children[j]), the grandchildren's attributes in + * html/i-j-k/attrName, etc. * - * The value of this attribute can be one of the following: + * When converting back to DOM, these HTML attributes will be restored except for attributes that + * were already set by toDomElements(). + * + * The value of this property can be one of the following: * - true, to preserve all attributes (default) * - false, to preserve none * - a string, to preserve only that attribute diff --git a/modules/ve/test/dm/ve.dm.example.js b/modules/ve/test/dm/ve.dm.example.js index 4bcb1a968b..1dc2f20a03 100644 --- a/modules/ve/test/dm/ve.dm.example.js +++ b/modules/ve/test/dm/ve.dm.example.js @@ -1066,7 +1066,9 @@ ve.dm.example.domToDataCases = { 'html/0/class': 'reference', 'html/0/data-parsoid': '{"src":"Bar"}', 'html/0/id': 'cite_ref-bar-1-0', - 'html/0/typeof': 'mw:Object/Ext/Ref' + 'html/0/typeof': 'mw:Object/Ext/Ref', + 'html/0-0/href': '#cite_note-bar-1', + 'html/0-0/data-parsoid': '{}' } }, { 'type': '/MWreference' }, @@ -1081,7 +1083,9 @@ ve.dm.example.domToDataCases = { 'html/0/class': 'reference', 'html/0/data-parsoid': '{"src":"Quux"}', 'html/0/id': 'cite_ref-quux-2-0', - 'html/0/typeof': 'mw:Object/Ext/Ref' + 'html/0/typeof': 'mw:Object/Ext/Ref', + 'html/0-0/href': '#cite_note-quux-2', + 'html/0-0/data-parsoid': '{}' } }, { 'type': '/MWreference' }, @@ -1096,7 +1100,9 @@ ve.dm.example.domToDataCases = { 'html/0/class': 'reference', 'html/0/data-parsoid': '{"src":""}', 'html/0/id': 'cite_ref-bar-1-1', - 'html/0/typeof': 'mw:Object/Ext/Ref' + 'html/0/typeof': 'mw:Object/Ext/Ref', + 'html/0-0/href': '#cite_note-bar-1', + 'html/0-0/data-parsoid': '{}' } }, { 'type': '/MWreference' }, @@ -1111,7 +1117,9 @@ ve.dm.example.domToDataCases = { 'html/0/class': 'reference', 'html/0/data-parsoid': '{"src":"No name"}', 'html/0/id': 'cite_ref-3-0', - 'html/0/typeof': 'mw:Object/Ext/Ref' + 'html/0/typeof': 'mw:Object/Ext/Ref', + 'html/0-0/href': '#cite_note-3', + 'html/0-0/data-parsoid': '{}' } }, { 'type': '/MWreference' },