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

Error thrown when merging cells with rowspan #4284

Closed
Comandeer opened this issue Sep 17, 2020 · 5 comments · Fixed by #5242
Closed

Error thrown when merging cells with rowspan #4284

Comandeer opened this issue Sep 17, 2020 · 5 comments · Fixed by #5242
Assignees
Labels
plugin:undo The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:bug A bug.
Milestone

Comments

@Comandeer
Copy link
Member

Type of report

Bug

Provide detailed reproduction steps (if any)

  1. Open https://jsfiddle.net/Comandeer/qj5367zy/
  2. Open console.
  3. Select column with a rowspanned cell and one other column.
  4. Open context menu and choose "Cell" → "Merge cells" option.

Expected result

Cells are merged, undo step is created.

Actual result

Cells are merged, however error is thrown and no undo step is created:

ckeditor.js:154 Uncaught TypeError: Cannot read property 'type' of null
    at a (ckeditor.js:154)
    at CKEDITOR.dom.range.createBookmark2 (ckeditor.js:156)
    at Array.createBookmarks2 (ckeditor.js:515)
    at CKEDITOR.dom.selection.createBookmarks2 (ckeditor.js:468)
    at new CKEDITOR.plugins.undo.Image (ckeditor.js:1125)
    at CKEDITOR.plugins.undo.UndoManager.save (ckeditor.js:1119)
    at a.c (ckeditor.js:1114)
    at a.d (ckeditor.js:10)
    at a.<anonymous> (ckeditor.js:11)
    at a.CKEDITOR.editor.CKEDITOR.editor.fire (ckeditor.js:13)

Other details

  • CKEditor version: 4.7.0+
  • Installed CKEditor plugins: undo, tableselection

What's the most interesting, it's the fact that it works correctly when whole standard preset is used: https://jsfiddle.net/Comandeer/2qfhuc8z/

@Comandeer Comandeer added type:bug A bug. status:confirmed An issue confirmed by the development team. plugin:undo The plugin which probably causes the issue. labels Sep 17, 2020
@corebonts
Copy link

I have this also locally, I tried to play around and I found out that if I modify the order of the plugins in your fiddle, the error goes away :D

https://jsfiddle.net/2t4hd9mj/

@corebonts
Copy link

corebonts commented Feb 1, 2022

It seems that the problem is that undo's afterCommandExec is sometimes called before the tabletools' afterCommandExec, and then the selection is not adjusted after the cell merge.

It can be fixed either in the plugin itself, or by executing the same code in a new listener, which can be used to quickfix broken applications.

    editor.on(
        "afterCommandExec",
        function (event) {
            if (event.data.name == "cellMerge") {
                // See tabletools plugin's fakeSelectCells
                var cell = event.data.commandData.cell;
                var range = editor.createRange();
                range.setStartBefore(cell);
                range.setEndAfter(cell);

                editor.getSelection().selectRanges([range]);
            }
        },
        null,
        null,
        5 // Listen with lower priority than the default (10)
    );

@Comandeer
Copy link
Member Author

@corebonts, would you like to propose an official solution based on your comment? There is a whole section dedicated to contributing in our docs if you'd decide to.

@corebonts
Copy link

of course I would like to, but I don't know when I can do it.

@CKEditorBot
Copy link
Collaborator

Closed in #5242

@CKEditorBot CKEditorBot added this to the 4.19.1 milestone Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin:undo The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:bug A bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants