Skip to content

Commit

Permalink
Merge pull request #11667 from ckeditor/cf/4564
Browse files Browse the repository at this point in the history
Fix (html-support): Extracted upcasting attributes of `figure` view element to separate converters for `table`, `image` and `media` integrations. Closes #11688.
  • Loading branch information
niegowski authored May 18, 2022
2 parents 8d59d83 + 9588063 commit b5bd3e9
Show file tree
Hide file tree
Showing 7 changed files with 945 additions and 424 deletions.
8 changes: 4 additions & 4 deletions packages/ckeditor5-html-support/src/datafilter.js
Original file line number Diff line number Diff line change
Expand Up @@ -623,9 +623,8 @@ function consumeAttributeMatches( viewElement, { consumable }, matcher ) {
// We only want to consume attributes, so element can be still processed by other converters.
delete match.match.name;

if ( consumable.consume( viewElement, match.match ) ) {
consumedMatches.push( match );
}
consumable.consume( viewElement, match.match );
consumedMatches.push( match );
}

return consumedMatches;
Expand All @@ -645,7 +644,8 @@ function removeConsumedAttributes( consumable, viewElement, match ) {
continue;
}

for ( const value of attributes ) {
// Iterating over a copy of an array so removing items doesn't influence iteration.
for ( const value of Array.from( attributes ) ) {
if ( !consumable.test( viewElement, ( { [ key ]: [ value ] } ) ) ) {
removeItemFromArray( attributes, value );
}
Expand Down
37 changes: 28 additions & 9 deletions packages/ckeditor5-html-support/src/integrations/image.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ export default class ImageElementSupport extends Plugin {
const conversion = editor.conversion;
const dataFilter = editor.plugins.get( DataFilter );

dataFilter.on( 'register:figure', () => {
conversion.for( 'upcast' ).add( viewToModelFigureAttributeConverter( dataFilter ) );
} );

dataFilter.on( 'register:img', ( evt, definition ) => {
if ( definition.model !== 'imageBlock' && definition.model !== 'imageInline' ) {
return;
Expand Down Expand Up @@ -96,9 +100,7 @@ function viewToModelImageAttributeConverter( dataFilter ) {

preserveElementAttributes( viewImageElement, 'htmlAttributes' );

if ( viewContainerElement.is( 'element', 'figure' ) ) {
preserveElementAttributes( viewContainerElement, 'htmlFigureAttributes' );
} else if ( viewContainerElement.is( 'element', 'a' ) ) {
if ( viewContainerElement.is( 'element', 'a' ) ) {
preserveLinkAttributes( viewContainerElement );
}

Expand All @@ -110,17 +112,34 @@ function viewToModelImageAttributeConverter( dataFilter ) {
}
}

// For a block image, we want to preserve the attributes on our own.
// The inline image attributes will be handled by the GHS automatically.
function preserveLinkAttributes( viewContainerElement ) {
if ( data.modelRange && data.modelRange.getContainedElement().is( 'element', 'imageBlock' ) ) {
preserveElementAttributes( viewContainerElement, 'htmlLinkAttributes' );
}
}
}, { priority: 'low' } );
};
}

// If we're in a link, then the `<figure>` element should be one level higher.
if ( viewContainerElement.parent.is( 'element', 'figure' ) ) {
preserveElementAttributes( viewContainerElement.parent, 'htmlFigureAttributes' );
}
// View-to-model conversion helper preserving allowed attributes on {@link module:image/image~Image Image}
// feature model element from figure view element.
//
// @private
// @param {module:html-support/datafilter~DataFilter} dataFilter
// @returns {Function} Returns a conversion callback.
function viewToModelFigureAttributeConverter( dataFilter ) {
return dispatcher => {
dispatcher.on( 'element:figure', ( evt, data, conversionApi ) => {
const viewFigureElement = data.viewItem;

if ( !data.modelRange || !viewFigureElement.hasClass( 'image' ) ) {
return;
}

const viewAttributes = dataFilter.processViewAttributes( viewFigureElement, conversionApi );

if ( viewAttributes ) {
conversionApi.writer.setAttribute( 'htmlFigureAttributes', viewAttributes, data.modelRange );
}
}, { priority: 'low' } );
};
Expand Down
33 changes: 28 additions & 5 deletions packages/ckeditor5-html-support/src/integrations/mediaembed.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ export default class MediaEmbedElementSupport extends Plugin {
view: mediaElementName
} );

dataFilter.on( 'register:figure', ( ) => {
conversion.for( 'upcast' ).add( viewToModelFigureAttributesConverter( dataFilter ) );
} );

dataFilter.on( `register:${ mediaElementName }`, ( evt, definition ) => {
if ( definition.model !== 'media' ) {
return;
Expand Down Expand Up @@ -71,14 +75,9 @@ function viewToModelMediaAttributesConverter( dataFilter, mediaElementName ) {

function upcastMedia( evt, data, conversionApi ) {
const viewMediaElement = data.viewItem;
const viewParent = viewMediaElement.parent;

preserveElementAttributes( viewMediaElement, 'htmlAttributes' );

if ( viewParent.is( 'element', 'figure' ) && viewParent.hasClass( 'media' ) ) {
preserveElementAttributes( viewParent, 'htmlFigureAttributes' );
}

function preserveElementAttributes( viewElement, attributeName ) {
const viewAttributes = dataFilter.processViewAttributes( viewElement, conversionApi );

Expand All @@ -89,6 +88,30 @@ function viewToModelMediaAttributesConverter( dataFilter, mediaElementName ) {
}
}

// View-to-model conversion helper preserving allowed attributes on {@link module:media-embed/mediaembed~MediaEmbed MediaEmbed}
// feature model element from figure view element.
//
// @private
// @param {module:html-support/datafilter~DataFilter} dataFilter
// @returns {Function} Returns a conversion callback.
function viewToModelFigureAttributesConverter( dataFilter ) {
return dispatcher => {
dispatcher.on( 'element:figure', ( evt, data, conversionApi ) => {
const viewFigureElement = data.viewItem;

if ( !data.modelRange || !viewFigureElement.hasClass( 'media' ) ) {
return;
}

const viewAttributes = dataFilter.processViewAttributes( viewFigureElement, conversionApi );

if ( viewAttributes ) {
conversionApi.writer.setAttribute( 'htmlFigureAttributes', viewAttributes, data.modelRange );
}
}, { priority: 'low' } );
};
}

function modelToViewMediaAttributeConverter( mediaElementName ) {
return dispatcher => {
addAttributeConversionDispatcherHandler( mediaElementName, 'htmlAttributes' );
Expand Down
34 changes: 28 additions & 6 deletions packages/ckeditor5-html-support/src/integrations/table.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import { Plugin } from 'ckeditor5/src/core';
import { setViewAttributes } from '../conversionutils.js';

import DataFilter from '../datafilter';

/**
Expand Down Expand Up @@ -39,6 +38,10 @@ export default class TableElementSupport extends Plugin {
const conversion = editor.conversion;
const dataFilter = editor.plugins.get( DataFilter );

dataFilter.on( 'register:figure', ( ) => {
conversion.for( 'upcast' ).add( viewToModelFigureAttributeConverter( dataFilter ) );
} );

dataFilter.on( 'register:table', ( evt, definition ) => {
if ( definition.model !== 'table' ) {
return;
Expand Down Expand Up @@ -74,11 +77,6 @@ function viewToModelTableAttributeConverter( dataFilter ) {

preserveElementAttributes( viewTableElement, 'htmlAttributes' );

const viewFigureElement = viewTableElement.parent;
if ( viewFigureElement.is( 'element', 'figure' ) ) {
preserveElementAttributes( viewFigureElement, 'htmlFigureAttributes' );
}

for ( const childNode of viewTableElement.getChildren() ) {
if ( childNode.is( 'element', 'thead' ) ) {
preserveElementAttributes( childNode, 'htmlTheadAttributes' );
Expand All @@ -96,6 +94,30 @@ function viewToModelTableAttributeConverter( dataFilter ) {
conversionApi.writer.setAttribute( attributeName, viewAttributes, data.modelRange );
}
}
} );
};
}

// View-to-model conversion helper preserving allowed attributes on {@link module:table/table~Table Table}
// feature model element from figure view element.
//
// @private
// @param {module:html-support/datafilter~DataFilter} dataFilter
// @returns {Function} Returns a conversion callback.
function viewToModelFigureAttributeConverter( dataFilter ) {
return dispatcher => {
dispatcher.on( 'element:figure', ( evt, data, conversionApi ) => {
const viewFigureElement = data.viewItem;

if ( !data.modelRange || !viewFigureElement.hasClass( 'table' ) ) {
return;
}

const viewAttributes = dataFilter.processViewAttributes( viewFigureElement, conversionApi );

if ( viewAttributes ) {
conversionApi.writer.setAttribute( 'htmlFigureAttributes', viewAttributes, data.modelRange );
}
}, { priority: 'low' } );
};
}
Expand Down
Loading

0 comments on commit b5bd3e9

Please sign in to comment.