Ticket #8246 (closed Bug: fixed)

Opened 3 years ago

Last modified 3 years ago

Performance problems with nested documents

Reported by: damo Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.6.2
Component: Performance Version:
Keywords: IBM Cc: satya, tmonahan

Description

This problem was raised by a customer using Outlook to copy and paste content into CKEditor.

When the content is pasted (or set using the source edit mode), in IE a "stack overflow" error will occur. In Firefox, this can manifest as an "unresponsive script" error.

It appears that the heavily nested nature of the document is what is causing the editor to hang or throw an error.

When the content is viewed on its own in a browser or copied and pasted on a vanilla contenteditable div, the above mentioned performance issues are not noticeable. This seems to suggest that the performance issues are a result of the editor's processing on the document rather than a browser issue only.

Attachments

content.html (14.2 KB) - added by damo 3 years ago.
Sample content
8246.patch (916 bytes) - added by garry.yao 3 years ago.
8246_2.patch (3.3 KB) - added by garry.yao 3 years ago.
8246_3.patch (4.1 KB) - added by garry.yao 3 years ago.

Change History

Changed 3 years ago by damo

Sample content

comment:1 Changed 3 years ago by j.swiderski

  • Status changed from new to confirmed

I have managed to reproduce it in all IE browsers.

Switch to source, paste the code form the attachment, switch to WYSIWYG:
IE6 - Browser crashes. Reoproducible from CKEditor 3.0.2
IE7 - Browser throws 'Stack Overflow' and editor crashes. Reproducible from CKEditor 3.0
IE8 - Browser displays info on long running script or freezes. Reproducible form CKE 3.0.2

Switch to source, paste the code form the attachment, switch to WYSIWYG, switch to source:
IE9 – Page crashes. Reproducible form CKE 3.0

Last edited 3 years ago by j.swiderski (previous) (diff)

comment:2 Changed 3 years ago by damo

Is there an update on this ticket?

comment:3 Changed 3 years ago by garry.yao

  • Owner set to garry.yao
  • Status changed from confirmed to review
  • Component changed from General to Performance

It looks like we have a performance issue with the CKEDITOR.dom.node::isReadOnly implementation, which was revealed in CKEDITOR.dom.range::moveToElementEditStart, causing the complexity of this method raised to O(n2).

There's a much simpler (and equivalent) implementation exists though, I've also expanded the relevant dts: http://ckeditor.t/dt/core/dom/node.html

Changed 3 years ago by garry.yao

Changed 3 years ago by garry.yao

comment:4 Changed 3 years ago by garry.yao

In previous patch I've neglected some side effects of the old API, which was addressed in the new patch and dts are enlarged to include them.

Last edited 3 years ago by garry.yao (previous) (diff)

comment:5 Changed 3 years ago by fredck

  • Status changed from review to review_failed

I have 12 unit test failures after patch, with FF at least.

comment:6 Changed 3 years ago by garry.yao

  • Status changed from review_failed to review

The following tt has been adjusted. http://ckeditor.t/tt/4781/1.html

Other tts that failed has been addressed by new patch.

Changed 3 years ago by garry.yao

comment:7 Changed 3 years ago by fredck

  • Status changed from review to review_failed

Sorry, but I still have 10 test failures after the new patch:
http://ckeditor.t/dt/plugins/styles/styles.html
http://ckeditor.t/tt/4781/1.html

comment:8 Changed 3 years ago by fredck

Btw, please do not do direct changes to ckeditor-tests/master for this ticket, making it on a ticket test branch instead.

comment:9 Changed 3 years ago by garry.yao

  • Status changed from review_failed to review

Now branch tt/8246 contains all tc updates brought by this ticket patch.

comment:10 Changed 3 years ago by fredck

  • Status changed from review to review_passed
  • Milestone set to CKEditor 3.6.2

Great job!

comment:11 Changed 3 years ago by garry.yao

  • Status changed from review_passed to closed
  • Resolution set to fixed

Fixed with [7235].

comment:12 Changed 3 years ago by satya

we are still getting stack over flow error in IE6 paste the date or set the data in source and navigate to rich text view.

IE7,IE8 Browser stopped responding when we paste the data or set the data in source and navigate to rich text view.

Please re open this ticket.

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