From 2bd6f8576aa2d0813b4ad4045574bdd684240d25 Mon Sep 17 00:00:00 2001 From: Ed Sanders Date: Sat, 20 Apr 2013 17:08:23 +0100 Subject: [PATCH] MWTemplateNode should serialise original HTML if unchanged To help the selective serialiser we can return the original HTML for generated content if it is unmodified. As the output of toDomElements now depends on changes to the dataElement we now have a 'modify' function in the some test cases. We also now have 'storeItems' to assert that the index-value store is correctly populated and for loading values back into the store for toDomElements tests. Also make 'mw' an attribute and remove 'about' property. Bug: 47394 Change-Id: I2bbb5d2d6a90c4eb87fa129671112c92a9b931e7 --- modules/ve/dm/nodes/ve.dm.MWTemplateNode.js | 35 +++-- modules/ve/dm/ve.dm.Converter.js | 13 +- modules/ve/test/dm/ve.dm.Converter.test.js | 32 +++- modules/ve/test/dm/ve.dm.example.js | 156 ++++++++++++++------ 4 files changed, 169 insertions(+), 67 deletions(-) diff --git a/modules/ve/dm/nodes/ve.dm.MWTemplateNode.js b/modules/ve/dm/nodes/ve.dm.MWTemplateNode.js index 0da0b133b0..823ebddbfc 100644 --- a/modules/ve/dm/nodes/ve.dm.MWTemplateNode.js +++ b/modules/ve/dm/nodes/ve.dm.MWTemplateNode.js @@ -35,34 +35,45 @@ ve.dm.MWTemplateNode.static.matchRdfaTypes = [ 'mw:Object/Template' ]; ve.dm.MWTemplateNode.static.getHashObject = function ( dataElement ) { return { type: dataElement.type, - mw: dataElement.mw + mw: dataElement.attributes.mw }; }; ve.dm.MWTemplateNode.static.toDataElement = function ( domElements, converter ) { var dataElement, - about = domElements[0].getAttribute( 'about' ), mw = JSON.parse( domElements[0].getAttribute( 'data-mw' ) ), isInline = this.isHybridInline( domElements, converter ), type = isInline ? 'MWtemplateInline' : 'MWtemplateBlock'; dataElement = { 'type': type, - 'mw': mw, - 'about': about + 'attributes': { + 'mw': mw, + 'mwOriginal': ve.copyObject( mw ) + } }; this.storeHtml( dataElement, domElements, converter.getStore() ); return dataElement; }; -ve.dm.MWTemplateNode.static.toDomElements = function ( dataElement, doc ) { - var span = doc.createElement( 'span' ); - // All we need to send back to Parsoid is the original template marker, - // with a reconstructed data-mw property. - span.setAttribute( 'about', dataElement.about ); - span.setAttribute( 'typeof', 'mw:Object/Template' ); - span.setAttribute( 'data-mw', JSON.stringify( dataElement.mw ) ); - return [ span ]; +ve.dm.MWTemplateNode.static.toDomElements = function ( dataElement, doc, converter ) { + var wrapper, span, index, html; + if ( ve.compareObjects( dataElement.attributes.mw, dataElement.attributes.mwOriginal ) ) { + // If the template is unchanged just send back the original html so selser can skip over it + index = converter.getStore().indexOfHash( ve.getHash( this.getHashObject( dataElement ) ) ); + html = converter.getStore().value( index ); + wrapper = doc.createElement( 'div' ); + $( wrapper ).html( html ); + // Convert wrapper.children to an array + return Array.prototype.slice.call( wrapper.childNodes, 0 ); + } else { + span = doc.createElement( 'span' ); + // All we need to send back to Parsoid is the original template marker, + // with a reconstructed data-mw property. + span.setAttribute( 'typeof', 'mw:Object/Template' ); + span.setAttribute( 'data-mw', JSON.stringify( dataElement.attributes.mw ) ); + return [ span ]; + } }; /* Concrete subclasses */ diff --git a/modules/ve/dm/ve.dm.Converter.js b/modules/ve/dm/ve.dm.Converter.js index 66191bed5a..e5e1701d81 100644 --- a/modules/ve/dm/ve.dm.Converter.js +++ b/modules/ve/dm/ve.dm.Converter.js @@ -770,26 +770,27 @@ ve.dm.Converter.prototype.getDataFromDomRecursion = function ( domElement, wrapp */ ve.dm.Converter.prototype.getDomFromData = function ( store, data ) { var doc = ve.createDocumentFromHTML( '' ); - this.getDomSubtreeFromData( store, data, doc.body ); + this.store = store; + this.getDomSubtreeFromData( data, doc.body ); + this.store = null; return doc; }; /** * Convert linear model data to an HTML DOM subtree and add it to a container element. * - * @param {ve.dm.IndexValueStore} store Index-value store * @param {Array} data Linear model data * @param {HTMLElement} container DOM element to add the generated elements to. Should be empty. * @throws Unbalanced data: looking for closing /type */ -ve.dm.Converter.prototype.getDomSubtreeFromData = function ( store, data, container ) { +ve.dm.Converter.prototype.getDomSubtreeFromData = function ( data, container ) { var text, i, j, k, annotations, annotationElement, dataElement, dataElementOrSlice, childDomElements, pre, ours, theirs, parentDomElement, lastChild, isContentNode, changed, parentChanged, sibling, previousSiblings, doUnwrap, textNode, conv = this, doc = container.ownerDocument, domElement = container, - annotationStack = new ve.dm.AnnotationSet( store ); + annotationStack = new ve.dm.AnnotationSet( this.store ); function openAnnotation( annotation ) { // Add text if needed @@ -873,7 +874,7 @@ ve.dm.Converter.prototype.getDomSubtreeFromData = function ( store, data, contai ) ) { annotations = new ve.dm.AnnotationSet( - store, data[i].annotations || data[i][1] + this.store, data[i].annotations || data[i][1] ); ve.dm.Converter.openAndCloseAnnotations( annotationStack, annotations, openAnnotation, closeAnnotation @@ -917,7 +918,7 @@ ve.dm.Converter.prototype.getDomSubtreeFromData = function ( store, data, contai domElement = domElement.parentNode; } // Clear annotationStack - annotationStack = new ve.dm.AnnotationSet( store ); + annotationStack = new ve.dm.AnnotationSet( this.store ); } else if ( data[i].type !== undefined ) { dataElement = data[i]; // Element diff --git a/modules/ve/test/dm/ve.dm.Converter.test.js b/modules/ve/test/dm/ve.dm.Converter.test.js index 687a104f85..bec9131a63 100644 --- a/modules/ve/test/dm/ve.dm.Converter.test.js +++ b/modules/ve/test/dm/ve.dm.Converter.test.js @@ -40,8 +40,7 @@ QUnit.test( 'getDomElementsFromDataElement', 20, function ( assert ) { } ); QUnit.test( 'getDataFromDom', function ( assert ) { - var msg, n = 0, - store = new ve.dm.IndexValueStore(), + var msg, store, i, length, hash, n = 0, cases = ve.copyObject( ve.dm.example.domToDataCases ); // TODO: this is a hack to make normal heading/preformatted @@ -52,25 +51,39 @@ QUnit.test( 'getDataFromDom', function ( assert ) { for ( msg in cases ) { if ( cases[msg].html !== null ) { n++; + if ( cases[msg].storeItems ) { + n += cases[msg].storeItems.length; + } } } QUnit.expect( n ); for ( msg in cases ) { if ( cases[msg].html !== null ) { + store = new ve.dm.IndexValueStore(); ve.dm.example.preprocessAnnotations( cases[msg].data, store ); assert.deepEqual( ve.dm.converter.getDataFromDom( store, ve.createDocumentFromHTML( cases[msg].html ) ).getData(), cases[msg].data, msg ); + // check storeItems have been added to store + if ( cases[msg].storeItems ) { + for ( i = 0, length = cases[msg].storeItems.length; i < length; i++ ) { + hash = cases[msg].storeItems[i].hash || ve.getHash( cases[msg].storeItems[i].value ); + assert.deepEqual( + store.value( store.indexOfHash( hash ) ), + cases[msg].storeItems[i].value, + msg + ': store item ' + i + ' found' + ); + } + } } } } ); QUnit.test( 'getDomFromData', function ( assert ) { - var msg, originalData, n = 0, - store = new ve.dm.IndexValueStore(), + var msg, originalData, store, i, length, n = 0, cases = ve.copyObject( ve.dm.example.domToDataCases ); for ( msg in cases ) { @@ -79,6 +92,17 @@ QUnit.test( 'getDomFromData', function ( assert ) { QUnit.expect( 2*n ); for ( msg in cases ) { + store = new ve.dm.IndexValueStore(); + // load storeItems into store + if ( cases[msg].storeItems ) { + for ( i = 0, length = cases[msg].storeItems.length; i < length; i++ ) { + store.index( cases[msg].storeItems[i].value, cases[msg].storeItems[i].hash ); + } + } + // functions won't be copied by ve.copyObject + if( ve.dm.example.domToDataCases[msg].modify ) { + ve.dm.example.domToDataCases[msg].modify( cases[msg].data ); + } ve.dm.example.preprocessAnnotations( cases[msg].data, store ); originalData = ve.copyArray( cases[msg].data ); assert.equalDomElement( diff --git a/modules/ve/test/dm/ve.dm.example.js b/modules/ve/test/dm/ve.dm.example.js index fbf9baaa78..cd3e1cb3e6 100644 --- a/modules/ve/test/dm/ve.dm.example.js +++ b/modules/ve/test/dm/ve.dm.example.js @@ -715,11 +715,75 @@ ve.dm.example.conversions = { }; ve.dm.example.MWImageHtml = 'Wiki.png'; -ve.dm.example.MWBlockTemplateSpan = ''; -ve.dm.example.MWBlockTemplateContent = '

Hello, world!

'; -ve.dm.example.MWInlineTemplateOpen = ''; -ve.dm.example.MWInlineTemplateContent = '$1,234.00'; -ve.dm.example.MWInlineTemplateClose = ''; +ve.dm.example.MWTemplate = { + 'blockSpan': '', + 'blockSpanModified': '', + 'blockContent': '

Hello, world!

', + 'blockData': { + 'type': 'MWtemplateBlock', + 'attributes': { + 'mw': { + 'id': 'mwt1', + 'target': { 'wt' : 'Test' }, + 'params': { + '1': { 'wt': 'Hello, world!' } + } + }, + 'mwOriginal': { + 'id': 'mwt1', + 'target': { 'wt' : 'Test' }, + 'params': { + '1': { 'wt': 'Hello, world!' } + } + }, + 'html/0/about': '#mwt1', + 'html/0/data-mw': '{\"id\":\"mwt1\",\"target\":{\"wt\":\"Test\"},\"params\":{\"1\":{\"wt\":\"Hello, world!\"}}}', + 'html/0/data-parsoid': '{\"tsr\":[18,40],\"src\":\"{{Test|Hello, world!}}\",\"dsr\":[18,40,null,null]}', + 'html/0/typeof': 'mw:Object/Template', + 'html/1/about': '#mwt1', + 'html/1/data-parsoid': '{}' + }, + }, + 'inlineOpen': '', + 'inlineOpenModified': '', + 'inlineContent': '$1,234.00', + 'inlineClose': '', + 'inlineData': { + 'type': 'MWtemplateInline', + 'attributes': { + 'mw': { + 'id': 'mwt1', + 'target': { 'wt' : 'Inline' }, + 'params': { + '1': { 'wt': '1,234' } + } + }, + 'mwOriginal': { + 'id': 'mwt1', + 'target': { 'wt' : 'Inline' }, + 'params': { + '1': { 'wt': '1,234' } + } + }, + 'html/0/about': '#mwt1', + 'html/0/data-mw': '{\"id\":\"mwt1\",\"target\":{\"wt\":\"Inline\"},\"params\":{\"1\":{\"wt\":\"1,234\"}}}', + 'html/0/data-parsoid': '{\"tsr\":[18,34],\"src\":\"{{Inline|1,234}}\",\"dsr\":[18,34,null,null]}', + 'html/0/typeof': 'mw:Object/Template' + }, + } +}; + +ve.dm.example.MWTemplate.blockParamsHash = ve.getHash( ve.dm.MWTemplateNode.static.getHashObject( ve.dm.example.MWTemplate.blockData ) ); +ve.dm.example.MWTemplate.blockStoreItems = { + 'hash': ve.dm.example.MWTemplate.blockParamsHash, + 'value': ve.dm.example.MWTemplate.blockSpan + ve.dm.example.MWTemplate.blockContent +}; + +ve.dm.example.MWTemplate.inlineParamsHash = ve.getHash( ve.dm.MWTemplateNode.static.getHashObject( ve.dm.example.MWTemplate.inlineData ) ); +ve.dm.example.MWTemplate.inlineStoreItems = { + 'hash': ve.dm.example.MWTemplate.inlineParamsHash, + 'value': ve.dm.example.MWTemplate.inlineOpen + ve.dm.example.MWTemplate.inlineContent + ve.dm.example.MWTemplate.inlineClose +}; ve.dm.example.domToDataCases = { 'paragraph with plain text': { @@ -781,56 +845,58 @@ ve.dm.example.domToDataCases = { ] }, 'mw:Template (block level)': { - 'html': '' + ve.dm.example.MWBlockTemplateSpan + ve.dm.example.MWBlockTemplateContent + '', + 'html': '' + ve.dm.example.MWTemplate.blockSpan + ve.dm.example.MWTemplate.blockContent + '', 'data': [ - { - 'type': 'MWtemplateBlock', - 'mw': { - 'id': 'mwt1', - 'target': { 'wt' : 'Test' }, - 'params': { - '1': { 'wt': 'Hello, world!' } - } - }, - 'about': '#mwt1', - 'attributes': { - 'html/0/about': '#mwt1', - 'html/0/data-mw': '{\"id\":\"mwt1\",\"target\":{\"wt\":\"Test\"},\"params\":{\"1\":{\"wt\":\"Hello, world!\"}}}', - 'html/0/data-parsoid': '{\"tsr\":[18,40],\"src\":\"{{Test|Hello, world!}}\",\"dsr\":[18,40,null,null]}', - 'html/0/typeof': 'mw:Object/Template', - 'html/1/about': '#mwt1', - 'html/1/data-parsoid': '{}' - }, - }, + ve.dm.example.MWTemplate.blockData, { 'type': '/MWtemplateBlock' }, ], - 'normalizedHtml': ve.dm.example.MWBlockTemplateSpan + 'storeItems': [ + ve.dm.example.MWTemplate.blockStoreItems + ], + 'normalizedHtml': ve.dm.example.MWTemplate.blockSpan + ve.dm.example.MWTemplate.blockContent + }, + 'mw:Template (block level - modified)': { + 'html': '' + ve.dm.example.MWTemplate.blockSpan + ve.dm.example.MWTemplate.blockContent + '', + 'data': [ + ve.dm.example.MWTemplate.blockData, + { 'type': '/MWtemplateBlock' }, + ], + 'storeItems': [ + ve.dm.example.MWTemplate.blockStoreItems + ], + 'modify': function( data ) { + data[0].attributes.mw.params['1'].wt = 'Hello, globe!'; + }, + 'normalizedHtml': ve.dm.example.MWTemplate.blockSpanModified }, 'mw:Template (inline)': { - 'html': '' + ve.dm.example.MWInlineTemplateOpen + ve.dm.example.MWInlineTemplateContent + ve.dm.example.MWInlineTemplateClose + '', + 'html': '' + ve.dm.example.MWTemplate.inlineOpen + ve.dm.example.MWTemplate.inlineContent + ve.dm.example.MWTemplate.inlineClose + '', 'data': [ { 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } }, - { - 'type': 'MWtemplateInline', - 'mw': { - 'id': 'mwt1', - 'target': { 'wt' : 'Inline' }, - 'params': { - '1': { 'wt': '1,234' } - } - }, - 'about': '#mwt1', - 'attributes': { - 'html/0/about': '#mwt1', - 'html/0/data-mw': '{\"id\":\"mwt1\",\"target\":{\"wt\":\"Inline\"},\"params\":{\"1\":{\"wt\":\"1,234\"}}}', - 'html/0/data-parsoid': '{\"tsr\":[18,34],\"src\":\"{{Inline|1,234}}\",\"dsr\":[18,34,null,null]}', - 'html/0/typeof': 'mw:Object/Template' - }, - }, + ve.dm.example.MWTemplate.inlineData, { 'type': '/MWtemplateInline' }, { 'type': '/paragraph' } ], - 'normalizedHtml': ve.dm.example.MWInlineTemplateOpen + ve.dm.example.MWInlineTemplateClose + 'storeItems': [ + ve.dm.example.MWTemplate.inlineStoreItems + ], + 'normalizedHtml': ve.dm.example.MWTemplate.inlineOpen + ve.dm.example.MWTemplate.inlineContent + ve.dm.example.MWTemplate.inlineClose + }, + 'mw:Template (inline - modified)': { + 'html': '' + ve.dm.example.MWTemplate.inlineOpen + ve.dm.example.MWTemplate.inlineContent + ve.dm.example.MWTemplate.inlineClose + '', + 'data': [ + { 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } }, + ve.dm.example.MWTemplate.inlineData, + { 'type': '/MWtemplateInline' }, + { 'type': '/paragraph' } + ], + 'storeItems': [ + ve.dm.example.MWTemplate.inlineStoreItems + ], + 'modify': function( data ) { + data[1].attributes.mw.params['1'].wt = '5,678'; + }, + 'normalizedHtml': ve.dm.example.MWTemplate.inlineOpenModified + ve.dm.example.MWTemplate.inlineClose }, 'paragraph with alienInline inside': { 'html': '

abc

',