Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug fixing for shape processing and inserting linked images #1088

Merged
merged 16 commits into from
Nov 8, 2017
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 73 additions & 19 deletions plugins/pastefromword/filter/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@
'meta',
'link'
],
shapeTags = [
'v:arc',
'v:curve',
'v:line',
'v:oval',
'v:polyline',
'v:rect',
'v:roundrect',
'v:group'
],
links = {},
inComment = 0;

Expand All @@ -31,7 +41,15 @@
CKEDITOR.plugins.pastefromword = {};

CKEDITOR.cleanWord = function( mswordHtml, editor ) {
var msoListsDetected = Boolean( mswordHtml.match( /mso-list:\s*l\d+\s+level\d+\s+lfo\d+/ ) );
var msoListsDetected = Boolean( mswordHtml.match( /mso-list:\s*l\d+\s+level\d+\s+lfo\d+/ ) ),
shapesIds = [];

function shapeTagging( element ) {
// Check if regular or canvas shape (#1088).
if ( element.attributes[ 'o:gfxdata' ] || element.parent.name === 'v:group' ) {
shapesIds.push( element.attributes.id );
}
}

// Before filtering inline all the styles to allow because some of them are available only in style
// sheets. This step is skipped in IEs due to their flaky support for custom types in dataTransfer. (http://dev.ckeditor.com/ticket/16847)
Expand All @@ -44,7 +62,7 @@

var fragment = CKEDITOR.htmlParser.fragment.fromHtml( mswordHtml );

filter = new CKEDITOR.htmlParser.filter( {
var filterDefinition = {
root: function( element ) {
element.filterChildren( filter );

Expand Down Expand Up @@ -110,6 +128,17 @@
element.attributes.alt && element.attributes.alt.match( /^https?:\/\// ) ) {
element.attributes.src = element.attributes.alt;
}

var imgShapesIds = element.attributes[ 'v:shapes' ] ? element.attributes[ 'v:shapes' ].split( ' ' ) : [];
// Check whether attribute contains shapes recognised earlier (stored in global list of shapesIds).
// If so, add additional data-attribute to img tag.
var isShapeFromList = CKEDITOR.tools.array.every( imgShapesIds, function( shapeId ) {
return shapesIds.indexOf( shapeId ) > -1;
} );
if ( imgShapesIds.length && isShapeFromList ) {
element.attributes[ 'data-cke-is-shape' ] = true;
}

},
'p': function( element ) {
element.filterChildren( filter );
Expand Down Expand Up @@ -363,31 +392,49 @@
'v:imagedata': remove,
// This is how IE8 presents images.
'v:shape': function( element ) {
var emptyShapeInsideGroup = element.parent && element.parent.name === 'v:group';

// In chrome a <v:shape> element may be followed by an <img> element with the same content.
var duplicate = false;
element.parent.getFirst( function( child ) {
if ( child.name == 'img' &&
// Remain for legacy comaptibility (#995, #1069).
if ( !element.attributes[ 'o:gfxdata' ] && !emptyShapeInsideGroup ) {
var duplicate = false,
child;

// Canvas leaves empty `v:shape`, which should not be converted into img tag.
// These empty `v:shape` contains 2 attributes which helps distinguish it.
child = element.getFirst( 'v:imagedata' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be assigned on initialization, 5 lines above. And then the comment will fit better, right after if which it describes.

Copy link
Contributor Author

@msamsel msamsel Nov 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer not to move comment after if, as it might imply relation to shapeTagging method. All other if description in entire file seems to be described just before it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@msamsel I fully agree, my mistake here, what I meant was

And then the comment will fit better, right before if which it describes

By merging var initialization you achieved just what I was thinking about so it is ok 👍

if ( child && ( child.attributes.croptop !== undefined || child.attributes.cropbottom !== undefined ) ) {
shapeTagging( element );
return;
}

// Sometimes child with proper ID might be nested in other tag.
element.parent.find( function( child ) {
if ( child.name == 'img' &&
child.attributes &&
child.attributes[ 'v:shapes' ] == element.attributes.id ) {
duplicate = true;
}
} );
duplicate = true;
}
}, true );

if ( duplicate ) return false;
if ( duplicate ) return false;

var src = '';
element.forEach( function( child ) {
if ( child.attributes && child.attributes.src ) {
src = child.attributes.src;
}
}, CKEDITOR.NODE_ELEMENT, true );
var src = '';
element.forEach( function( child ) {
if ( child.attributes && child.attributes.src ) {
src = child.attributes.src;
}
}, CKEDITOR.NODE_ELEMENT, true );

element.filterChildren( filter );
element.filterChildren( filter );

element.name = 'img';
element.attributes.src = element.attributes.src || src;
element.name = 'img';
element.attributes.src = element.attributes.src || src;

delete element.attributes.type;
delete element.attributes.type;
} else {
shapeTagging( element );
}
},

'style': function() {
Expand Down Expand Up @@ -439,8 +486,15 @@

return content;
}
};

// Add shape processing to filter definition.
CKEDITOR.tools.array.forEach( shapeTags, function( shapeTag ) {
filterDefinition.elements[ shapeTag ] = shapeTagging;
} );

filter = new CKEDITOR.htmlParser.filter( filterDefinition );

var writer = new CKEDITOR.htmlParser.basicWriter();

filter.applyTo( fragment );
Expand Down
1 change: 1 addition & 0 deletions plugins/pastefromword/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
var forceFromWord = 0,
path = this.path;

editor.filter.allow( 'img[data-cke-is-shape]' );
editor.addCommand( 'pastefromword', {
// Snapshots are done manually by editable.insertXXX methods.
canUndo: false,
Expand Down
22 changes: 10 additions & 12 deletions plugins/pastefromwordimage/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@
extractImagesFromRtf: function( rtfContent ) {
var ret = [],
rePictureHeader = /\{\\pict[\s\S]+?\\bliptag\-?\d+(\\blipupi\-?\d+)?(\{\\\*\\blipuid\s?[\da-fA-F]+)?[\s\}]*?/,
reShapeHeader = /\{\\shp\{\\\*\\shpinst[\s\S]+?\{\\\*\\svb\s?/,
rePictureOrShape = new RegExp( '(?:(' + rePictureHeader.source + ')|(' + reShapeHeader.source + '))([\\da-fA-F\\s]+)\\}', 'g' ),
rePicture = new RegExp( '(?:(' + rePictureHeader.source + '))([\\da-fA-F\\s]+)\\}', 'g' ),
wholeImages,
imageType;
wholeImages = rtfContent.match( rePictureOrShape );

wholeImages = rtfContent.match( rePicture );
if ( !wholeImages ) {
return ret;
}
Expand All @@ -103,20 +103,15 @@
hex: imageType ? wholeImages[ i ].replace( rePictureHeader, '' ).replace( /[^\da-fA-F]/g, '' ) : null,
type: imageType
} );
} else if ( reShapeHeader.test( wholeImages[ i ] ) ) {
// We left information about shapes, to have proper indexes of images.
ret.push( {
hex: null,
type: null
} );
}
}

return ret;
},

/**
* Method extracts array of src attributes in img tags from given HTML.
* Method extracts array of src attributes in img tags from given HTML. Img tags belong to VML shapes are removed.
* Method base on `data-cke-is-shape="true"` attribute, which is add to shapes by Paste From Word plugin.
*
* CKEDITOR.plugins.pastefromwordimage.extractImgTagsFromHtmlString( html );
* // Returns: [ 'http://example-picture.com/random.png', 'http://example-picture.com/another.png' ]
Expand All @@ -127,15 +122,18 @@
* @returns {String[]} Array of strings represent src attribute of img tags found in `html`.
*/
extractImgTagsFromHtml: function( html ) {
var regexp = /<img[^>]+src="([^"]+)/g,
var regexp = /<img[^>]+src="([^"]+)[^>]+/g,
ret = [],
item;

while ( item = regexp.exec( html ) ) {
ret.push( item[ 1 ] );
if ( item[ 0 ].indexOf( 'data-cke-is-shape="true"' ) === -1 ) {
ret.push( item[ 1 ] );
}
}

return ret;
}

};
} )();
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p style="margin-left:0in; margin-right:0in"></p><table style="width:100%"><tbody><tr><td><div><p>hello world</p></div></td></tr></tbody></table><p><br /></p><p><br /></p><table align="left"><tbody><tr><td><br /></td><td><br /></td><td><br /></td></tr><tr><td><br /></td><td colspan="2" valign="top"><img data-cke-is-shape="true" src="file:///c:\users\foobar\appdata\local\temp\msohtmlclip1\01\clip_image001.png" style="height:106px; width:348px" /></td></tr><tr><td><br /></td></tr><tr><td><br /></td><td valign="top"><img data-cke-is-shape="true" src="file:///c:\users\foobar\appdata\local\temp\msohtmlclip1\01\clip_image002.png" style="height:106px; width:120px" /></td></tr></tbody></table><p style="margin-left:0in; margin-right:0in"><br /></p><p style="margin-left:0in; margin-right:0in"><br /></p><p style="margin-left:0in; margin-right:0in"><br /></p><p style="margin-left:0in; margin-right:0in"><br /></p><p style="margin-left:0in; margin-right:0in"><br /></p><p style="margin-left:0in; margin-right:0in"><br /></p><p style="margin-left:0in; margin-right:0in"><br /></p><p><br /></p>
Loading