Opened 4 years ago

Closed 4 years ago

#12110 closed Bug (fixed)

[PR#104] Editor crash after deleting a table.

Reported by: Piotr Jasiun Owned by: Marek Lewandowski
Priority: Normal Milestone: CKEditor 4.4.3
Component: General Version: 4.0 Beta
Keywords: Cc: mike@…

Description (last modified by Marek Lewandowski)

Scenario:

  1. Have an inline initialized editor
  2. In that editor add a table and ensure that it is the sole element in the editable container (nothing before and nothing after)
  3. Use the contextual menu to delete the table
  4. The editor is no longer working :(

Note: This can be reproduced also on the demo site: http://ckeditor.com/demo#inline

The fix: Just added a check to not remove the parent element of the table if that parent is the container.

PR: https://github.com/ckeditor/ckeditor-dev/pull/104

Related ticket: #12122

Change History (7)

comment:1 Changed 4 years ago by Jakub Ś

Status: newconfirmed
Version: 4.0 Beta

Problem can be reproduced from CKEditor 4.0 beta.

Reminded me #9315 but they are rather not related.

comment:2 Changed 4 years ago by Jakub Ś

Similar issue: #12122.

comment:3 Changed 4 years ago by Mike Burgh

Cc: mike@… added

comment:4 Changed 4 years ago by Marek Lewandowski

Owner: set to Marek Lewandowski
Status: confirmedassigned

comment:5 Changed 4 years ago by Marek Lewandowski

Status: assignedreview

Branch rebased and pushed tests to t/12110 at dev.

I've also removed parent.is( 'body' ) part in git:f3468044ab - as it seems to be redundant to me when checking editable.equals( parent ).

From my testing it does not break removing table directly in fullpage mode - as fullpage <body/> elmeent is still editable.

comment:6 Changed 4 years ago by Marek Lewandowski

Description: modified (diff)
Milestone: CKEditor 4.4.3

comment:7 Changed 4 years ago by Piotrek Koszuliński

Resolution: fixed
Status: reviewclosed

Merged to master with git:89f4a31. Additionally, I needed to improve tools.html.compareInnerHtml - https://github.com/benderjs/benderjs-ckeditor/commit/08be96bcec82ab2779f226319d99fe8f025e1666

Warning for the future: verify that tests for code you're changing really work (and exist first of all). In this case test for removing table's wrapper existed but was buggy and was always green. That was masking a bug in code touched by this patch. This bug was introduced in this patch too. I stumbled upon this when trying to make that existing test fail, by commenting out the condition from dev code.

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy