Skip to content

Commit

Permalink
Pasting an inline widget as plain text no longer inserts additional e…
Browse files Browse the repository at this point in the history
…mpty lines before and after widget text.
  • Loading branch information
Mati365 committed Sep 12, 2024
1 parent 88e555d commit 66bf853
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 15 deletions.
2 changes: 1 addition & 1 deletion packages/ckeditor5-clipboard/src/clipboardpipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ export default class ClipboardPipeline extends Plugin {
this.listenTo<ViewDocumentClipboardOutputEvent>( viewDocument, 'clipboardOutput', ( evt, data ) => {
if ( !data.content.isEmpty ) {
data.dataTransfer.setData( 'text/html', this.editor.data.htmlProcessor.toData( data.content ) );
data.dataTransfer.setData( 'text/plain', viewToPlainText( data.content ) );
data.dataTransfer.setData( 'text/plain', viewToPlainText( editor.editing.view.domConverter, data.content ) );
}

if ( data.method == 'cut' ) {
Expand Down
16 changes: 13 additions & 3 deletions packages/ckeditor5-clipboard/src/utils/viewtoplaintext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @module clipboard/utils/viewtoplaintext
*/

import type { ViewDocumentFragment, ViewElement, ViewItem } from '@ckeditor/ckeditor5-engine';
import type { DomConverter, ViewDocumentFragment, ViewElement, ViewItem } from '@ckeditor/ckeditor5-engine';

// Elements which should not have empty-line padding.
// Most `view.ContainerElement` want to be separate by new-line, but some are creating one structure
Expand All @@ -19,10 +19,14 @@ const listElements = [ 'ol', 'ul' ];
/**
* Converts {@link module:engine/view/item~Item view item} and all of its children to plain text.
*
* @param converter The converter instance.
* @param viewItem View item to convert.
* @returns Plain text representation of `viewItem`.
*/
export default function viewToPlainText( viewItem: ViewItem | ViewDocumentFragment ): string {
export default function viewToPlainText(
converter: DomConverter,
viewItem: ViewItem | ViewDocumentFragment
): string {
if ( viewItem.is( '$text' ) || viewItem.is( '$textProxy' ) ) {
return viewItem.data;
}
Expand All @@ -44,7 +48,7 @@ export default function viewToPlainText( viewItem: ViewItem | ViewDocumentFragme
let prev: ViewElement | null = null;

for ( const child of ( viewItem as ViewElement | ViewDocumentFragment ).getChildren() ) {
text += newLinePadding( child as ViewElement, prev ) + viewToPlainText( child );
text += newLinePadding( converter, child as ViewElement, prev ) + viewToPlainText( converter, child );
prev = child as ViewElement;
}

Expand All @@ -55,6 +59,7 @@ export default function viewToPlainText( viewItem: ViewItem | ViewDocumentFragme
* Returns new line padding to prefix the given elements with.
*/
function newLinePadding(
converter: DomConverter,
element: ViewElement,
previous: ViewElement | null
): string {
Expand Down Expand Up @@ -94,6 +99,11 @@ function newLinePadding(
return '';
}

if ( !converter.isBlockViewElement( previous ) && !converter.isBlockViewElement( element ) ) {
// Don't add padding between non-block elements.
return '';
}

// Add empty lines between container elements.
return '\n\n';
}
20 changes: 18 additions & 2 deletions packages/ckeditor5-clipboard/tests/utils/viewtoplaintext.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,23 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

import { DomConverter, StylesProcessor, ViewDocument } from '@ckeditor/ckeditor5-engine';
import viewToPlainText from '../../src/utils/viewtoplaintext.js';

import { parse as parseView } from '@ckeditor/ckeditor5-engine/src/dev-utils/view.js';

describe( 'viewToPlainText()', () => {
let converter;

beforeEach( () => {
const viewDocument = new ViewDocument( new StylesProcessor() );

converter = new DomConverter( viewDocument );
} );

function testViewToPlainText( viewString, expectedText ) {
const view = parseView( viewString );
const text = viewToPlainText( view );
const text = viewToPlainText( converter, view );

expect( text ).to.equal( expectedText );
}
Expand All @@ -34,14 +43,21 @@ describe( 'viewToPlainText()', () => {
);
} );

it( 'should not put empty line between inline container elements', () => {
testViewToPlainText(
'<container:strong>Foo</container:strong><container:strong>Bar</container:strong>',
'FooBar'
);
} );

it( 'should not put empty line before or after the element with `dataPipeline:transparentRendering` property', () => {
const viewString = 'Abc <container:h1>Header</container:h1> xyz';
const expectedText = 'Abc Header xyz';

const view = parseView( viewString );
view.getChild( 1 )._setCustomProperty( 'dataPipeline:transparentRendering', true );

const text = viewToPlainText( view );
const text = viewToPlainText( converter, view );

expect( text ).to.equal( expectedText );
} );
Expand Down
18 changes: 9 additions & 9 deletions packages/ckeditor5-engine/src/view/domconverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ export default class DomConverter {

if ( viewChild !== null ) {
// Whitespace cleaning before entering a block element (between block elements).
if ( this._isBlockViewElement( viewChild ) ) {
if ( this.isBlockViewElement( viewChild ) ) {
this._processDomInlineNodes( domElement, inlineNodes, options );
}

Expand Down Expand Up @@ -1162,6 +1162,13 @@ export default class DomConverter {
return node && node.nodeType == Node.DOCUMENT_FRAGMENT_NODE;
}

/**
* Returns `true` if a view node belongs to {@link #blockElements}. `false` otherwise.
*/
public isBlockViewElement( node: ViewNode ): boolean {
return node.is( 'element' ) && this.blockElements.includes( node.name );
}

/**
* Checks if the node is an instance of the block filler for this DOM converter.
*
Expand Down Expand Up @@ -1443,7 +1450,7 @@ export default class DomConverter {
if ( this._isViewElementWithRawContent( viewElement, options ) ) {
viewElement._setCustomProperty( '$rawContent', ( domNode as DomElement ).innerHTML );

if ( !this._isBlockViewElement( viewElement ) ) {
if ( !this.isBlockViewElement( viewElement ) ) {
inlineNodes.push( viewElement );
}

Expand Down Expand Up @@ -1743,13 +1750,6 @@ export default class DomConverter {
return this.isElement( node ) && this.blockElements.includes( node.tagName.toLowerCase() );
}

/**
* Returns `true` if a view node belongs to {@link #blockElements}. `false` otherwise.
*/
private _isBlockViewElement( node: ViewNode ): boolean {
return node.is( 'element' ) && this.blockElements.includes( node.name );
}

/**
* Returns `true` if a DOM node belongs to {@link #inlineObjectElements}. `false` otherwise.
*/
Expand Down
28 changes: 28 additions & 0 deletions packages/ckeditor5-engine/tests/view/domconverter/domconverter.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { StylesProcessor } from '../../../src/view/stylesmap.js';
import ViewPosition from '../../../src/view/position.js';
import ViewRange from '../../../src/view/range.js';
import { ViewText } from '@ckeditor/ckeditor5-engine';
import { parse as parseView } from '@ckeditor/ckeditor5-engine/src/dev-utils/view.js';

describe( 'DomConverter', () => {
let converter, viewDocument;
Expand Down Expand Up @@ -425,6 +426,33 @@ describe( 'DomConverter', () => {
} );
} );

describe( 'isBlockViewElement()', () => {
const blockElements = [
'address', 'article', 'aside', 'blockquote', 'caption', 'center', 'dd', 'details', 'dir', 'div',
'dl', 'dt', 'fieldset', 'figcaption', 'figure', 'footer', 'form', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'header',
'hgroup', 'legend', 'li', 'main', 'menu', 'nav', 'ol', 'p', 'pre', 'section', 'summary', 'table', 'tbody',
'td', 'tfoot', 'th', 'thead', 'tr', 'ul'
];

const inlineElements = [ 'span', 'i', 'b', 'strong', 'a' ];

for ( const elementName of blockElements ) {
it( `should return true for <${ elementName }>`, () => {
const view = parseView( `<${ elementName }></${ elementName }>` );

expect( converter.isBlockViewElement( view ) ).to.be.true;
} );
}

for ( const elementName of inlineElements ) {
it( `should return false for <${ elementName }>`, () => {
const view = parseView( `<${ elementName }></${ elementName }>` );

expect( converter.isBlockViewElement( view ) ).to.be.false;
} );
}
} );

describe( 'shouldRenderAttribute()', () => {
it( 'should allow all in data pipeline', () => {
expect( converter.shouldRenderAttribute( 'onclick', 'anything' ) ).to.be.false;
Expand Down

0 comments on commit 66bf853

Please sign in to comment.