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 inserting table within selection composed of multiple heading styles #3276

Closed
wf-gijs opened this issue Jul 17, 2019 · 6 comments · Fixed by ckeditor/ckeditor5-engine#1770
Assignees
Labels
package:table type:bug This issue reports a buggy (incorrect) behavior.

Comments

@wf-gijs
Copy link

wf-gijs commented Jul 17, 2019

Steps to reproduce

I was able to reproduce this issue on the official examples / latest version.

  1. Insert text with heading style set to paragraph.
  2. Select the text and apply a heading 1 style.
  3. While the selection is still active, insert a new table.
  4. Deselect.
  5. Make a new selection starting at the heading 1 text and include the inserted table.
  6. When the table gets selected, the active heading style denoted on the toolbar dropdown button will switch to paragraph style.
  7. Insert a new table while the selection is still active.
  8. Editor crashes with an uncaught TypeError.

Backtrace

Uncaught TypeError: Cannot read property 'model' of null
    at LiveRange.bindWithDocument (liverange.js:121)
    at new LiveRange (liverange.js:31)
    at Function.fromRange (liverange.js:59)
    at LiveSelection._prepareRange (documentselection.js:815)
    at LiveSelection._pushRange (documentselection.js:787)
    at LiveSelection._setRanges (selection.js:469)
    at LiveSelection.setTo (selection.js:379)
    at LiveSelection.setTo (documentselection.js:719)
    at DocumentSelection._setTo (documentselection.js:418)
    at Writer.setSelection (writer.js:1155)
bindWithDocument     @ liverange.js:121
LiveRange            @ liverange.js:31
fromRange            @ liverange.js:59
_prepareRange        @ documentselection.js:815
_pushRange           @ documentselection.js:787
_setRanges           @ selection.js:469
setTo                @ selection.js:379
setTo                @ documentselection.js:719
_setTo               @ documentselection.js:418
setSelection         @ writer.js:1155
(anonymous)          @ inserttablecommand.js:63
_runPendingChanges   @ model.js:737
change               @ model.js:160
execute              @ inserttablecommand.js:58
(anonymous)          @ observablemixin.js:254
fire                 @ emittermixin.js:207
(anonymous function) @ observablemixin.js:258
execute              @ commandcollection.js:68
execute              @ editor.js:268
(anonymous)          @ tableui.js:59
fire                 @ emittermixin.js:207
fireDelegatedEvents  @ emittermixin.js:617
fire                 @ emittermixin.js:230
(anonymous)          @ inserttableview.js:97
callback             @ template.js:950
fire                 @ emittermixin.js:207
domListener          @ emittermixin.js:235
@Mgsy
Copy link
Member

Mgsy commented Jul 22, 2019

Hello, I'm not able to reproduce the issue. Can you record a screencast to show exact steps to reproduce? Also, please check if you're able to reproduce it here.

@wf-gijs
Copy link
Author

wf-gijs commented Jul 22, 2019

Hello,

I recorded a screencast showing the steps taken to reproduce this issue via the Classic Editor example:

screencast

@Mgsy
Copy link
Member

Mgsy commented Jul 22, 2019

Thanks, @wf-gijs, now I'm able to reproduce the issue. However, the scenario could be simplified, you just need to insert the table at the beginning of the document, select the second block and the table, then insert a new table.

It throws with the following error:

liverange.js:121 Uncaught TypeError: Cannot read property 'model' of null
    at r.<anonymous> (liverange.js:121)
    at new r (liverange.js:31)
    at Function.fromRange (liverange.js:59)
    at p._prepareRange (documentselection.js:817)
    at p._pushRange (documentselection.js:789)
    at p._setRanges (selection.js:475)
    at p.setTo (selection.js:379)
    at p.setTo (documentselection.js:720)
    at g._setTo (documentselection.js:418)
    at We.setSelection (writer.js:1177)

bug_cke5

cc @jodator

@wf-gijs
Copy link
Author

wf-gijs commented Jul 22, 2019

Ah, I see. Thank you for the quick response!

@jodator
Copy link
Contributor

jodator commented Jul 23, 2019

There's a potential bug in the findeOptimalPosition() method - it ends up in the table and not in the root causing table to not be inserted into the model.

@jodator
Copy link
Contributor

jodator commented Jul 23, 2019

And I have a quick fix so if the change will not cause some side-effects we will try to address this in the current iteration.

@jodator jodator self-assigned this Jul 23, 2019
Reinmar referenced this issue in ckeditor/ckeditor5-engine Sep 3, 2019
Other: Remove the `Selection#getTopMostBlocks()` method. Closes ckeditor/ckeditor5-widget#95. Closes ckeditor/ckeditor5-table#199.

BREAKING CHANGE: The `Selection#getTopMostBlocks()` was removed from the public API. Use `Selection#getSelectedBlocks()` instead.

BREAKING CHANGE: The `Selection#getSelectedBlocks()` does not return blocks nested in other blocks now.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-table Oct 9, 2019
@mlewand mlewand added this to the iteration 27 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:bug This issue reports a buggy (incorrect) behavior. package:table labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:table type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants