#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)
Change History (16)
Changed 14 years ago by
| Attachment: | content.html added |
|---|
comment:1 Changed 14 years ago by
| Status: | new → 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
comment:3 Changed 14 years ago by
| Component: | General → Performance |
|---|---|
| Owner: | set to Garry Yao |
| Status: | confirmed → review |
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 14 years ago by
| Attachment: | 8246.patch added |
|---|
Changed 14 years ago by
| Attachment: | 8246_2.patch added |
|---|
comment:4 Changed 14 years ago by
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.
comment:5 Changed 14 years ago by
| Status: | review → review_failed |
|---|
I have 12 unit test failures after patch, with FF at least.
comment:6 Changed 14 years ago by
| Status: | review_failed → 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 14 years ago by
| Attachment: | 8246_3.patch added |
|---|
comment:7 Changed 14 years ago by
| Status: | review → 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 14 years ago by
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 14 years ago by
| Status: | review_failed → review |
|---|
Now branch tt/8246 contains all tc updates brought by this ticket patch.
comment:10 Changed 14 years ago by
| Milestone: | → CKEditor 3.6.2 |
|---|---|
| Status: | review → review_passed |
Great job!
comment:11 Changed 14 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review_passed → closed |
Fixed with [7235].
comment:12 Changed 14 years ago by
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.
Sample content