Wrap inline elements properly

The HTML "1<br/>2" was being converted to a linmod that looked like
"<p>1</p><br></br><p>2</p>". This commit fixes the wrapping logic such
that the result is "<p>1<br></br>2</p>" instead. In general, inline
nodes (content nodes) should not interrupt the wrapping, but block nodes
should.

This creates a problem for alien nodes: normally, we determine whether an
alien node is a block alien or an inline alien based on context, but if
we're in wrapping mode we're unsure of the context. We can't tell the
difference between "1<tt>Foo</tt>2" (should be wrapped as one, because
tt is inline) and "1<figure></figure>2" (1 and 2 should be wrapped
separately, because figure is block) using context alone, so in these
cases (and ONLY in these cases) we look up whether the HTML tag in
question is an inline tag or a block tag and use that to decide.

Change-Id: I75e7f3da387dd401d9b93e09a21751951eccbb83

modules/ve/dm/ve.dm.Converter.js

@@ -336,8 +336,23 @@ ve.dm.Converter.prototype.getDataFromDom = function ( domElement, annotations, d
 					);
 					break;
 				}
+
+				// Look up child element type
+				childDataElement = this.getDataElementFromDomElement( childDomElement );
 				// End auto-wrapping of bare content from a previously processed node
-				if ( wrapping ) {
+				// but only if childDataElement is a non-content element or if
+				// we are about to produce a block alien
+				if (
+					wrapping && (
+						(
+							childDataElement &&
+							!ve.dm.nodeFactory.isNodeContent( childDataElement.type )
+						) || (
+							!childDataElement &&
+							ve.isBlockElement( childDomElement )
+						)
+					)
+				) {
 					if ( wrappedWhitespace !== '' ) {
 						// Remove wrappedWhitespace from data
 						data.splice( -wrappedWhitespace.length, wrappedWhitespace.length );
@@ -350,7 +365,6 @@ ve.dm.Converter.prototype.getDataFromDom = function ( domElement, annotations, d
 					wrappingIsOurs = false;
 				}
 				// Append child element data
-				childDataElement = this.getDataElementFromDomElement( childDomElement );
 				if ( childDataElement ) {
 					data = data.concat(
 						this.getDataFromDom(
@@ -365,8 +379,13 @@ ve.dm.Converter.prototype.getDataFromDom = function ( domElement, annotations, d
 					prevElement = childDataElement;
 					break;
 				}
-				// We don't know what this is, fall back to alien
-				alien = createAlien( childDomElement, branchIsContent );
+				// We don't know what this is, fall back to alien.
+				// If we're in wrapping mode, we don't know if this alien is
+				// supposed to be block or inline, so detect it based on the HTML
+				// tag name.
+				alien = createAlien( childDomElement, wrapping ?
+					!ve.isBlockElement( childDomElement ) : branchIsContent
+				);
 				data = data.concat( alien );
 				processNextWhitespace( alien[0] );
 				prevElement = alien[0];

modules/ve/test/dm/ve.dm.example.js

@@ -477,7 +477,6 @@ ve.dm.example.domToDataCases = {
 			{ 'type': '/paragraph' }
 		]
 	},
-	// TODO these last two are broken due to newline hacks, will be unbroken once we remove the newline hacks
 	'paragraphs with an alienBlock between them': {
 		'html': '<p>abc</p><figure>abc</figure><p>def</p>',
 		'data': [
@@ -495,6 +494,88 @@ ve.dm.example.domToDataCases = {
 			{ 'type': '/paragraph' }
 		]
 	},
+	'wrapping of bare content': {
+		'html': 'abc',
+		'data': [
+			{ 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } },
+			'a',
+			'b',
+			'c',
+			{ 'type': '/paragraph' }
+		]
+	},
+	'wrapping of bare content with inline node': {
+		'html': '1<br/>2',
+		'data': [
+			{ 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } },
+			'1',
+			{ 'type': 'break' },
+			{ 'type': '/break' },
+			'2',
+			{ 'type': '/paragraph' }
+		]
+	},
+	'wrapping of bare content with inline alien': {
+		'html': '1<tt class="bar">baz</tt>2',
+		'data': [
+			{ 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } },
+			'1',
+			{
+				'type': 'alienInline',
+				'attributes': { 'html': '<tt class="bar">baz</tt>' }
+			},
+			{ 'type': '/alienInline' },
+			'2',
+			{ 'type': '/paragraph' }
+		]
+	},
+	'wrapping of bare content with block alien': {
+		'html': '1<figure class="bar">baz</figure>2',
+		'data': [
+			{ 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } },
+			'1',
+			{ 'type': '/paragraph' },
+			{
+				'type': 'alienBlock',
+				'attributes': { 'html': '<figure class="bar">baz</figure>' }
+			},
+			{ 'type': '/alienBlock' },
+			{ 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } },
+			'2',
+			{ 'type': '/paragraph' }
+		]
+	},
+	'wrapping of bare content between structural nodes': {
+		'html': '<table></table>abc<table></table>',
+		'data': [
+			{ 'type': 'table' },
+			{ 'type': '/table' },
+			{ 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } },
+			'a',
+			'b',
+			'c',
+			{ 'type': '/paragraph' },
+			{ 'type': 'table' },
+			{ 'type': '/table' }
+		]
+	},
+	'wrapping of bare content between paragraphs': {
+		'html': '<p>abc</p>def<p></p>',
+		'data': [
+			{ 'type': 'paragraph' },
+			'a',
+			'b',
+			'c',
+			{ 'type': '/paragraph' },
+			{ 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } },
+			'd',
+			'e',
+			'f',
+			{ 'type': '/paragraph' },
+			{ 'type': 'paragraph' },
+			{ 'type': '/paragraph' }
+		]
+	},
 	'example document': {
 		'html': ve.dm.example.html,
 		'data': ve.dm.example.data

modules/ve/ve.js

@@ -748,6 +748,44 @@
 		}
 	};
 
+	/**
+	 * Check whether a given DOM element is of a block or inline type
+	 * @param {HTMLElement} element
+	 * @returns {Boolean} True if element is block, false if it is inline
+	 */
+	ve.isBlockElement = function ( element ) {
+		return ve.isBlockElementType( element.nodeName.toLowerCase() );
+	};
+
+	/**
+	 * Check whether a given tag name is a block or inline tag
+	 * @param {String} nodeName All-lowercase HTML tag name
+	 * @returns {Boolean} True if block, false if inline
+	 */
+	ve.isBlockElementType = function ( nodeName ) {
+		return ve.indexOf( nodeName, ve.isBlockElementType.blockTypes ) !== -1;
+	};
+
+	/**
+	 * Private data for ve.isBlockElementType()
+	 */
+	ve.isBlockElementType.blockTypes = [
+		'div', 'p',
+		// tables
+		'table', 'tbody', 'thead', 'tfoot', 'caption',  'th', 'tr', 'td',
+		// lists
+		'ul', 'ol', 'li', 'dl', 'dt', 'dd',
+		// HTML5 heading content
+		'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'hgroup',
+		// HTML5 sectioning content
+		'article', 'aside', 'body', 'nav', 'section', 'footer', 'header', 'figure',
+		'figcaption', 'fieldset', 'details', 'blockquote',
+		// other
+		'br', 'hr', 'button', 'canvas', 'center', 'col', 'colgroup', 'embed',
+		// 'img', // Really?
+		'map', 'object', 'pre', 'progress', 'video'
+	];
+
 	// Expose
 	window.ve = ve;
 }() );