From b79c98f1e3f14a2acdefe01602e68092be56d605 Mon Sep 17 00:00:00 2001 From: Ed Sanders Date: Tue, 5 Jun 2018 12:41:09 +0100 Subject: [PATCH] Simplify conversion of images Assume that images provided by Parsoid will adhere to the DOMspec. Change-Id: I52eb4f27e6b1e1cd092133c3e27e42021ae83783 --- .../ve-mw/ce/nodes/ve.ce.MWInlineImageNode.js | 2 +- modules/ve-mw/dm/models/ve.dm.MWImageModel.js | 5 -- .../ve-mw/dm/nodes/ve.dm.MWBlockImageNode.js | 47 +++++++------------ .../ve-mw/dm/nodes/ve.dm.MWInlineImageNode.js | 39 +++++++-------- modules/ve-mw/tests/dm/ve.dm.mwExample.js | 7 ++- 5 files changed, 39 insertions(+), 61 deletions(-) diff --git a/modules/ve-mw/ce/nodes/ve.ce.MWInlineImageNode.js b/modules/ve-mw/ce/nodes/ve.ce.MWInlineImageNode.js index 5fe444582f..aa932c4c3f 100644 --- a/modules/ve-mw/ce/nodes/ve.ce.MWInlineImageNode.js +++ b/modules/ve-mw/ce/nodes/ve.ce.MWInlineImageNode.js @@ -25,7 +25,7 @@ ve.ce.MWInlineImageNode = function VeCeMWInlineImageNode( model, config ) { .text( model.getFilename() ); $image = $( [] ); } else { - if ( model.getAttribute( 'isLinked' ) ) { + if ( model.getAttribute( 'href' ) ) { this.$element = $( '' ).addClass( 'image' ); $image = $( '' ).appendTo( this.$element ); } else { diff --git a/modules/ve-mw/dm/models/ve.dm.MWImageModel.js b/modules/ve-mw/dm/models/ve.dm.MWImageModel.js index 984192f427..2085f12e14 100644 --- a/modules/ve-mw/dm/models/ve.dm.MWImageModel.js +++ b/modules/ve-mw/dm/models/ve.dm.MWImageModel.js @@ -572,11 +572,6 @@ ve.dm.MWImageModel.prototype.getUpdatedAttributes = function () { attrs.href = this.getImageHref(); attrs.resource = this.getImageResourceName(); - // If converting from block to inline, set isLinked=true to avoid |link= - if ( origAttrs.isLinked === undefined && this.getImageNodeType() === 'mwInlineImage' ) { - attrs.isLinked = true; - } - return attrs; }; diff --git a/modules/ve-mw/dm/nodes/ve.dm.MWBlockImageNode.js b/modules/ve-mw/dm/nodes/ve.dm.MWBlockImageNode.js index d7071b3af8..cb4ce5b7a6 100644 --- a/modules/ve-mw/dm/nodes/ve.dm.MWBlockImageNode.js +++ b/modules/ve-mw/dm/nodes/ve.dm.MWBlockImageNode.js @@ -68,26 +68,17 @@ ve.dm.MWBlockImageNode.static.classAttributes = { ve.dm.MWBlockImageNode.static.toDataElement = function ( domElements, converter ) { var dataElement, newDimensions, attributes, figure, imgWrapper, img, caption, - classAttr, typeofAttrs, errorIndex, width, height, altText, types; - - // Workaround for jQuery's .children() being expensive due to - // https://github.com/jquery/sizzle/issues/311 - function findChildren( parent, nodeNames ) { - return Array.prototype.filter.call( parent.childNodes, function ( element ) { - return nodeNames.indexOf( element.nodeName.toLowerCase() ) !== -1; - } ); - } + classAttr, typeofAttrs, errorIndex, width, height, types; figure = domElements[ 0 ]; - imgWrapper = findChildren( figure, [ 'a', 'span' ] )[ 0 ] || null; - img = imgWrapper && findChildren( imgWrapper, [ 'img', 'video' ] )[ 0 ] || null; - caption = findChildren( figure, [ 'figcaption' ] )[ 0 ] || null; + imgWrapper = figure.children[ 0 ]; // or + img = imgWrapper.children[ 0 ]; // or
or undefined classAttr = figure.getAttribute( 'class' ); typeofAttrs = figure.getAttribute( 'typeof' ).split( ' ' ); errorIndex = typeofAttrs.indexOf( 'mw:Error' ); - width = img && img.getAttribute( 'width' ); - height = img && img.getAttribute( 'height' ); - altText = img && img.getAttribute( 'alt' ); + width = img.getAttribute( 'width' ); + height = img.getAttribute( 'height' ); if ( errorIndex !== -1 ) { typeofAttrs.splice( errorIndex, 1 ); @@ -98,25 +89,19 @@ ve.dm.MWBlockImageNode.static.toDataElement = function ( domElements, converter attributes = { mediaClass: types.mediaClass, type: types.frameType, - href: ( imgWrapper && imgWrapper.getAttribute( 'href' ) ) || '', - src: ( img && ( img.getAttribute( 'src' ) || img.getAttribute( 'poster' ) ) ) || '', - resource: img && img.getAttribute( 'resource' ) + src: img.getAttribute( 'src' ) || img.getAttribute( 'poster' ), + href: imgWrapper.getAttribute( 'href' ), + resource: img.getAttribute( 'resource' ), + width: width !== null && width !== '' ? +width : null, + height: height !== null && height !== '' ? +height : null, + alt: img.getAttribute( 'alt' ), + isError: errorIndex !== -1 }; - if ( altText !== null ) { - attributes.alt = altText; - } - if ( errorIndex !== -1 ) { - attributes.isError = true; - } - this.setClassAttributes( attributes, classAttr ); attributes.align = attributes.align || 'default'; - attributes.width = width !== null && width !== '' ? Number( width ) : null; - attributes.height = height !== null && height !== '' ? Number( height ) : null; - // Default-size if ( attributes.defaultSize ) { // Force wiki-default size for thumb and frameless @@ -166,7 +151,7 @@ ve.dm.MWBlockImageNode.static.toDomElements = function ( data, doc, converter ) dataElement = data[ 0 ], mediaClass = dataElement.attributes.mediaClass, figure = doc.createElement( 'figure' ), - imgWrapper = doc.createElement( dataElement.attributes.href !== '' ? 'a' : 'span' ), + imgWrapper = doc.createElement( dataElement.attributes.href ? 'a' : 'span' ), img = doc.createElement( mediaClass === 'Image' ? 'img' : 'video' ), wrapper = doc.createElement( 'div' ), classAttr = this.getClassAttrFromAttributes( dataElement.attributes ), @@ -179,7 +164,7 @@ ve.dm.MWBlockImageNode.static.toDomElements = function ( data, doc, converter ) figure.className = classAttr; } - if ( dataElement.attributes.href !== '' ) { + if ( dataElement.attributes.href ) { imgWrapper.setAttribute( 'href', dataElement.attributes.href ); } @@ -200,7 +185,7 @@ ve.dm.MWBlockImageNode.static.toDomElements = function ( data, doc, converter ) img.setAttribute( 'width', width ); img.setAttribute( 'height', height ); img.setAttribute( 'resource', dataElement.attributes.resource ); - if ( dataElement.attributes.alt !== undefined ) { + if ( typeof dataElement.attributes.alt === 'string' ) { img.setAttribute( 'alt', dataElement.attributes.alt ); } figure.appendChild( imgWrapper ); diff --git a/modules/ve-mw/dm/nodes/ve.dm.MWInlineImageNode.js b/modules/ve-mw/dm/nodes/ve.dm.MWInlineImageNode.js index 1368c6d995..53235ffcc8 100644 --- a/modules/ve-mw/dm/nodes/ve.dm.MWInlineImageNode.js +++ b/modules/ve-mw/dm/nodes/ve.dm.MWInlineImageNode.js @@ -52,15 +52,15 @@ ve.dm.MWInlineImageNode.static.blacklistedAnnotationTypes = [ 'link' ]; ve.dm.MWInlineImageNode.static.toDataElement = function ( domElements, converter ) { var dataElement, attributes, types, - $figureInline = $( domElements[ 0 ] ), - $firstChild = $figureInline.children().first(), // could be or - $img = $firstChild.children().first(), - typeofAttrs = $figureInline.attr( 'typeof' ).split( ' ' ), - classes = $figureInline.attr( 'class' ), + figureInline = domElements[ 0 ], + imgWrapper = figureInline.children[ 0 ], // could be or + img = imgWrapper.children[ 0 ], + typeofAttrs = ( figureInline.getAttribute( 'typeof' ) || '' ).split( ' ' ), + classes = figureInline.getAttribute( 'class' ), recognizedClasses = [], errorIndex = typeofAttrs.indexOf( 'mw:Error' ), - width = $img.attr( 'width' ), - height = $img.attr( 'height' ); + width = img.getAttribute( 'width' ), + height = img.getAttribute( 'height' ); if ( errorIndex !== -1 ) { typeofAttrs.splice( errorIndex, 1 ); @@ -71,23 +71,16 @@ ve.dm.MWInlineImageNode.static.toDataElement = function ( domElements, converter attributes = { mediaClass: types.mediaClass, type: types.frameType, - src: $img.attr( 'src' ), - resource: $img.attr( 'resource' ), - originalClasses: classes + src: img.getAttribute( 'src' ), + href: imgWrapper.getAttribute( 'href' ), + resource: img.getAttribute( 'resource' ), + originalClasses: classes, + width: width !== null && width !== '' ? +width : null, + height: height !== null && height !== '' ? +height : null, + alt: img.getAttribute( 'alt' ), + isError: errorIndex !== -1 }; - if ( errorIndex !== -1 ) { - attributes.isError = true; - } - - attributes.width = width !== undefined && width !== '' ? Number( width ) : null; - attributes.height = height !== undefined && height !== '' ? Number( height ) : null; - - attributes.isLinked = $firstChild.is( 'a' ); - if ( attributes.isLinked ) { - attributes.href = $firstChild.attr( 'href' ); - } - // Extract individual classes classes = typeof classes === 'string' ? classes.trim().split( /\s+/ ) : []; @@ -169,7 +162,7 @@ ve.dm.MWInlineImageNode.static.toDomElements = function ( data, doc ) { figureInline.className = classes.join( ' ' ); } - if ( data.attributes.isLinked ) { + if ( data.attributes.href ) { firstChild = doc.createElement( 'a' ); firstChild.setAttribute( 'href', data.attributes.href ); } else { diff --git a/modules/ve-mw/tests/dm/ve.dm.mwExample.js b/modules/ve-mw/tests/dm/ve.dm.mwExample.js index a45c08a74b..a72d1f97c8 100644 --- a/modules/ve-mw/tests/dm/ve.dm.mwExample.js +++ b/modules/ve-mw/tests/dm/ve.dm.mwExample.js @@ -267,6 +267,8 @@ ve.dm.mwExample.MWBlockImage = { src: ve.ce.minImgDataUri, width: 1, height: 2, + alt: null, + isError: false, resource: 'FooBar', originalClasses: 'mw-halign-right foobar', unrecognizedClasses: [ 'foobar' ] @@ -299,7 +301,8 @@ ve.dm.mwExample.MWInlineImage = { mediaClass: 'Image', width: 135, height: 155, - isLinked: true, + alt: null, + isError: false, valign: 'text-top', resource: './File:Wiki.png', type: 'none', @@ -1491,6 +1494,8 @@ ve.dm.mwExample.domToDataCases = { src: ve.ce.minImgDataUri, width: 1, height: 2, + alt: null, + isError: false, resource: 'FooBar' } },