Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabled support for nested block quotes #9382

Merged
merged 5 commits into from
Apr 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion docs/builds/guides/migration/migration-to-28.0.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,10 @@ Listed below are the most important changes that require your attention when upg

Prior to version 28.0.0 inserting a table into another table was not allowed.

If you wish to bring back this restriction see {@link features/table#disallowing-nesting-tables "Disallowing nesting tables"}.
If you wish to bring back this restriction see {@link features/table#disallowing-nesting-tables "Disallowing nesting tables"}.

## Disallowing nesting block quotes

Prior to version 28.0.0 inserting block quote into a block quote was not allowed.

If you wish to bring back the old behavior see {@link features/block-quote#disallowing-nesting-block-quotes "Disallowing nesting block quotes"}.
27 changes: 27 additions & 0 deletions packages/ckeditor5-block-quote/docs/features/block-quote.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,33 @@ ClassicEditor
Read more about {@link builds/guides/integration/installing-plugins installing plugins}.
</info-box>

## Disallowing nesting block quotes

By default, the editor supports a block quote inserted into another block quote.

In order to disallow nesting block quotes you need to register an additional schema rule. It needs to be added before the data gets loaded into the editor, hence it is best to implement it as a plugin:

```js
function DisallowNestingBlockQuotes( editor ) {
editor.model.schema.addChildCheck( ( context, childDefinition ) => {
if ( context.endsWith( 'blockQuote' ) && childDefinition.name == 'blockQuote' ) {
return false;
}
} );
}

// Pass it via config.extraPlugins or config.plugins:

ClassicEditor
.create( document.querySelector( '#editor' ), {
extraPlugins: [ DisallowNestingBlockQuotes ],

// The rest of the config.
} )
.then( ... )
.catch( ... );
```

## Common API

The {@link module:block-quote/blockquote~BlockQuote} plugin registers:
Expand Down
12 changes: 2 additions & 10 deletions packages/ckeditor5-block-quote/src/blockquoteediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,6 @@ export default class BlockQuoteEditing extends Plugin {
allowContentOf: '$root'
} );

// Disallow blockQuote in blockQuote.
schema.addChildCheck( ( ctx, childDef ) => {
if ( ctx.endsWith( 'blockQuote' ) && childDef.name == 'blockQuote' ) {
return false;
}
} );

editor.conversion.elementToElement( { model: 'blockQuote', view: 'blockquote' } );

// Postfixer which cleans incorrect model states connected with block quotes.
Expand All @@ -77,13 +70,12 @@ export default class BlockQuoteEditing extends Plugin {

return true;
} else if ( element.is( 'element', 'blockQuote' ) && !schema.checkChild( entry.position, element ) ) {
// Added a blockQuote in incorrect place - most likely inside another blockQuote. Unwrap it
// so the content inside is not lost.
// Added a blockQuote in incorrect place. Unwrap it so the content inside is not lost.
writer.unwrap( element );

return true;
} else if ( element.is( 'element' ) ) {
// Just added an element. Check its children to see if there are no nested blockQuotes somewhere inside.
// Just added an element. Check that all children meet the scheme rules.
const range = writer.createRangeIn( element );

for ( const child of range.getItems() ) {
Expand Down
52 changes: 46 additions & 6 deletions packages/ckeditor5-block-quote/tests/blockquoteediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ describe( 'BlockQuoteEditing', () => {
expect( model.schema.checkChild( [ '$root', 'blockQuote' ], 'paragraph' ) ).to.be.true;
} );

it( 'does not allow for blockQuote in blockQuote', () => {
expect( model.schema.checkChild( [ '$root', 'blockQuote' ], 'blockQuote' ) ).to.be.false;
it( 'allows for blockQuote in blockQuote', () => {
expect( model.schema.checkChild( [ '$root', 'blockQuote' ], 'blockQuote' ) ).to.be.true;
} );

it( 'does not break when checking an unregisterd item', () => {
Expand Down Expand Up @@ -102,7 +102,7 @@ describe( 'BlockQuoteEditing', () => {
expect( editor.getData( { trim: 'none' } ) ).to.equal( '<p>&nbsp;</p>' ); // Autoparagraphed.
} );

it( 'should unwrap a blockQuote if it was inserted into another blockQuote', () => {
it( 'should not unwrap a blockQuote if it was inserted into another blockQuote', () => {
setModelData( model, '<blockQuote><paragraph>Foo</paragraph></blockQuote>' );

model.change( writer => {
Expand All @@ -115,10 +115,10 @@ describe( 'BlockQuoteEditing', () => {
writer.insert( bq, root.getChild( 0 ), 1 ); // Insert after <p>Foo</p>.
} );

expect( editor.getData() ).to.equal( '<blockquote><p>Foo</p><p>Bar</p></blockquote>' );
expect( editor.getData() ).to.equal( '<blockquote><p>Foo</p><blockquote><p>Bar</p></blockquote></blockquote>' );
} );

it( 'should unwrap nested blockQuote if it was wrapped into another blockQuote', () => {
it( 'should not unwrap nested blockQuote if it was wrapped into another blockQuote', () => {
setModelData( model, '<blockQuote><paragraph>Foo</paragraph></blockQuote><paragraph>Bar</paragraph>' );

model.change( writer => {
Expand All @@ -127,7 +127,7 @@ describe( 'BlockQuoteEditing', () => {
writer.wrap( writer.createRangeIn( root ), 'blockQuote' );
} );

expect( editor.getData() ).to.equal( '<blockquote><p>Foo</p><p>Bar</p></blockquote>' );
expect( editor.getData() ).to.equal( '<blockquote><blockquote><p>Foo</p></blockquote><p>Bar</p></blockquote>' );
} );

it( 'postfixer should do nothing on attribute change', () => {
Expand All @@ -143,4 +143,44 @@ describe( 'BlockQuoteEditing', () => {

expect( editor.getData() ).to.equal( '<blockquote><p><strong>Foo</strong></p></blockquote>' );
} );

describe( 'nested blockQuote forbidden by custom rule', () => {
// Nested block quotes are supported since https://github.com/ckeditor/ckeditor5/issues/9210, so let's check
// if the editor will not blow up in case nested block quotes are forbidden by custom scheme rule.
beforeEach( () => {
model.schema.addChildCheck( ( ctx, childDef ) => {
if ( ctx.endsWith( 'blockQuote' ) && childDef.name == 'blockQuote' ) {
return false;
}
} );
} );

it( 'should unwrap a blockQuote if it was inserted into another blockQuote', () => {
setModelData( model, '<blockQuote><paragraph>Foo</paragraph></blockQuote>' );

model.change( writer => {
const root = model.document.getRoot();
const bq = writer.createElement( 'blockQuote' );
const p = writer.createElement( 'paragraph' );

writer.insertText( 'Bar', p, 0 ); // <p>Bar</p>.
writer.insert( p, bq, 0 ); // <blockquote><p>Bar</p></blockquote>.
writer.insert( bq, root.getChild( 0 ), 1 ); // Insert after <p>Foo</p>.
} );

expect( editor.getData() ).to.equal( '<blockquote><p>Foo</p><p>Bar</p></blockquote>' );
} );

it( 'should unwrap nested blockQuote if it was wrapped into another blockQuote', () => {
setModelData( model, '<blockQuote><paragraph>Foo</paragraph></blockQuote><paragraph>Bar</paragraph>' );

model.change( writer => {
const root = model.document.getRoot();

writer.wrap( writer.createRangeIn( root ), 'blockQuote' );
} );

expect( editor.getData() ).to.equal( '<blockquote><p>Foo</p><p>Bar</p></blockquote>' );
} );
} );
} );
7 changes: 7 additions & 0 deletions packages/ckeditor5-block-quote/tests/manual/blockquote.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,11 @@
</ul>
</blockquote>
<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum.</p>
<blockquote>
<p>Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris.</p>
<blockquote>
<p>Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum.</p>
</blockquote>
</blockquote>
<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum.</p>
</div>
3 changes: 2 additions & 1 deletion packages/ckeditor5-block-quote/tests/manual/blockquote.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ Check block quote related behaviors:
* <kbd>Backspace</kbd>,
* undo/redo,
* applying headings and lists,
* stability when used with nested lists.
* stability when used with nested lists,
* stability when used with nested block quotes.
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<style>
body {
font-family: Helvetica, Arial, sans-serif;
font-size: 14px;
}
</style>

<div id="editor">
<p>Nested block quotes (in the data):</p>

<blockquote>
<p>Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris.</p>
<blockquote>
<p>Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum.</p>
<blockquote>
<p>Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum.</p>
</blockquote>
</blockquote>
</blockquote>
</div>
35 changes: 35 additions & 0 deletions packages/ckeditor5-block-quote/tests/manual/blockquotenonesting.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* @license Copyright (c) 2003-2021, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/* globals console, window, document */

import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';

function DisallowNestingBlockQuotes( editor ) {
editor.model.schema.addChildCheck( ( context, childDefinition ) => {
if ( context.endsWith( 'blockQuote' ) && childDefinition.name == 'blockQuote' ) {
return false;
}
} );
}

ClassicEditor
.create( document.querySelector( '#editor' ), {
plugins: [ ArticlePluginSet, DisallowNestingBlockQuotes ],
toolbar: [
'heading', '|', 'insertTable', '|', 'bold', 'italic', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo'
],
table: {
contentToolbar: [ 'tableColumn', 'tableRow', 'mergeTableCells' ],
tableToolbar: [ 'bold', 'italic' ]
}
} )
.then( editor => {
window.editor = editor;
} )
.catch( err => {
console.error( err.stack );
} );
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
* The nested block quote present in the data should be "unnested" ("unwrapped") on data load. All block quotes (the top level block quote and all nested ones) should be converted to siblings in editor's root.
* It should not be possible to insert a block quote into another block quote as a direct child in any way (UI, paste, d&d, etc.). It is allowed to have nested block quotes indirectly, i.e. a block quote inside a table, which is inside another block quote.
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ The {@link module:engine/model/schema~Schema#checkChild `Schema#checkChild()`} m

These listeners can be added either by listening directly to the {@link module:engine/model/schema~Schema#event:checkChild} event or by using the handy {@link module:engine/model/schema~Schema#addChildCheck `Schema#addChildCheck()`} method.

For instance, the block quote feature defines such a listener to disallow nested `<blockQuote>` structures:
For instance, to disallow nested `<blockQuote>` structures, you can define such a listener:

```js
schema.addChildCheck( ( context, childDefinition ) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,9 @@ describe( 'transform', () => {
expectClients(
'<blockQuote>' +
'<paragraph>A</paragraph>' +
'<paragraph>B</paragraph>' +
'<blockQuote>' +
'<paragraph>B</paragraph>' +
'</blockQuote>' +
'<paragraph>C</paragraph>' +
'</blockQuote>'
);
Expand Down