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

Crash when copying nested table which was pasted into the editor #8917

Closed
FilipTokarski opened this issue Jan 25, 2021 · 1 comment · Fixed by #8942
Closed

Crash when copying nested table which was pasted into the editor #8917

FilipTokarski opened this issue Jan 25, 2021 · 1 comment · Fixed by #8942
Assignees
Labels
package:table squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@FilipTokarski
Copy link
Member

FilipTokarski commented Jan 25, 2021

📝 Provide detailed reproduction steps (if any)

  1. Enable nested tables with the code:
editor.model.schema.on( 'checkChild', ( evt, args ) => {
	const context = args[ 0 ];
	const childDefinition = args[ 1 ];

	if ( context.endsWith( 'tableCell' ) && childDefinition && childDefinition.name == 'table' ) {
		// Prevent next listeners from being called.
		evt.stop();
		// Set the checkChild()'s return value.
		evt.return = true;
	}
}, { priority: 'highest' } );
  1. Create a table with 1 table inside
  2. Copy and paste thes table somewhere in the editor
  3. Select the pasted table and press ctrl / cmd + c

✔️ Expected result

The second table is copied, no errors.

❌ Actual result

Error in the console:

Uncaught TypeError: Cannot read property 'getAttributeKeys' of null
    at hasAnyAttribute (downcast.js:523)
    at createViewTableCellElement (downcast.js:382)
    at DowncastDispatcher.<anonymous> (downcast.js:69)
    at DowncastDispatcher.fire (emittermixin.js:221)
    at DowncastDispatcher._testAndFire (downcastdispatcher.js:560)
    at DowncastDispatcher._convertInsertWithAttributes (downcastdispatcher.js:582)
    at DowncastDispatcher.convertInsert (downcastdispatcher.js:197)
    at DataController.toView (datacontroller.js:248)
    at Document.onCopyCut (clipboard.js:151)
    at Document.fire (emittermixin.js:221)
0_table1.mp4

📃 Other details

Related - #3232.


If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@FilipTokarski FilipTokarski added type:bug This issue reports a buggy (incorrect) behavior. package:table domain:v4-compatibilty labels Jan 25, 2021
@mat128
Copy link
Contributor

mat128 commented Jan 26, 2021

I was able to reproduce the problem with the official demo editor as of today:

  1. Navigate to https://ckeditor.com/ckeditor-5/demo/
  2. Open console and attach checkChild handler to editor to allow nested tables
editors[0].model.schema.on( 'checkChild', ( evt, args ) => {
	const context = args[ 0 ];
	const childDefinition = args[ 1 ];

	if ( context.endsWith( 'tableCell' ) && childDefinition && childDefinition.name == 'table' ) {
		// Prevent next listeners from being called.
		evt.stop();
		// Set the checkChild()'s return value.
		evt.return = true;
	}
}, { priority: 'highest' } );
  1. Add 3x3 table
  2. Add 3x4 table in the middle cell
  3. Copy table
  4. Paste table
  5. Re-copy pasted table

@oleq oleq added squad:core Issue to be handled by the Core team. and removed squad:compat labels Feb 1, 2021
@oleq oleq added this to the iteration 40 milestone Feb 1, 2021
niegowski pushed a commit that referenced this issue Feb 8, 2021
Previously, it was possible for table cells to be entirely empty.
However, a recent change introduced in 93dbbf9 changed the evaluation order for table cell children.

This led to calling hasAnyAttribute on a null reference, which is not supported.

Move isSingleParagraph check prior to hasAnyAttribute. isSingleParagraph is correctly calculated because it relies on the childCount for the table cell.

Fixes #8941
Fixes #8917
@oleq oleq closed this as completed in #9004 Feb 8, 2021
oleq added a commit that referenced this issue Feb 8, 2021
Fix (table): Editor should not crash while downcasting in the data pipeline if any cell does not contain at least an empty paragraph. Closes #8941. Closes #8917. Closes #8979.

Thanks to @mat128!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:table squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants