search

ckeditor / ckeditor4

The best enterprise-grade WYSIWYG editor. Fully customizable with countless features and plugins.
https://ckeditor.com/ckeditor-4
Other
5.79k stars 2.48k forks source link

Warning logs in console when table is removed from editor #979

Open msamsel opened 7 years ago

msamsel commented 7 years ago

Are you reporting a feature request or a bug?

Bug

Provide detailed reproduction steps (if any)

  1. Open sample from local repository. Problem seems to be not visible in build version.
  2. Open browser console.
  3. Insert table
  4. Select entire table from elements' path.
  5. Press Delete.

Expected result

No warnings in console

Actual result

Console with bunch of warnings.

Other details

msamsel commented 7 years ago

Warning sources: https://github.com/ckeditor/ckeditor-dev/blob/a0ba16d36458847bbc8af4bac73d25657b461340/core/dom/range.js#L2828-L2837 https://github.com/ckeditor/ckeditor-dev/blob/a0ba16d36458847bbc8af4bac73d25657b461340/core/dom/range.js#L2846-L2855

msamsel commented 7 years ago

Problem seems to be cause by this fragment: https://github.com/ckeditor/ckeditor-dev/blob/a0ba16d36458847bbc8af4bac73d25657b461340/plugins/tableselection/plugin.js#L908-L910 Range is body and for its boundryNodes is returned head which later on is returned as html. I tested some little change like below which seems to fix the issue (but maybe there is better way to fix it not necessary in this place):

startNode = boundaryNodes.startNode.getName() !== 'head' ? boundaryNodes.startNode : mergedRanges[ 0 ].startContainer ,
endNode = boundaryNodes.endNode.getName() !== 'head' ? boundaryNodes.endNode : mergedRanges[ 0 ].endContainer ;
mlewand commented 7 years ago

Thank you @msamsel for reporting this issue so it will be fixed by #873.