Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#8246 closed Bug (fixed)

Performance problems with nested documents

Reported by: Damian Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.6.2
Component: Performance Version:
Keywords: IBM Cc: Satya Minnekanti, Teresa Monahan

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 (4)

content.html (14.2 KB) - added by Damian 10 years ago.
Sample content
8246.patch (916 bytes) - added by Garry Yao 10 years ago.
8246_2.patch (3.3 KB) - added by Garry Yao 10 years ago.
8246_3.patch (4.1 KB) - added by Garry Yao 10 years ago.

Download all attachments as: .zip

Change History (16)

Changed 10 years ago by Damian

Attachment: content.html added

Sample content

comment:1 Changed 10 years ago by Jakub Ś

Status: newconfirmed

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 10 years ago by Jakub Ś (previous) (diff)

comment:2 Changed 10 years ago by Damian

Is there an update on this ticket?

comment:3 Changed 10 years ago by Garry Yao

Component: GeneralPerformance
Owner: set to Garry Yao
Status: confirmedreview

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 10 years ago by Garry Yao

Attachment: 8246.patch added

Changed 10 years ago by Garry Yao

Attachment: 8246_2.patch added

comment:4 Changed 10 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 10 years ago by Garry Yao (previous) (diff)

comment:5 Changed 10 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

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

comment:6 Changed 10 years ago by Garry Yao

Status: review_failedreview

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

Other tts that failed has been addressed by new patch.

Changed 10 years ago by Garry Yao

Attachment: 8246_3.patch added

comment:7 Changed 10 years ago by Frederico Caldeira Knabben

Status: reviewreview_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 10 years ago by Frederico Caldeira Knabben

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 10 years ago by Garry Yao

Status: review_failedreview

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

comment:10 Changed 10 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.6.2
Status: reviewreview_passed

Great job!

comment:11 Changed 10 years ago by Garry Yao

Resolution: fixed
Status: review_passedclosed

Fixed with [7235].

comment:12 Changed 10 years ago by Satya Minnekanti

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 – 2021 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy