ソースを参照

Fix: The generic media should have semantic representation in data.

Aleksander Nowodzinski 7 年 前
コミット
6155757b52

+ 1 - 1
packages/ckeditor5-media-embed/src/converters.js

@@ -69,7 +69,7 @@ export function modelToViewUrlAttributeConverter( mediaRegistry, options ) {
 	const mediaViewElementOptions = {
 		useSemanticWrapper: options.semanticDataOutput,
 		renderContent: !options.semanticDataOutput,
-		isViewPipeline: options.isViewPipeline
+		renderForEditingView: options.renderForEditingView
 	};
 
 	return dispatcher => {

+ 2 - 2
packages/ckeditor5-media-embed/src/mediaembedediting.js

@@ -170,7 +170,7 @@ export default class MediaEmbedEditing extends Plugin {
 			view: ( modelElement, viewWriter ) => {
 				const url = modelElement.getAttribute( 'url' );
 				const figure = createMediaFigureElement( viewWriter, mediaRegistry, url, {
-					isViewPipeline: true,
+					renderForEditingView: true,
 					renderContent: true
 				} );
 
@@ -181,7 +181,7 @@ export default class MediaEmbedEditing extends Plugin {
 		// Model -> View (url -> data-oembed-url)
 		conversion.for( 'editingDowncast' ).add(
 			modelToViewUrlAttributeConverter( mediaRegistry, {
-				isViewPipeline: true
+				renderForEditingView: true
 			} ) );
 
 		// View -> Model (data-oembed-url -> url)

+ 16 - 14
packages/ckeditor5-media-embed/src/mediaregistry.js

@@ -21,7 +21,7 @@ export class MediaRegistry {
 		if ( url ) {
 			media = this._getMedia( url );
 		} else {
-			// Generate a view for a renderer–less media.
+			// Generate a view for a renderer–less media. It will render as an empty media.
 			media = new Media( this.editor.t, url );
 		}
 
@@ -33,7 +33,7 @@ export class MediaRegistry {
 
 		url = url.trim();
 
-		for ( let { url: pattern, html: renderer } of mediaProviders ) {
+		for ( let { url: pattern, html: rendererFunction } of mediaProviders ) {
 			if ( !Array.isArray( pattern ) ) {
 				pattern = [ pattern ];
 			}
@@ -42,7 +42,7 @@ export class MediaRegistry {
 				const match = url.match( subPattern );
 
 				if ( match ) {
-					return new Media( this.editor.t, url, match, renderer );
+					return new Media( this.editor.t, url, match, rendererFunction );
 				}
 			}
 		}
@@ -52,18 +52,18 @@ export class MediaRegistry {
 }
 
 class Media {
-	constructor( t, url, match, renderer ) {
+	constructor( t, url, match, rendererFunction ) {
 		this.t = t;
 		this.url = url;
 		this.match = match;
-		this.renderer = renderer;
+		this.rendererFunction = rendererFunction;
 	}
 
 	getViewElement( writer, options ) {
 		let renderFunction;
 
 		if ( options.renderContent ) {
-			const mediaHtml = this._getContentHtml();
+			const mediaHtml = this._getContentHtml( options );
 
 			renderFunction = function( domDocument ) {
 				const domElement = this.toDomElement( domDocument );
@@ -76,7 +76,7 @@ class Media {
 
 		const attributes = {};
 
-		if ( options.useSemanticWrapper ) {
+		if ( options.useSemanticWrapper || ( this.url && !this.rendererFunction && !options.renderForEditingView ) ) {
 			if ( this.url ) {
 				attributes.url = this.url;
 			}
@@ -87,7 +87,7 @@ class Media {
 				attributes[ 'data-oembed-url' ] = this.url;
 			}
 
-			if ( options.isViewPipeline ) {
+			if ( options.renderForEditingView ) {
 				attributes.class = 'ck-media__wrapper';
 			}
 
@@ -95,15 +95,17 @@ class Media {
 		}
 	}
 
-	_getContentHtml() {
-		if ( this.renderer ) {
-			return this.renderer( this.match.pop() );
+	_getContentHtml( options ) {
+		if ( this.rendererFunction ) {
+			return this.rendererFunction( this.match.pop() );
 		} else {
-			if ( !this.url ) {
-				return '';
+			// The placeholder only makes sense for editing view and media which have URLs.
+			// Placeholder is never displayed in data and URL-less media have no content.
+			if ( this.url && options.renderForEditingView ) {
+				return this._getPlaceholderHtml();
 			}
 
-			return this._getPlaceholderHtml();
+			return '';
 		}
 	}
 

+ 62 - 41
packages/ckeditor5-media-embed/tests/mediaembedediting.js

@@ -14,6 +14,9 @@ describe( 'MediaEmbedEditing', () => {
 	let editor, model, doc, view;
 	const mediaDefinitions = [
 		{
+			url: /^https:\/\/generic/
+		},
+		{
 			url: /(.*)/,
 			html: id => `<iframe src="${ id }"></iframe>`
 		}
@@ -219,11 +222,11 @@ describe( 'MediaEmbedEditing', () => {
 
 				describe( 'model to view', () => {
 					it( 'should convert', () => {
-						setModelData( model, '<media url="http://ckeditor.com"></media>' );
+						setModelData( model, '<media url="https://ckeditor.com"></media>' );
 
 						expect( editor.getData() ).to.equal(
 							'<figure class="media">' +
-								'<oembed url="http://ckeditor.com"></oembed>' +
+								'<oembed url="https://ckeditor.com"></oembed>' +
 							'</figure>' );
 					} );
 
@@ -235,14 +238,23 @@ describe( 'MediaEmbedEditing', () => {
 								'<oembed></oembed>' +
 							'</figure>' );
 					} );
+
+					it( 'should convert (generic media)', () => {
+						setModelData( model, '<media url="https://generic"></media>' );
+
+						expect( editor.getData() ).to.equal(
+							'<figure class="media">' +
+								'<oembed url="https://generic"></oembed>' +
+							'</figure>' );
+					} );
 				} );
 
 				describe( 'view to model', () => {
 					it( 'should convert media figure', () => {
-						editor.setData( '<figure class="media"><oembed url="http://ckeditor.com"></oembed></figure>' );
+						editor.setData( '<figure class="media"><oembed url="https://ckeditor.com"></oembed></figure>' );
 
 						expect( getModelData( model, { withoutSelection: true } ) )
-							.to.equal( '<media url="http://ckeditor.com"></media>' );
+							.to.equal( '<media url="https://ckeditor.com"></media>' );
 					} );
 
 					it( 'should not convert if there is no media class', () => {
@@ -284,7 +296,7 @@ describe( 'MediaEmbedEditing', () => {
 						editor.conversion.elementToElement( { model: 'blockquote', view: 'blockquote' } );
 
 						editor.setData(
-							'<blockquote><figure class="media"><oembed url="http://ckeditor.com"></oembed></figure></blockquote>' );
+							'<blockquote><figure class="media"><oembed url="https://ckeditor.com"></oembed></figure></blockquote>' );
 
 						expect( getModelData( model, { withoutSelection: true } ) )
 							.to.equal( '<blockquote></blockquote>' );
@@ -296,7 +308,7 @@ describe( 'MediaEmbedEditing', () => {
 							conversionApi.consumable.consume( img, { name: true } );
 						}, { priority: 'high' } );
 
-						editor.setData( '<figure class="media"><oembed url="http://ckeditor.com"></oembed></figure>' );
+						editor.setData( '<figure class="media"><oembed url="https://ckeditor.com"></oembed></figure>' );
 
 						expect( getModelData( model, { withoutSelection: true } ) )
 							.to.equal( '' );
@@ -307,17 +319,17 @@ describe( 'MediaEmbedEditing', () => {
 							conversionApi.consumable.consume( data.viewItem, { name: true, class: 'image' } );
 						}, { priority: 'high' } );
 
-						editor.setData( '<figure class="media"><oembed url="http://ckeditor.com"></oembed></figure>' );
+						editor.setData( '<figure class="media"><oembed url="https://ckeditor.com"></oembed></figure>' );
 
 						expect( getModelData( model, { withoutSelection: true } ) )
 							.to.equal( '' );
 					} );
 
 					it( 'should discard the contents of the media', () => {
-						editor.setData( '<figure class="media"><oembed url="http://ckeditor.com">foo bar</oembed></figure>' );
+						editor.setData( '<figure class="media"><oembed url="https://ckeditor.com">foo bar</oembed></figure>' );
 
 						expect( getModelData( model, { withoutSelection: true } ) )
-							.to.equal( '<media url="http://ckeditor.com"></media>' );
+							.to.equal( '<media url="https://ckeditor.com"></media>' );
 					} );
 
 					it( 'should not convert unknown media', () => {
@@ -328,7 +340,7 @@ describe( 'MediaEmbedEditing', () => {
 									semanticDataOutput: true,
 									media: [
 										{
-											url: /^http:\/\/known\.media/,
+											url: /^https:\/\/known\.media/,
 											html: () => 'known-media'
 										}
 									]
@@ -336,11 +348,11 @@ describe( 'MediaEmbedEditing', () => {
 							} )
 							.then( newEditor => {
 								newEditor.setData(
-									'<figure class="media"><oembed url="http://unknown.media"></oembed></figure>' +
-									'<figure class="media"><oembed url="http://known.media"></oembed></figure>' );
+									'<figure class="media"><oembed url="https://unknown.media"></oembed></figure>' +
+									'<figure class="media"><oembed url="https://known.media"></oembed></figure>' );
 
 								expect( getModelData( newEditor.model, { withoutSelection: true } ) )
-									.to.equal( '<media url="http://known.media"></media>' );
+									.to.equal( '<media url="https://known.media"></media>' );
 
 								return newEditor.destroy();
 							} );
@@ -368,12 +380,12 @@ describe( 'MediaEmbedEditing', () => {
 				describe( 'conversion in the data pipeline', () => {
 					describe( 'model to view', () => {
 						it( 'should convert', () => {
-							setModelData( model, '<media url="http://ckeditor.com"></media>' );
+							setModelData( model, '<media url="https://ckeditor.com"></media>' );
 
 							expect( editor.getData() ).to.equal(
 								'<figure class="media">' +
-									'<div data-oembed-url="http://ckeditor.com">' +
-										'<iframe src="http://ckeditor.com"></iframe>' +
+									'<div data-oembed-url="https://ckeditor.com">' +
+										'<iframe src="https://ckeditor.com"></iframe>' +
 									'</div>' +
 								'</figure>' );
 						} );
@@ -387,19 +399,28 @@ describe( 'MediaEmbedEditing', () => {
 									'</oembed>' +
 								'</figure>' );
 						} );
+
+						it( 'should convert (generic media)', () => {
+							setModelData( model, '<media url="https://generic"></media>' );
+
+							expect( editor.getData() ).to.equal(
+								'<figure class="media">' +
+									'<oembed url="https://generic"></oembed>' +
+								'</figure>' );
+						} );
 					} );
 
 					describe( 'view to model', () => {
 						it( 'should convert media figure', () => {
 							editor.setData(
 								'<figure class="media">' +
-									'<div data-oembed-url="http://ckeditor.com">' +
-										'<iframe src="http://cksource.com"></iframe>' +
+									'<div data-oembed-url="https://ckeditor.com">' +
+										'<iframe src="https://cksource.com"></iframe>' +
 									'</div>' +
 								'</figure>' );
 
 							expect( getModelData( model, { withoutSelection: true } ) )
-								.to.equal( '<media url="http://ckeditor.com"></media>' );
+								.to.equal( '<media url="https://ckeditor.com"></media>' );
 						} );
 
 						it( 'should not convert if there is no media class', () => {
@@ -436,8 +457,8 @@ describe( 'MediaEmbedEditing', () => {
 							editor.setData(
 								'<div>' +
 									'<figure class="media">' +
-										'<div data-oembed-url="http://ckeditor.com">' +
-											'<iframe src="http://cksource.com"></iframe>' +
+										'<div data-oembed-url="https://ckeditor.com">' +
+											'<iframe src="https://cksource.com"></iframe>' +
 										'</div>' +
 									'</figure>' +
 								'</div>' );
@@ -455,8 +476,8 @@ describe( 'MediaEmbedEditing', () => {
 							editor.setData(
 								'<div>' +
 									'<figure class="media">' +
-										'<div data-oembed-url="http://ckeditor.com">' +
-											'<iframe src="http://cksource.com"></iframe>' +
+										'<div data-oembed-url="https://ckeditor.com">' +
+											'<iframe src="https://cksource.com"></iframe>' +
 										'</div>' +
 									'</figure>' +
 								'</div>' );
@@ -470,7 +491,7 @@ describe( 'MediaEmbedEditing', () => {
 								conversionApi.consumable.consume( data.viewItem, { name: true, class: 'image' } );
 							}, { priority: 'high' } );
 
-							editor.setData( '<figure class="media"><div data-oembed-url="http://ckeditor.com"></div></figure>' );
+							editor.setData( '<figure class="media"><div data-oembed-url="https://ckeditor.com"></div></figure>' );
 
 							expect( getModelData( model, { withoutSelection: true } ) )
 								.to.equal( '' );
@@ -479,13 +500,13 @@ describe( 'MediaEmbedEditing', () => {
 						it( 'should discard the contents of the media', () => {
 							editor.setData(
 								'<figure class="media">' +
-									'<div data-oembed-url="http://ckeditor.com">' +
+									'<div data-oembed-url="https://ckeditor.com">' +
 										'foo bar baz' +
 									'</div>' +
 								'</figure>' );
 
 							expect( getModelData( model, { withoutSelection: true } ) )
-								.to.equal( '<media url="http://ckeditor.com"></media>' );
+								.to.equal( '<media url="https://ckeditor.com"></media>' );
 						} );
 
 						it( 'should not convert unknown media', () => {
@@ -495,7 +516,7 @@ describe( 'MediaEmbedEditing', () => {
 									mediaEmbed: {
 										media: [
 											{
-												url: /^http:\/\/known\.media/,
+												url: /^https:\/\/known\.media/,
 												html: () => 'known-media'
 											}
 										]
@@ -504,16 +525,16 @@ describe( 'MediaEmbedEditing', () => {
 								.then( newEditor => {
 									newEditor.setData(
 										'<figure class="media">' +
-											'<div data-oembed-url="http://known.media">' +
+											'<div data-oembed-url="https://known.media">' +
 											'</div>' +
 										'</figure>' +
 										'<figure class="media">' +
-											'<div data-oembed-url="http://unknown.media">' +
+											'<div data-oembed-url="https://unknown.media">' +
 											'</div>' +
 										'</figure>' );
 
 									expect( getModelData( newEditor.model, { withoutSelection: true } ) )
-										.to.equal( '<media url="http://known.media"></media>' );
+										.to.equal( '<media url="https://known.media"></media>' );
 
 									return newEditor.destroy();
 								} );
@@ -568,36 +589,36 @@ describe( 'MediaEmbedEditing', () => {
 			function test() {
 				describe( 'model to view', () => {
 					it( 'should convert', () => {
-						setModelData( model, '<media url="http://ckeditor.com"></media>' );
+						setModelData( model, '<media url="https://ckeditor.com"></media>' );
 
 						expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
 							'<figure class="ck-widget media" contenteditable="false">' +
-								'<div class="ck-media__wrapper" data-oembed-url="http://ckeditor.com">' +
-									'<iframe src="http://ckeditor.com"></iframe>' +
+								'<div class="ck-media__wrapper" data-oembed-url="https://ckeditor.com">' +
+									'<iframe src="https://ckeditor.com"></iframe>' +
 								'</div>' +
 							'</figure>'
 						);
 					} );
 
 					it( 'should convert the url attribute change', () => {
-						setModelData( model, '<media url="http://ckeditor.com"></media>' );
+						setModelData( model, '<media url="https://ckeditor.com"></media>' );
 						const media = doc.getRoot().getChild( 0 );
 
 						model.change( writer => {
-							writer.setAttribute( 'url', 'http://cksource.com', media );
+							writer.setAttribute( 'url', 'https://cksource.com', media );
 						} );
 
 						expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
 							'<figure class="ck-widget media" contenteditable="false">' +
-								'<div class="ck-media__wrapper" data-oembed-url="http://cksource.com">' +
-									'<iframe src="http://cksource.com"></iframe>' +
+								'<div class="ck-media__wrapper" data-oembed-url="https://cksource.com">' +
+									'<iframe src="https://cksource.com"></iframe>' +
 								'</div>' +
 							'</figure>'
 						);
 					} );
 
 					it( 'should convert the url attribute removal', () => {
-						setModelData( model, '<media url="http://ckeditor.com"></media>' );
+						setModelData( model, '<media url="https://ckeditor.com"></media>' );
 						const media = doc.getRoot().getChild( 0 );
 
 						model.change( writer => {
@@ -614,7 +635,7 @@ describe( 'MediaEmbedEditing', () => {
 					} );
 
 					it( 'should not convert the url attribute removal if is already consumed', () => {
-						setModelData( model, '<media url="http://ckeditor.com"></media>' );
+						setModelData( model, '<media url="https://ckeditor.com"></media>' );
 						const media = doc.getRoot().getChild( 0 );
 
 						editor.editing.downcastDispatcher.on( 'attribute:url:media', ( evt, data, conversionApi ) => {
@@ -627,8 +648,8 @@ describe( 'MediaEmbedEditing', () => {
 
 						expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
 							'<figure class="ck-widget media" contenteditable="false">' +
-								'<div class="ck-media__wrapper" data-oembed-url="http://ckeditor.com">' +
-									'<iframe src="http://ckeditor.com"></iframe>' +
+								'<div class="ck-media__wrapper" data-oembed-url="https://ckeditor.com">' +
+									'<iframe src="https://ckeditor.com"></iframe>' +
 								'</div>' +
 							'</figure>'
 						);