8
0
Quellcode durchsuchen

Merge pull request #132 from ckeditor/t/ckeditor5/2036

Internal: Upcast conversion of correct data does not unnecessarily call post-fixer. Closes https://github.com/ckeditor/ckeditor5/issues/2036.
Maciej vor 6 Jahren
Ursprung
Commit
85c89e0bd0
2 geänderte Dateien mit 89 neuen und 3 gelöschten Zeilen
  1. 39 2
      packages/ckeditor5-heading/src/title.js
  2. 50 1
      packages/ckeditor5-heading/tests/title.js

+ 39 - 2
packages/ckeditor5-heading/src/title.js

@@ -88,8 +88,13 @@ export default class Title extends Plugin {
 		editor.editing.mapper.on( 'modelToViewPosition', mapModelPositionToView( editor.editing.view ) );
 		editor.data.mapper.on( 'modelToViewPosition', mapModelPositionToView( editor.editing.view ) );
 
-		// `title-content` <-> `h1` conversion.
-		editor.conversion.elementToElement( { model: 'title-content', view: 'h1' } );
+		// Conversion.
+		editor.conversion.for( 'downcast' ).elementToElement( { model: 'title-content', view: 'h1' } );
+		// Custom converter is used for data v -> m conversion to avoid calling post-fixer when setting data.
+		// See https://github.com/ckeditor/ckeditor5/issues/2036.
+		editor.data.upcastDispatcher.on( 'element:h1', dataViewModelH1Insertion, { priority: 'high' } );
+		editor.data.upcastDispatcher.on( 'element:h2', dataViewModelH1Insertion, { priority: 'high' } );
+		editor.data.upcastDispatcher.on( 'element:h3', dataViewModelH1Insertion, { priority: 'high' } );
 
 		// Take care about correct `title` element structure.
 		model.document.registerPostFixer( writer => this._fixTitleContent( writer ) );
@@ -434,6 +439,38 @@ export default class Title extends Plugin {
 	}
 }
 
+// A view-to-model converter for the h1 that appears at the beginning of the document (a title element).
+//
+// @see module:engine/conversion/upcastdispatcher~UpcastDispatcher#event:element
+// @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event.
+// @param {Object} data An object containing conversion input, a placeholder for conversion output and possibly other values.
+// @param {module:engine/conversion/upcastdispatcher~UpcastConversionApi} conversionApi Conversion interface to be used by the callback.
+function dataViewModelH1Insertion( evt, data, conversionApi ) {
+	const modelCursor = data.modelCursor;
+	const viewItem = data.viewItem;
+
+	if ( !modelCursor.isAtStart || !modelCursor.parent.is( '$root' ) ) {
+		return;
+	}
+
+	if ( !conversionApi.consumable.consume( viewItem, { name: true } ) ) {
+		return;
+	}
+
+	const modelWriter = conversionApi.writer;
+
+	const title = modelWriter.createElement( 'title' );
+	const titleContent = modelWriter.createElement( 'title-content' );
+
+	modelWriter.append( titleContent, title );
+	modelWriter.insert( title, modelCursor );
+
+	conversionApi.convertChildren( viewItem, modelWriter.createPositionAt( titleContent, 0 ) );
+
+	data.modelRange = modelWriter.createRangeOn( title );
+	data.modelCursor = modelWriter.createPositionAt( data.modelRange.end );
+}
+
 // Maps position from the beginning of the model `title` element to the beginning of the view `h1` element.
 //
 // <title>^<title-content>Foo</title-content></title> -> <h1>^Foo</h1>

+ 50 - 1
packages/ckeditor5-heading/tests/title.js

@@ -17,7 +17,7 @@ import Image from '@ckeditor/ckeditor5-image/src/image';
 import ImageUpload from '@ckeditor/ckeditor5-image/src/imageupload';
 import Undo from '@ckeditor/ckeditor5-undo/src/undo';
 
-import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
+import { setData, getData, stringify } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
 
 describe( 'Title', () => {
@@ -77,6 +77,55 @@ describe( 'Title', () => {
 		expect( editor.getData() ).to.equal( '<h1>Foo</h1><p>Bar</p>' );
 	} );
 
+	it( 'should convert h1 to the title if it is the first root child', () => {
+		editor.setData( '<h1>Foo</h1><p>Bar</p>' );
+
+		expect( getData( model ) ).to.equal(
+			'<title><title-content>[]Foo</title-content></title>' +
+			'<paragraph>Bar</paragraph>'
+		);
+	} );
+
+	it( 'should avoid calling post-fixers to parse view to correct model (h1)', () => {
+		const modelFrag = editor.data.parse( '<h1>Foo</h1><p>Bar</p>' );
+
+		expect( stringify( modelFrag ) ).to.equal(
+			'<title><title-content>Foo</title-content></title>' +
+			'<paragraph>Bar</paragraph>'
+		);
+	} );
+
+	it( 'should avoid calling post-fixers to parse view to correct model (h2)', () => {
+		const modelFrag = editor.data.parse( '<h2>Foo</h2><p>Bar</p>' );
+
+		expect( stringify( modelFrag ) ).to.equal(
+			'<title><title-content>Foo</title-content></title>' +
+			'<paragraph>Bar</paragraph>'
+		);
+	} );
+
+	it( 'should avoid calling post-fixers to parse view to correct model (h3)', () => {
+		const modelFrag = editor.data.parse( '<h3>Foo</h3><p>Bar</p>' );
+
+		expect( stringify( modelFrag ) ).to.equal(
+			'<title><title-content>Foo</title-content></title>' +
+			'<paragraph>Bar</paragraph>'
+		);
+	} );
+
+	it( 'should allow to override custom v->m title converter', () => {
+		const spy = sinon.spy();
+
+		editor.data.upcastDispatcher.on( 'element:h1', ( evt, data, api ) => {
+			api.consumable.consume( data.viewItem, { name: true } );
+			spy();
+		}, { priority: 'highest' } );
+
+		editor.setData( '<h1>Foo</h1><p>Bar</p>' );
+
+		sinon.assert.called( spy );
+	} );
+
 	describe( 'model post-fixing', () => {
 		it( 'should set title and content elements', () => {
 			setData( model,