Skip to content

Commit

Permalink
Merge pull request #16275 from ckeditor/ck/16218
Browse files Browse the repository at this point in the history 
Fix (restricted-editing): Fixed removing an inline image inside an editable region. Closes #16218.
  • Loading branch information
@niegowski
niegowski committed Apr 29, 2024
2 parents ecaeaa9 + 4cd22f5 commit 6a83afa
Show file tree
Hide file tree
Showing 2 changed files with 191 additions and 5 deletions.
55 changes: 51 additions & 4 deletions packages/ckeditor5-restricted-editing/src/restrictededitingmodeediting.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
import type {
DocumentSelection,
Marker,
DowncastAddMarkerEvent,
ModelDeleteContentEvent,
ModelPostFixer,
Range,
Expand Down Expand Up @@ -182,8 +183,54 @@ export default class RestrictedEditingModeEditing extends Plugin {
} ) );

// Currently the marker helpers are tied to other use-cases and do not render a collapsed marker as highlight.
// That's why there are 2 downcast converters for them:
// 1. The default marker-to-highlight will wrap selected text with `<span>`.
// Also, markerToHighlight can not convert marker on an inline object. It handles only text and widgets,
// but it is not a case in the data pipeline. That's why there are 3 downcast converters for them:
//
// 1. The custom inline item (text or inline object) converter (but not the selection).
editor.conversion.for( 'downcast' ).add( dispatcher => {
dispatcher.on<DowncastAddMarkerEvent>( 'addMarker:restrictedEditingException', ( evt, data, conversionApi ): void => {
// Only convert per-item conversion.
if ( !data.item ) {
return;
}

// Do not convert the selection or non-inline items.
if ( data.item.is( 'selection' ) || !conversionApi.schema.isInline( data.item ) ) {
return;
}

if ( !conversionApi.consumable.consume( data.item, evt.name ) ) {
return;
}

const viewWriter = conversionApi.writer;
const viewElement = viewWriter.createAttributeElement(
'span',
{
class: 'restricted-editing-exception'
},
{
id: data.markerName,
priority: -10
}
);

const viewRange = conversionApi.mapper.toViewRange( data.range! );
const rangeAfterWrap = viewWriter.wrap( viewRange, viewElement );

for ( const element of rangeAfterWrap.getItems() ) {
if ( element.is( 'attributeElement' ) && element.isSimilar( viewElement ) ) {
conversionApi.mapper.bindElementToMarker( element, data.markerName );

// One attribute element is enough, because all of them are bound together by the view writer.
// Mapper uses this binding to get all the elements no matter how many of them are registered in the mapper.
break;
}
}
} );
} );

// 2. The marker-to-highlight converter for the document selection.
editor.conversion.for( 'downcast' ).markerToHighlight( {
model: 'restrictedEditingException',
// Use callback to return new object every time new marker instance is created - otherwise it will be seen as the same marker.
Expand All @@ -196,8 +243,8 @@ export default class RestrictedEditingModeEditing extends Plugin {
}
} );

// 2. But for collapsed marker we need to render it as an element.
// Additionally the editing pipeline should always display a collapsed marker.
// 3. And for collapsed marker we need to render it as an element.
// Additionally, the editing pipeline should always display a collapsed marker.
editor.conversion.for( 'editingDowncast' ).markerToElement( {
model: 'restrictedEditingException',
view: ( markerData, { writer } ) => {
Expand Down
141 changes: 140 additions & 1 deletion packages/ckeditor5-restricted-editing/tests/restrictededitingmodeediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ describe( 'RestrictedEditingModeEditing', () => {
describe( 'conversion', () => {
beforeEach( async () => {
editor = await VirtualTestEditor.create( {
plugins: [ Paragraph, TableEditing, RestrictedEditingModeEditing, ClipboardPipeline ]
plugins: [ Paragraph, TableEditing, RestrictedEditingModeEditing, ImageInlineEditing, ClipboardPipeline ]
} );
model = editor.model;
} );
Expand Down Expand Up @@ -333,6 +333,145 @@ describe( 'RestrictedEditingModeEditing', () => {
'</figure>'
);
} );

it( 'inline image should not split span between text nodes', () => {
setModelData( model, '<paragraph>foo <imageInline src="foo/bar.jpg"></imageInline> baz</paragraph>' );

const paragraph = model.document.getRoot().getChild( 0 );

model.change( writer => {
writer.addMarker( 'restrictedEditingException:1', {
range: writer.createRangeIn( paragraph ),
usingOperation: true,
affectsData: true
} );
} );

expect( editor.getData() ).to.equal(
'<p><span class="restricted-editing-exception">foo <img src="foo/bar.jpg">baz</span></p>'
);
expect( getViewData( editor.editing.view, { withoutSelection: true } ) ).to.equal(
'<p>' +
'<span class="restricted-editing-exception restricted-editing-exception_selected">' +
'foo' +
' <span class="ck-widget image-inline" contenteditable="false"><img src="foo/bar.jpg"></img></span>' +
'baz' +
'</span>' +
'</p>'
);
} );

it( 'inline image should not split span between text nodes (inline image at start)', () => {
setModelData( model, '<paragraph><imageInline src="foo/bar.jpg"></imageInline>foo baz</paragraph>' );

const paragraph = model.document.getRoot().getChild( 0 );

model.change( writer => {
writer.addMarker( 'restrictedEditingException:1', {
range: writer.createRangeIn( paragraph ),
usingOperation: true,
affectsData: true
} );
} );

expect( editor.getData() ).to.equal(
'<p><span class="restricted-editing-exception"><img src="foo/bar.jpg">foo baz</span></p>'
);
expect( getViewData( editor.editing.view, { withoutSelection: true } ) ).to.equal(
'<p>' +
'<span class="restricted-editing-exception restricted-editing-exception_selected">' +
'<span class="ck-widget image-inline" contenteditable="false"><img src="foo/bar.jpg"></img></span>' +
'foo ' +
'baz' +
'</span>' +
'</p>'
);
} );

it( 'inline image should not split span between text nodes (inline image at the end)', () => {
setModelData( model, '<paragraph>foo baz<imageInline src="foo/bar.jpg"></imageInline></paragraph>' );

const paragraph = model.document.getRoot().getChild( 0 );

model.change( writer => {
writer.addMarker( 'restrictedEditingException:1', {
range: writer.createRangeIn( paragraph ),
usingOperation: true,
affectsData: true
} );
} );

expect( editor.getData() ).to.equal(
'<p><span class="restricted-editing-exception">foo baz<img src="foo/bar.jpg"></span></p>'
);
expect( getViewData( editor.editing.view, { withoutSelection: true } ) ).to.equal(
'<p>' +
'<span class="restricted-editing-exception restricted-editing-exception_selected">' +
'foo ' +
'baz' +
'<span class="ck-widget image-inline" contenteditable="false"><img src="foo/bar.jpg"></img></span>' +
'</span>' +
'</p>'
);
} );

it( 'should be possible to override marker conversion', () => {
editor.conversion.for( 'downcast' ).add( dispatcher => {
dispatcher.on( 'addMarker:restrictedEditingException', ( evt, data, conversionApi ) => {
if ( !data.item || data.item.is( 'selection' ) || !conversionApi.schema.isInline( data.item ) ) {
return;
}

if ( !conversionApi.consumable.consume( data.item, evt.name ) ) {
return;
}

const viewWriter = conversionApi.writer;
const viewElement = viewWriter.createAttributeElement(
'span',
{
class: 'restricted-editing-custom-exception'
},
{
id: data.markerName,
priority: -10
}
);

const viewRange = conversionApi.mapper.toViewRange( data.range );
const rangeAfterWrap = viewWriter.wrap( viewRange, viewElement );

for ( const element of rangeAfterWrap.getItems() ) {
if ( element.is( 'attributeElement' ) && element.isSimilar( viewElement ) ) {
conversionApi.mapper.bindElementToMarker( element, data.markerName );

// One attribute element is enough, because all of them are bound together by the view writer.
// Mapper uses this binding to get all the elements no matter how many of them are registered in the mapper.
break;
}
}
}, { priority: 'high' } );
} );

setModelData( model, '<paragraph>foo bar baz</paragraph>' );

const paragraph = model.document.getRoot().getChild( 0 );

model.change( writer => {
writer.addMarker( 'restrictedEditingException:1', {
range: writer.createRangeIn( paragraph ),
usingOperation: true,
affectsData: true
} );
} );

expect( editor.getData() ).to.equal(
'<p><span class="restricted-editing-custom-exception">foo bar baz</span></p>'
);
expect( getViewData( editor.editing.view, { withoutSelection: true } ) ).to.equal(
'<p><span class="restricted-editing-custom-exception restricted-editing-exception_selected">foo bar baz</span></p>'
);
} );
} );

describe( 'flattening exception markers', () => {
Expand Down

0 comments on commit 6a83afa

Please sign in to comment.