8
0
Просмотр исходного кода

Added unwraping single AttributeElement to treeView.Writer.unwrap method.

Szymon Kupś 9 лет назад
Родитель
Сommit
59f3f54ab6

+ 65 - 3
packages/ckeditor5-engine/src/treeview/writer.js

@@ -348,13 +348,13 @@ import DocumentFragment from './documentfragment.js';
 		if ( range.end.isEqual( range.start.getShiftedBy( 1 ) ) ) {
 			const node = range.start.nodeAfter;
 
-			if ( node instanceof AttributeElement && wrapAttributes( attribute, node ) ) {
+			if ( node instanceof AttributeElement && wrapAttributeElement( attribute, node ) ) {
 				return range;
 			}
 		}
 
 		// Range is inside single attribute and spans on all children.
-		if ( rangeSpansOnAllChildren( range ) && wrapAttributes( attribute, range.start.parent ) ) {
+		if ( rangeSpansOnAllChildren( range ) && wrapAttributeElement( attribute, range.start.parent ) ) {
 			const parent = range.start.parent.parent;
 			const index = range.start.parent.getIndex();
 
@@ -479,6 +479,18 @@ import DocumentFragment from './documentfragment.js';
 			return range;
 		}
 
+		// Range around one element - check if AttributeElement can be unwrapped partially when it's not similar.
+		// For example:
+		// <b class="foo bar" title="baz"></b> unwrap with:	<b class="foo"></p> result: <b class"bar" title="baz"></b>
+		if ( range.end.isEqual( range.start.getShiftedBy( 1 ) ) ) {
+			const node = range.start.nodeAfter;
+
+			// Unwrap single attribute element.
+			if ( !attribute.isSimilar( node ) && node instanceof AttributeElement && unwrapAttributeElement( attribute, node ) ) {
+				return range;
+			}
+		}
+
 		// Break attributes at range start and end.
 		const { start: breakStart, end: breakEnd } = this.breakRange( range );
 		const parentContainer = breakStart.parent;
@@ -706,7 +718,7 @@ function mergeTextNodes( t1, t2 ) {
 // @param {engine.treeView.AttributeElement} wrapper Wrapper AttributeElement.
 // @param {engine.treeView.AttributeElement} toWrap AttributeElement to wrap using wrapper element.
 // @returns {Boolean} Returns `true` if elements are merged.
-function wrapAttributes( wrapper, toWrap ) {
+function wrapAttributeElement( wrapper, toWrap ) {
 	// Can't merge if name or priority differs.
 	if ( wrapper.name !== toWrap.name || wrapper.priority !== toWrap.priority ) {
 		return false;
@@ -760,6 +772,56 @@ function wrapAttributes( wrapper, toWrap ) {
 	return true;
 }
 
+function unwrapAttributeElement( wrapper, toUnwrap ) {
+	// Can't unwrap if name or priority differs.
+	if ( wrapper.name !== toUnwrap.name || wrapper.priority !== toUnwrap.priority ) {
+		return false;
+	}
+
+	// Check if AttributeElement has all wrapper attributes.
+	for ( let key of wrapper.getAttributeKeys() ) {
+		// Classes and styles should be checked separately.
+		if ( key === 'class' || key === 'style' ) {
+			continue;
+		}
+
+		// If some attributes are different we cannot unwrap.
+		if ( toUnwrap.hasAttribute( key ) && toUnwrap.getAttribute( key ) !== wrapper.getAttribute( key ) ) {
+			return false;
+		}
+	}
+
+	// Check if AttributeElement has all wrapper classes.
+	if ( !toUnwrap.hasClass( ...wrapper.getClassNames() ) ) {
+		return false;
+	}
+
+	// Check if AttributeElement has all wrapper styles.
+	for ( let key of wrapper.getStyleNames() ) {
+		if ( toUnwrap.hasStyle( key ) && toUnwrap.getStyle( key ) !== wrapper.getStyle( key ) ) {
+			return false;
+		}
+	}
+
+	// Remove all wrapper's attributes from unwrapped element.
+	for ( let key of wrapper.getAttributeKeys() ) {
+		// Classes and styles should be checked separately.
+		if ( key === 'class' || key === 'style' ) {
+			continue;
+		}
+
+		toUnwrap.removeAttribute( key );
+	}
+
+	// Remove all wrapper's classes from unwrapped element.
+	toUnwrap.removeClass( ...wrapper.getClassNames() );
+
+	// Remove all wrapper's styles from unwrapped element.
+	toUnwrap.removeStyle( ...wrapper.getStyleNames() );
+
+	return true;
+}
+
 // Returns `true` if range is located in same {@link engine.treeView.AttributeElement AttributeElement}
 // (`start` and `end` positions are located inside same {@link engine.treeView.AttributeElement AttributeElement}),
 // starts on 0 offset and ends after last child node.

+ 6 - 7
packages/ckeditor5-engine/tests/treecontroller/advanced-converters.js

@@ -16,7 +16,6 @@ import ModelRange from '/ckeditor5/engine/treemodel/range.js';
 import ModelPosition from '/ckeditor5/engine/treemodel/position.js';
 import ModelWalker from '/ckeditor5/engine/treemodel/treewalker.js';
 
-import ViewDocumentFragment from '/ckeditor5/engine/treeview/documentfragment.js';
 import ViewElement from '/ckeditor5/engine/treeview/element.js';
 import ViewContainerElement from '/ckeditor5/engine/treeview/containerelement.js';
 import ViewAttributeElement from '/ckeditor5/engine/treeview/attributeelement.js';
@@ -460,7 +459,6 @@ describe( 'custom attribute handling for given element', () => {
 		} );
 		modelDispatcher.on( 'removeAttribute:linkTitle:quote', modelChangeLinkAttrQuoteConverter );
 
-
 		// QUOTE VIEW TO MODEL CONVERTERS
 		viewDispatcher.on( 'element:blockquote', ( evt, data, consumable, conversionApi ) => {
 			if ( consumable.consume( data.input, { name: true } ) ) {
@@ -495,7 +493,7 @@ describe( 'custom attribute handling for given element', () => {
 
 		// The expected value is different than in example or than what is really expected as it should be one link,
 		// but writer does not have merging feature yet.
-		expect( viewToString( viewRoot ) ).to.equal( '<div><a title="Foo title"><a href="foo.html">foo</a></a></div>' );
+		expect( viewToString( viewRoot ) ).to.equal( '<div><a href="foo.html" title="Foo title">foo</a></div>' );
 
 		// Let's change link's attributes.
 		for ( let value of range ) {
@@ -505,7 +503,7 @@ describe( 'custom attribute handling for given element', () => {
 		modelDispatcher.convertAttribute( 'changeAttribute', range, 'linkHref', 'foo.html', 'bar.html' );
 		modelDispatcher.convertAttribute( 'changeAttribute', range, 'linkTitle', 'Foo title', 'Bar title' );
 
-		expect( viewToString( viewRoot ) ).to.equal( '<div><a title="Bar title"><a href="bar.html">foo</a></a></div>' );
+		expect( viewToString( viewRoot ) ).to.equal( '<div><a href="bar.html" title="Bar title">foo</a></div>' );
 
 		// Let's remove a letter from the link.
 		const removed = modelRoot.removeChildren( 0, 1 );
@@ -515,7 +513,7 @@ describe( 'custom attribute handling for given element', () => {
 			ModelRange.createFromElement( modelDoc.graveyard )
 		);
 
-		expect( viewToString( viewRoot ) ).to.equal( '<div><a title="Bar title"><a href="bar.html">oo</a></a></div>' );
+		expect( viewToString( viewRoot ) ).to.equal( '<div><a href="bar.html" title="Bar title">oo</a></div>' );
 
 		range = ModelRange.createFromElement( modelRoot );
 
@@ -591,7 +589,7 @@ describe( 'custom attribute handling for given element', () => {
 					'a',
 					{
 						href: 'foo.html',
-						title:'Foo source'
+						title: 'Foo source'
 					},
 					new ViewText( 'see source' )
 				)
@@ -661,7 +659,8 @@ it( 'default table view to model converter', () => {
 	let model = viewDispatcher.convert( viewTable );
 	let modelFragment = new ModelDocumentFragment( model );
 
-	expect( modelToString( modelFragment ) ).to.equal( '<paragraph>foo <$text linkHref="bar.html">bar</$text></paragraph><paragraph>abc</paragraph>' );
+	expect( modelToString( modelFragment ) )
+		.to.equal( '<paragraph>foo <$text linkHref="bar.html">bar</$text></paragraph><paragraph>abc</paragraph>' );
 } );
 
 // Model converter that converts any non-converted elements and attributes into view elements and attributes.