Strip generated <p> tags in dataToDom

domToData wraps bare content in paragraph elements, which were then
converted to <p> tags by domToData. With this fix, HTML with "missing"
<p> tags actually round-trips through the editor correctly now, rather
than having <p> 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
This commit is contained in:
Catrope 2012-08-16 13:06:18 -07:00
parent ce60b54d33
commit 7319038ed6
2 changed files with 43 additions and 13 deletions

View file

@ -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;
}

View file

@ -13,6 +13,7 @@ ve.dm.example = {};
* Serialized HTML.
*
* This is what the parser will emit.
* TODO remove some of the <p>s here to test automatic wrapping
*/
ve.dm.example.html =
'<h1>a<b>b</b><i>c</i></h1>' +
@ -677,26 +678,25 @@ ve.dm.example.domToDataCases = {
]
},
'whitespace preservation in list items': {
'normalizedHtml': '<ul><li><p>Foo</p></li><li><p> Bar</p></li><li><p>Baz </p></li><li><p> Quux </p></li></ul>',
'html': '<ul><li>Foo</li><li> Bar</li><li>Baz </li><li> Quux </li></ul>',
'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',