Skip to content

Commit

Permalink
Merge pull request #15634 from ckeditor/ck/15527
Browse files Browse the repository at this point in the history
Fix (html-support): The editor should not stuck in an infinite post-fixing loop while modifying a list structure inside a GHS element. Closes #15527. Closes #15565.
  • Loading branch information
niegowski committed Jan 5, 2024
2 parents 51fd2e8 + 081d782 commit 7d600e2
Show file tree
Hide file tree
Showing 2 changed files with 142 additions and 2 deletions.
2 changes: 1 addition & 1 deletion packages/ckeditor5-html-support/src/datafilter.ts
Expand Up @@ -461,7 +461,7 @@ export default class DataFilter extends Plugin {
}

// Remove the coupled GHS attributes on the same range as the feature attribute was removed.
for ( const { item } of change.range.getWalker( { shallow: true } ) ) {
for ( const { item } of change.range.getWalker() ) {
for ( const attributeKey of attributeKeys ) {
if ( item.hasAttribute( attributeKey ) ) {
writer.removeAttribute( attributeKey, item );
Expand Down
142 changes: 141 additions & 1 deletion packages/ckeditor5-html-support/tests/integrations/list.js
Expand Up @@ -8,6 +8,7 @@ import GeneralHtmlSupport from '../../src/generalhtmlsupport.js';
import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor.js';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph.js';
import ListEditing from '@ckeditor/ckeditor5-list/src/list/listediting.js';
import TableEditing from '@ckeditor/ckeditor5-table/src/tableediting.js';

import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils.js';
import stubUid from '@ckeditor/ckeditor5-list/tests/list/_utils/uid.js';
Expand All @@ -28,7 +29,7 @@ describe( 'ListElementSupport', () => {

editor = await ClassicTestEditor
.create( editorElement, {
plugins: [ Paragraph, GeneralHtmlSupport, ListEditing ]
plugins: [ Paragraph, GeneralHtmlSupport, ListEditing, TableEditing ]
} );
model = editor.model;
dataFilter = editor.plugins.get( 'DataFilter' );
Expand Down Expand Up @@ -1013,6 +1014,145 @@ describe( 'ListElementSupport', () => {
} );
} );

describe( '#15527 and #15565', () => {
// See https://github.com/ckeditor/ckeditor5/issues/15527.
it( 'should not remove attribute from other elements (inside GHS div)', () => {
const dataFilter = editor.plugins.get( 'DataFilter' );

// Allow everything
dataFilter.allowElement( /^.*$/ );
dataFilter.allowAttributes( { name: /^.*$/, attributes: true } );
dataFilter.allowAttributes( { name: /^.*$/, classes: true } );

editor.setData(
'<div>' +
'<ol>' +
'<li>foo</li>' +
'<li>' +
'<div>' +
'<ol>' +
'<li>bar</li>' +
'</ol>' +
'</div>' +
'</li>' +
'</ol>' +
'</div>'
);

model.change( writer => {
writer.setSelection( model.document.getRoot().getNodeByPath( [ 0, 1, 0 ] ), 0 );
} );

expect( getModelDataWithAttributes( model ) ).to.deep.equal( {
data:
'<htmlDiv>' +
'<paragraph htmlLiAttributes="(1)" htmlOlAttributes="(2)" listIndent="0" listItemId="a00" listType="numbered">' +
'foo' +
'</paragraph>' +
'<htmlDiv htmlLiAttributes="(3)" htmlOlAttributes="(4)" listIndent="0" listItemId="a02" listType="numbered">' +
'<paragraph' +
' htmlLiAttributes="(5)" htmlOlAttributes="(6)" listIndent="0" listItemId="a01" listType="numbered">' +
'[]bar' +
'</paragraph>' +
'</htmlDiv>' +
'</htmlDiv>',
attributes: {
1: {},
2: {},
3: {},
4: {},
5: {},
6: {}
}
} );

editor.editing.view.document.fire( 'delete', { direction: 'backward', preventDefault() {} } );

expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( {
data:
'<htmlDiv>' +
'<paragraph htmlLiAttributes="(1)" htmlOlAttributes="(2)" listIndent="0" listItemId="a00" listType="numbered">' +
'foo' +
'</paragraph>' +
'<htmlDiv htmlLiAttributes="(3)" htmlOlAttributes="(4)" listIndent="0" listItemId="a02" listType="numbered">' +
'<paragraph>bar</paragraph>' +
'</htmlDiv>' +
'</htmlDiv>',
attributes: {
1: {},
2: {},
3: {},
4: {}
}
} );
} );

// See https://github.com/ckeditor/ckeditor5/issues/15565.
it( 'should not remove attribute from other elements (inside table cell)', () => {
const dataFilter = editor.plugins.get( 'DataFilter' );

// Allow everything
dataFilter.allowElement( /^.*$/ );
dataFilter.allowAttributes( { name: /^.*$/, attributes: true } );
dataFilter.allowAttributes( { name: /^.*$/, classes: true } );

editor.setData(
'<ul>' +
'<li>' +
'<p><br>&nbsp;</p>' +
'<figure class="table">' +
'<table>' +
'<tbody>' +
'<tr>' +
'<td>' +
'<ul><li>&nbsp;</li></ul>' +
'</td>' +
'</tr>' +
'</tbody>' +
'</table>' +
'</figure>' +
'</li>' +
'</ul>'
);

model.change( writer => {
writer.setSelection( model.document.getRoot().getNodeByPath( [ 1, 0, 0, 0 ] ), 0 );
} );

expect( getModelData( model ) ).to.equal(
'<paragraph htmlLiAttributes="{}" htmlUlAttributes="{}" listIndent="0" listItemId="a01" listType="bulleted">' +
'<htmlCustomElement htmlContent="" htmlElementName="br"></htmlCustomElement> ' +
'</paragraph>' +
'<table htmlLiAttributes="{}" htmlUlAttributes="{}" listIndent="0" listItemId="a01" listType="bulleted">' +
'<tableRow>' +
'<tableCell>' +
'<paragraph htmlLiAttributes="{}" htmlUlAttributes="{}" listIndent="0" listItemId="a00" listType="bulleted">' +
'[]' +
'</paragraph>' +
'</tableCell>' +
'</tableRow>' +
'</table>'
);

editor.editing.view.document.fire( 'enter', { preventDefault() {} } );

expect( getModelData( model ) ).to.equal(
'<paragraph htmlLiAttributes="{}" htmlUlAttributes="{}" listIndent="0" listItemId="a01" listType="bulleted">' +
'<htmlCustomElement htmlContent="" htmlElementName="br"></htmlCustomElement> ' +
'</paragraph>' +
'<table htmlLiAttributes="{}" htmlUlAttributes="{}" listIndent="0" listItemId="a01" listType="bulleted">' +
'<tableRow>' +
'<tableCell>' +
'<paragraph>' +
'[]' +
'</paragraph>' +
'</tableCell>' +
'</tableRow>' +
'</table>'
);
} );
} );

function paragraph( text, id, indent, type, listAttributes ) {
const attributeName = type === 'bulleted' ?
'htmlUlAttributes' :
Expand Down

0 comments on commit 7d600e2

Please sign in to comment.