From 444961af2f4811c4b1a20d7360de43173587fa57 Mon Sep 17 00:00:00 2001 From: Catrope Date: Fri, 3 Aug 2012 12:12:09 -0700 Subject: [PATCH] Kill all but one of the Parsoid compat hacks, we don't need them any more Also remove traces of Parsoid workarounds in tests. Tests are now passing, yay :) Change-Id: I8a59fc92c567c3595849e3e9377ce6eb6acde280 --- modules/ve/dm/ve.dm.Converter.js | 146 ++-------------------------- modules/ve/test/dm/ve.dm.example.js | 14 +-- 2 files changed, 13 insertions(+), 147 deletions(-) diff --git a/modules/ve/dm/ve.dm.Converter.js b/modules/ve/dm/ve.dm.Converter.js index 3cc1687680..95d6d30c70 100644 --- a/modules/ve/dm/ve.dm.Converter.js +++ b/modules/ve/dm/ve.dm.Converter.js @@ -308,58 +308,20 @@ ve.dm.Converter.prototype.getDataFromDom = function( domElement, annotations, da data = data.concat( createAlien( childDomElement, branchIsContent ) ); break; case Node.TEXT_NODE: - // HACK: strip trailing newlines in
  • tags. Workaround for a Parsoid bug + // HACK: strip trailing newline in
  • tags. Workaround for a Parsoid bug text = childDomElement.data; if ( domElement.nodeName.toLowerCase() === 'li' ) { - text = text.replace( /\n+$/, '' ); - } - if ( text === '' ) { - // Don't produce an empty text node or an empty paragraph - break; - } - // HACK: strip implied leading and trailing newlines in

    tags - // Workaround for a Parsoid bug - /* - * Leading newlines: - * If the previous sibling is a paragraph, do not strip leading newlines - * If there is no previous sibling, do not strip leading newlines - * Otherwise, strip 1 leading newline - * - * Trailing newlines: - * If the next sibling is a paragraph, strip 2 trailing newlines - * If there is no next sibling, do not strip trailing newlines - * Otherwise, strip 1 trailing newline - */ - contentNode = childDomElement.parentNode; - if ( contentNode.nodeName.toLowerCase() === 'p' ) { - if ( - contentNode.previousSibling && - contentNode.previousSibling.nodeName.toLowerCase() !== 'p' && - text.charAt( 0 ) === '\n' - ) { - text = text.substr( 1 ); - } - if ( contentNode.nextSibling ) { - // Strip one trailing newline - if ( text.charAt( text.length - 1 ) === "\n" ) { - text = text.substr( 0, text.length - 1 ); - } - if ( contentNode.nextSibling.nodeName.toLowerCase() === 'p' ) { - // Strip another one - if ( text.charAt( text.length - 1 ) === "\n" ) { - text = text.substr( 0, text.length - 1 ); - } - } - } + text = text.replace( /\n$/, '' ); } if ( !branchIsContent ) { // If it's bare content, strip leading and trailing newlines + // FIXME these newlines should be turned into placeholders instead text = text.replace( /^\n+/, '' ).replace( /\n+$/, '' ); - if ( text === '' ) { - // Don't produce an empty text node - break; - } + } + if ( text === '' ) { + // Don't produce an empty text node or an empty paragraph + break; } // Start auto-wrapping of bare content @@ -375,6 +337,8 @@ ve.dm.Converter.prototype.getDataFromDom = function( domElement, annotations, da break; case Node.COMMENT_NODE: // TODO: Preserve comments by inserting them into the linear model too + // Could use placeholders for this too, although they'd need to be + // inline in certain cases break; } } @@ -419,40 +383,6 @@ ve.dm.Converter.prototype.getDomFromData = function( data ) { container = document.createElement( 'div' ), domElement = container, annotationStack = {}; // { hash: DOMnode } - - function fixupText( text, node ) { - // HACK reintroduce newlines needed to make Parsoid not freak out - // This reverses the newline stripping done in getDataFromDom() - /* - * Leading newlines: - * If the previous sibling is a heading, add 1 leading newline - * Otherwise, do not add any leading newlines - * - * Trailing newlines: - * If the next sibling is a paragraph, add 2 trailing newlines - * If there is no next sibling, do not add any trailing newlines - * Otherwise, add 1 trailing newline - */ - if ( node.parentNode.nodeName.toLowerCase() === 'p' ) { - if ( - node.parentNode.previousSibling && - node.parentNode.previousSibling.nodeName.toLowerCase().match( /h\d/ ) && - !node.previousSibling - ) { - text = "\n" + text; - } - if ( node.parentNode.nextSibling && !node.nextSibling ) { - // Add one trailing newline - text += "\n"; - if ( node.parentNode.nextSibling.nodeName.toLowerCase() === 'p' ) { - // Add another one - text += "\n"; - } - } - } - return text; - } - for ( i = 0; i < data.length; i++ ) { if ( typeof data[i] === 'string' ) { // Text @@ -579,64 +509,6 @@ ve.dm.Converter.prototype.getDomFromData = function( data ) { } } } - - // HACK: do postprocessing on the data to work around bugs in Parsoid concerning paragraphs - // inside list items - $( container ).find( 'li, dd, dt' ).each( function() { - var $sublists = $(this).children( 'ul, ol, dl' ), - $firstChild = $(this.firstChild), - $lastChild = $(this.lastChild); - if ( $firstChild.is( 'p' ) ) { - // Unwrap the first paragraph, unless it has stx=html - datamw = $.parseJSON( $firstChild.attr( 'data-rt' ) ) || {}; - if ( datamw.stx !== 'html' ) { - $firstChild.replaceWith( $firstChild.contents() ); - } - } - - // Append a newline to the end of the

  • , provided its last child is not a list - if ( $lastChild.is( ':not(ul,ol)' ) ) { - $(this).append( "\n" ); - } - // Append a newline before every sublist that is preceded by something - $sublists.each( function() { - if ( this.previousSibling ) { - if ( this.previousSibling.nodeName.toLowerCase() === 'text' ) { - this.previousSibling.data += "\n"; - } else { - this.parentNode.insertBefore( document.createTextNode( "\n" ), this ); - } - } - }); - }); - - // HACK more postprocessing, this time to add newlines to paragraphs so Parsoid doesn't freak out - $( container ) - // Get all text nodes - .find( '*' ) - .contents() - .filter( function() { - // Text nodes only - return this.nodeType === 3 && - // Exclude text nodes within lists - $( this.parentNode ).closest( 'li, dd, dt' ).length === 0; - } ) - .each( function() { - this.data = fixupText( this.data, this ); - } ); - // And add newlines after headings too - $( container ).find( 'h1, h2, h3, h4, h5, h6' ).each( function() { - // If there is no next sibling, we don't need to add a newline - // If the next sibling is a paragraph, fixupText() has taken care of it - // Otherwise, add a newline after the heading - if ( this.nextSibling && this.nextSibling.nodeName.toLowerCase() !== 'p' ) { - this.parentNode.insertBefore( document.createTextNode( "\n" ), this.nextSibling ); - } - // If the previous sibling exists and is a pre, we need to add a newline before - if ( this.previousSibling && this.previousSibling.nodeName.toLowerCase() === 'pre' ) { - this.parentNode.insertBefore( document.createTextNode( "\n" ), this ); - } - } ); return container; }; diff --git a/modules/ve/test/dm/ve.dm.example.js b/modules/ve/test/dm/ve.dm.example.js index f461d58f01..d99a5ec23c 100644 --- a/modules/ve/test/dm/ve.dm.example.js +++ b/modules/ve/test/dm/ve.dm.example.js @@ -22,20 +22,17 @@ ve.dm.example.html = '

    d

    ' + '' + '
      ' + '
    1. ' + - 'g' + // Not wrapped in a

      due to Parsoid behavior - "\n" + // Workaround for Parsoid bug + '

      g

      ' + '
      2. ' + '
    ' + '' + @@ -441,10 +438,7 @@ ve.dm.example.domToDataCases = { 'data': ve.dm.example.data }, 'list item with space followed by link': { - // This HTML is weird because of workarounds for Parsoid bugs: - // * newline before
    • - // * first paragraph in an
  • not wrapped in

    - 'html': '

    ', + 'html': '', 'data': [ { 'type': 'list', 'attributes': { 'style': 'bullet' } }, { 'type': 'listItem' },