From 7319038ed6aabc94dfbc0b435d60d3f153319694 Mon Sep 17 00:00:00 2001 From: Catrope Date: Thu, 16 Aug 2012 13:06:18 -0700 Subject: [PATCH] Strip generated

tags in dataToDom domToData wraps bare content in paragraph elements, which were then converted to

tags by domToData. With this fix, HTML with "missing"

tags actually round-trips through the editor correctly now, rather than having

tags added wherever VE believes they should exist. * Mark generated paragraph elements with .internal.generated = 'wrapper' ** This signifies the wrapper was generated but its contents were not, so the right thing to do when converting back to HTML is to remove the wrapper and keep the contents. We might want to use other values of generated in the future. * Unwrap nodes with generated=wrapper when converting to HTML Tests: * Add 'generated': 'wrapper' as appropriate. Only affects 1 test * Remove 'normalizedHtml' for this test because it is no longer needed ** Need to keep 'normalizedHtml' for now because we normalize hrefs * Eventually the main example should test bare content, but that requires touching a lot of stuff. The main example could use some beefing up anyway. Change-Id: I277ad5fe3f64e07c1bbf49007d6bbaecc90b7466 --- modules/ve/dm/ve.dm.Converter.js | 46 ++++++++++++++++++++++++----- modules/ve/test/dm/ve.dm.example.js | 10 +++---- 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/modules/ve/dm/ve.dm.Converter.js b/modules/ve/dm/ve.dm.Converter.js index 3750766f08..4f1dbce89a 100644 --- a/modules/ve/dm/ve.dm.Converter.js +++ b/modules/ve/dm/ve.dm.Converter.js @@ -275,7 +275,12 @@ ve.dm.Converter.prototype.getDataFromDom = function ( domElement, annotations, d if ( annotation ) { // Start auto-wrapping of bare content if ( !wrapping && !alreadyWrapped && !branchIsContent ) { - data.push( { 'type': 'paragraph' } ); + // Mark this paragraph as having been generated by + // us, so we can strip it on the way out + data.push( { + 'type': 'paragraph', + 'internal': { 'generated': 'wrapper' } + } ); wrapping = true; } // Append child element data @@ -328,7 +333,12 @@ ve.dm.Converter.prototype.getDataFromDom = function ( domElement, annotations, d // Start auto-wrapping of bare content if ( !wrapping && !alreadyWrapped && !branchIsContent ) { - paragraph = { 'type': 'paragraph' }; + // Mark this paragraph as having been generated by + // us, so we can strip it on the way out + paragraph = { + 'type': 'paragraph', + 'internal': { 'generated': 'wrapper' } + }; data.push( paragraph ); wrapping = true; wrapperElement = paragraph; @@ -400,7 +410,7 @@ ve.dm.Converter.prototype.getDataFromDom = function ( domElement, annotations, d !ve.dm.nodeFactory.isNodeContent( branchType ) && ( childTypes === null || ve.indexOf( 'paragraph', childTypes ) !== -1 ) ) { - data.push( { 'type': 'paragraph' } ); + data.push( { 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } } ); data.push( { 'type': '/paragraph' } ); } @@ -410,7 +420,10 @@ ve.dm.Converter.prototype.getDataFromDom = function ( domElement, annotations, d } // Don't return an empty document if ( branchType === 'document' && data.length === 0 ) { - return [{ 'type': 'paragraph' }, { 'type': '/paragraph' }]; + return [ + { 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } }, + { 'type': '/paragraph' } + ]; } return data; }; @@ -423,8 +436,8 @@ ve.dm.Converter.prototype.getDataFromDom = function ( domElement, annotations, d * @returns {HTMLElement} Wrapper div containing the resulting HTML */ ve.dm.Converter.prototype.getDomFromData = function ( data ) { - var text, i, annotations, hash, annotationElement, done, dataElement, wrapper, - childDomElement, pre, post, + var text, i, j, annotations, hash, annotationElement, done, dataElement, wrapper, + childDomElement, pre, post, parentDomElement, container = document.createElement( 'div' ), domElement = container, annotationStack = {}; // { hash: DOMnode } @@ -572,13 +585,30 @@ ve.dm.Converter.prototype.getDomFromData = function ( data ) { } } } + + // If closing a generated wrapper node, unwrap it + // It would be nicer if we could avoid generating in the first + // place, but then remembering where we have to skip ascending + // to the parent would be tricky. + parentDomElement = domElement.parentNode; + if ( domElement.veInternal && domElement.veInternal.generated === 'wrapper' ) { + for ( j = 0; j < domElement.childNodes.length; j++ ) { + parentDomElement.insertBefore( + domElement.childNodes[j], + domElement + ); + } + parentDomElement.removeChild( domElement ); + } + + // Clean up the internal data delete domElement.veInternal; // Ascend to parent node - domElement = domElement.parentNode; + domElement = parentDomElement; } else { // Create node from data childDomElement = this.getDomElementFromDataElement( dataElement ); - // Add reference to internal data to propagate whitespace info + // Add reference to internal data if ( dataElement.internal ) { childDomElement.veInternal = dataElement.internal; } diff --git a/modules/ve/test/dm/ve.dm.example.js b/modules/ve/test/dm/ve.dm.example.js index c278809b20..fe70d0c47a 100644 --- a/modules/ve/test/dm/ve.dm.example.js +++ b/modules/ve/test/dm/ve.dm.example.js @@ -13,6 +13,7 @@ ve.dm.example = {}; * Serialized HTML. * * This is what the parser will emit. + * TODO remove some of the

s here to test automatic wrapping */ ve.dm.example.html = '

abc

' + @@ -677,26 +678,25 @@ ve.dm.example.domToDataCases = { ] }, 'whitespace preservation in list items': { - 'normalizedHtml': '', 'html': '', 'data': [ { 'type': 'list', 'attributes': { 'style': 'bullet' } }, { 'type': 'listItem' }, - { 'type': 'paragraph' }, + { 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } }, 'F', 'o', 'o', { 'type': '/paragraph' }, { 'type': '/listItem' }, { 'type': 'listItem' }, - { 'type': 'paragraph', 'internal': { 'whitespace': [ undefined, ' ' ] } }, + { 'type': 'paragraph', 'internal': { 'whitespace': [ undefined, ' ' ], 'generated': 'wrapper' } }, 'B', 'a', 'r', { 'type': '/paragraph' }, { 'type': '/listItem' }, { 'type': 'listItem' }, - { 'type': 'paragraph', 'internal': { 'whitespace': [ undefined, undefined, ' ' ] } }, + { 'type': 'paragraph', 'internal': { 'whitespace': [ undefined, undefined, ' ' ], 'generated': 'wrapper' } }, 'B', 'a', 'z', @@ -705,7 +705,7 @@ ve.dm.example.domToDataCases = { { 'type': 'listItem' }, { 'type': 'paragraph', - 'internal': { 'whitespace': [ undefined, ' ', ' ' ] } + 'internal': { 'whitespace': [ undefined, ' ', ' ' ], 'generated': 'wrapper' } }, 'Q', 'u',