Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #192 from ckeditor/t/191
Browse files Browse the repository at this point in the history
Fixed: Table cell view post-fixer will not crash if an element inside a cell got attribute and was removed at the same time. Closes #191.
  • Loading branch information
Piotr Jasiun authored May 20, 2019
2 parents 00848a8 + 30ce2c5 commit 900c178
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
6 changes: 6 additions & 0 deletions src/converters/tablecell-post-fixer.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,12 @@ function getElementsToCheck( view ) {
// - span: for single paragraph with no attributes.
// - p : in other cases.
function ensureProperElementName( currentViewElement, mapper, writer ) {
// This situation may happen if a view element was changed and removed at the same time.
// In this case, the view element is already unbound so the post-fixer would crash.
if ( !currentViewElement.root.is( 'rootElement' ) ) {
return false;
}

const modelParagraph = mapper.toModelElement( currentViewElement );
const expectedViewElementName = getExpectedElementName( modelParagraph.parent, modelParagraph );

Expand Down
22 changes: 21 additions & 1 deletion tests/converters/tablecell-post-fixer.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import env from '@ckeditor/ckeditor5-utils/src/env';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';

import Delete from '@ckeditor/ckeditor5-typing/src/delete';

describe( 'TableCell post-fixer', () => {
let editor, model, doc, root, view;

Expand All @@ -26,7 +28,7 @@ describe( 'TableCell post-fixer', () => {
// Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`.
testUtils.sinon.stub( env, 'isEdge' ).get( () => false );

return ClassicTestEditor.create( element )
return ClassicTestEditor.create( element, { extraPlugins: [ Delete ] } )
.then( newEditor => {
editor = newEditor;
model = editor.model;
Expand Down Expand Up @@ -276,4 +278,22 @@ describe( 'TableCell post-fixer', () => {
// Trying to map view selection to DOM range shouldn't throw after post-fixer will fix inserted <p> to <span>.
expect( () => view.domConverter.viewRangeToDom( viewRange ) ).to.not.throw();
} );

// https://github.com/ckeditor/ckeditor5-table/issues/191.
it( 'should not fire (and crash) for removed view elements', () => {
editor.setData( viewTable( [ [ '<p>foo</p>' ] ] ) );

const p = root.getNodeByPath( [ 0, 0, 0, 0 ] );

// Replace table cell contents with paragraph - as model.deleteContent() does.
model.change( writer => {
writer.setSelection( writer.createRangeIn( root ) );
editor.execute( 'delete' ); // For some reason it didn't crash with `writer.remove()`.

writer.setAttribute( 'foo', 'bar', p );
} );

// Trying to map view selection to DOM range shouldn't throw after post-fixer will fix inserted <p> to <span>.
expect( editor.getData() ).to.equal( '' );
} );
} );

0 comments on commit 900c178

Please sign in to comment.