Opened 6 years ago

Last modified 5 years ago

#10400 confirmed Bug

CKEDITOR(inline) crashes when using ol/ul list

Reported by: Mandeep Owned by:
Priority: Normal Milestone:
Component: General Version: 4.0 Beta
Keywords: Cc:

Description

I am using Inline CKEitor version 4.0.2. I have a ol which has many li tags. In each li, there are contenteditable divs initialized with Inline CKEditors. Now I create a ol/ul list inside the contenteditable div. Next I add some more content below the ol/ul list. Then I select the complete text below the list and complete text of any li together(see attachment for selection) and press enter, the editor crashes.

Here is the stacktrace of the error:

Uncaught TypeError: Cannot call method 'getParent' of null ckeditor.js:139 CKEDITOR.dom.range.setStartBefore ckeditor.js:139 CKEDITOR.dom.range.moveToBookmark ckeditor.js:123 CKEDITOR.dom.selection.selectBookmarks ckeditor.js:331 p.exec ckeditor.js:616 exec ckeditor.js:162 CKEDITOR.tools.extend.execCommand ckeditor.js:195 CKEDITOR.plugins.enterkey.enterBlock ckeditor.js:619 m ckeditor.js:618 a.addCommand.exec ckeditor.js:619 exec ckeditor.js:162 CKEDITOR.tools.extend.execCommand ckeditor.js:195 c ckeditor.js:171 h ckeditor.js:10 CKEDITOR.event.CKEDITOR.event.fire ckeditor.js:12 (anonymous function)

Note: This does not happen if the text is not selected completely either of the li or the text below it. Also, it happens only upon clicking enter. There is no error on clicking backspace. And it does not happen if my CKEdtitor div is not present in an li tag of an ol.

Here is the jsfiddle demonstrating the issue: http://jsfiddle.net/FLZhn/1/

I have fount this bug also in 4.0.1 and 4.1.1

Let me know if you have any other queries. Thanks.

Attachments (2)

CKEditor_ol_ul_crash_bug.png (63.3 KB) - added by Mandeep 6 years ago.
tescik.html (740 bytes) - added by Jakub Ś 6 years ago.

Download all attachments as: .zip

Change History (28)

Changed 6 years ago by Mandeep

comment:1 Changed 6 years ago by Mandeep

Can I get an update on this? Or atleast a reply. Waiting patiently. Thanks :)

Changed 6 years ago by Jakub Ś

Attachment: tescik.html added

comment:2 Changed 6 years ago by Jakub Ś

Status: newconfirmed
Version: 4.0.24.0 Beta

JS error:
Message: Cannot call method 'getParent' of null
URI: /ckeditor4/core/dom/range.js Line: 1523

This particular issue can be reproduced from CKEditor 4.0 beta in every browser.

This ticket looks very similar to #9679. Ticket #9679 throws same error in same line but it only throws it in IE and Webkit (This ticket throws error in all browsers). HTML result for #9679 is however disturbing and IMHO suggests silent fail in other browsers thus I think that both tickets have same fix.

Last edited 5 years ago by Jakub Ś (previous) (diff)

comment:3 Changed 6 years ago by Mandeep

Can I get an update on this? Thanks :)

comment:4 Changed 6 years ago by Mandeep

Hi, I know you guyz are working hard. I hope you wont mind giving some details of when will this be fixed. Waiting patiently :)

comment:5 Changed 6 years ago by Frederico Caldeira Knabben

Component: Core : ListsGeneral
Milestone: CKEditor 4.1.3

We're going to investigate this issue soon.

comment:6 Changed 6 years ago by Piotr Jasiun

Owner: set to Piotr Jasiun
Status: confirmedassigned

comment:7 Changed 6 years ago by Piotr Jasiun

Milestone: CKEditor 4.1.3CKEditor 4.2

Add unit and manual tests for this ticket (manual test can be deleted later).

test branch: 251359dfb08c422cd432610c54c302870d2a8b0a

http://ckeditor4.t/tt/10400/1.html

Because it is based on indent plugin with is rewriting in #10027 this ticked must wait until #10027 will be finished.

comment:8 Changed 6 years ago by Piotr Jasiun

Because of dependencies (with #10027) I think we should move this ticket to the next milestone (4.2.1 ?).

comment:9 Changed 6 years ago by Olek Nowodziński

Since #10027, editor no longer crashes in such case. However the result of pressing enter is far from expectations.

I assume that there are 4 basic cases for cross-selection of the last list item. There are, of course, corresponding cases for the first element of the list. Also, if we started from second/second-to-last item, there are event more cases to be checked and so on. But let's take a look only on those four at the moment.

    +            +            +            +
    |   Input    |   t/10027  |    LO3     | MSO 2003
+---|------------|------------|------------|----------+
    |            |            |            |
    |  1. x      |    1. x    |    1. x    |   1. x
 1. |  2. [y     |    [y      |    2.      |   ^
    |  z]        |    z]      |    3. ^    |
    |            |            |            |
+---|------------|------------|------------|----------+
    |            |            |            |
    |  1. x      |    1. x    |    1. x    |   1. x
 2. |  2. [y     |    2.      |    ^z      |   ^z
    |  z]z       |    1.      |            |
    |            |    ^z      |            |
+---|------------|------------|------------|----------+
    |            |            |            |
    |  1. x      |    1. x    |    1. x    |   1. x
 3. |  2. y[y    |    2. y    |    2. y    |   y
    |  z]        |    1. ^    |    3. ^    |   ^
    |            |            |            |
+---|------------|------------|------------|----------+
    |            |            |            |
    |  1. x      |    1. x    |    1. x    |   1. x
 4. |  2. y[y    |    2. y    |    2. y    |   y
    |  z]z       |    ^z      |    3. ^z   |   ^z
    +            +            +            +

I compared editor output with LibreOffice and MSO 2003. Conclusion: current implementation is very weak. The only one that has some logic is MSO 2003. It's intuitive and I'm for it as well as Reinmar.

Questions:

  • Do we really need to focus on this issue since this is not a regression (also buggy in v3) and no longer crashes the editor? I don't think so.
  • Is so, 4.2 really?
  • If so, should we handle all possible cases (there are at least 8 of them, possibly more)?
  • Which behavior is correct then?

comment:10 Changed 6 years ago by Olek Nowodziński

Owner: changed from Piotr Jasiun to Olek Nowodziński

comment:11 in reply to:  9 Changed 6 years ago by Frederico Caldeira Knabben

Replying to a.nowodzinski:

  • Do we really need to focus on this issue since this is not a regression (also buggy in v3) and no longer crashes the editor? I don't think so.

If we have time for it, yes.

  • Is so, 4.2 really?

Yes, because of the changes we did on ENTER key.

  • If so, should we handle all possible cases (there are at least 8 of them, possibly more)?

Well... the more, the better... with priority to the ticket tc.

  • Which behavior is correct then?

From you list, I like 1 and 2 from MSO and 3 and 4 from LO.

Except for few cases (like 2) and to make it simpler, I think the general behavior for ENTER on a non-collapsed selection should simulate DEL + ENTER.

comment:12 Changed 6 years ago by Piotr Jasiun

Last time I checked it with #10027 there was also such case:

         Input:                   Output:

    Editor:                  Editor:
   +--------------+          +--------------+
 1.|1.a           |        1.|1.a           |
   |2.b[          |   ->     |2.            |
   |c]            |        2.|              |
   +--------------+          +--------------+

There should be one test for that. I think that content outside the editor should be never changed by editor.

I think it is not a big difference if result will be like LO3 or MSO 2003. Both of them are enough good and simpler to implement is better. I agree with Fred that DEL + ENTER is good behavior in general.

comment:13 Changed 6 years ago by Piotrek Koszuliński

DEL+ENTER gives:

  1. MSO 2003 (OK for me)
  2. Something new:
1. x
2.
3. ^z

Which is fine for me too.

  1. LO3 (OK)
  2. LO3 (OK)

So now I'm also for this :).

comment:14 Changed 6 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 4.2CKEditor 4.2.1

This issue became bigger than we thought, so I'm postponing it because we'll not have time to work on it for the 4.2.

comment:15 Changed 6 years ago by Mandeep

OK. Thanks anyways :)

comment:16 Changed 6 years ago by Olek Nowodziński

Owner: Olek Nowodziński deleted
Status: assignedconfirmed

As this ticket may require lots of research, coding and testing, I doubt it can be done withing 4.2.1.

comment:17 Changed 5 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.2.1

I'm removing the 4.2.1 milestone. I agree with Olek that this ticket is too complex for a minor release. 4.3 is already full, so this ticket needs to wait longer.

comment:18 Changed 5 years ago by Mandeep

I wouldnt have bothered, but the results of this bug are very disturbing. I have a lot of data in my ol which I have to save. Wiping it completely is a very bad loss for the user and it would not be acceptable for the user to enter that data again. Isnt there any simple solution that tells to remove only the nearest ol/ul and not others?

comment:19 Changed 5 years ago by Piotrek Koszuliński

I don't understand. This ticket is now (after #10027 - see comment:11) about making the enter key work consistently. I don't see any data loss, or excessive deletion mentioned. Could you elaborate more?

comment:20 Changed 5 years ago by Mandeep

I am sorry, I missed the fact that the editor no longer crashes after http://dev.ckeditor.com/ticket/10027 was fixed. Thanks for pointing that out.

comment:21 Changed 5 years ago by santaclaus21

I've got the same problem in CKE 4.3.3. After some switching normal mode / source (+codemirror 3rd) I can observe see in console: Uncaught TypeError: Cannot call method 'getParent' of undefined [ckeditor.js:149].

In this time also function "CKEDITOR.on('instanceReady', function (ev)..." is not running after loading CKE (the same instance without previously destroy). So im my conclusion, my save function (made by bind CKE "save") stops working.

I waste some hours looking for problem. Now I see - problem exist when div > ul > li are in source. Maybe something around that... I'm not sure about cases of exist (context) this problem.

UPDATE: OK, ul/li are not reason. In my case error show up (sometimes) when I'm switching normal/source mode. In html exists only p,div,img,nbsp. I'am using some 3rd plugins. So I will try to make new build without plugins. To edit source I'm using codemirror plugin.

UPDATE 2: calling an error (100% repeat, removed all 3rd plugins):

1) open ckeditor by CKEDITOR.replace('element_id') 2) go to soure edit 3) switch to normal edit 4) destroy by { ckeditor editor.destroy(); editor = null; } 5) open ckeditor by CKEDITOR.replace('element_id')

Uncaught TypeError: Cannot call method 'getParent' of undefined [ckeditor:149]

UPDATE 3: In my case - problem was coming from f... plugin KeepTextSelection.

Last edited 5 years ago by santaclaus21 (previous) (diff)

comment:22 Changed 5 years ago by Jakub Ś

I have just tried your 100% TC and didn't get this issue. I have tried it in default editor full package with no third-party plugins.

Please provide sample HTML that is causing this problem in default editor.

comment:23 Changed 5 years ago by Jakub Ś

@santaclaus21 thanks for explanation and making sure that third-party plugin is responsible for this. You should report it to plugin author in such case.

comment:24 in reply to:  22 Changed 5 years ago by santaclaus21

Replying to j.swiderski:

I have just tried your 100% TC and didn't get this issue. I have tried it in default editor full package with no third-party plugins.

Please provide sample HTML that is causing this problem in default editor.

my fault, I didn't removed all 3rd plugin carefully firstly.

comment:25 in reply to:  23 Changed 5 years ago by santaclaus21

Replying to j.swiderski:

@santaclaus21 thanks for explanation and making sure that third-party plugin is responsible for this. You should report it to plugin author in such case.

Reported. I was using this plugin (KeepTextSelection) in 4.3.1 with success.

comment:26 Changed 5 years ago by Jakub Ś

Reported. I was using this plugin (KeepTextSelection) in 4.3.1 with success.

When we make changes in editor we can't take every plugin into account that is out there. This is still plugin author job to find out what is wrong and try to fix it in his code.

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