#7494 closed Bug (fixed)
Getting Unresponsive Script Error when we try to paste Numbered/Bulleted lists with one more than one level
Reported by: | Satya Minnekanti | Owned by: | Garry Yao |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 3.5.3 |
Component: | Plugin : Paste from Word | Version: | 3.5.3 |
Keywords: | IBM | Cc: | Damian, James Cunningham, Teresa Monahan |
Description
To reproduce the defect:
- Open Ajax sample, Copy Numbered/Bulleted list from the attached doc and open Paste from Word dialog.
- Paste the content in to the dialog and press OK button.
Expected Result:
Numbered/Bulleted list is pasted properly.
Actual Result:
we are getting an Unresponsive Script Error
Attachments (4)
Change History (20)
Changed 14 years ago by
Attachment: | Numbers & Bulltes lists.doc added |
---|
comment:1 Changed 14 years ago by
Status: | new → confirmed |
---|
comment:2 Changed 14 years ago by
Milestone: | → CKEditor 3.5.3 |
---|
comment:3 Changed 14 years ago by
It causes all bowsers to hang, except Opera - everything looks fine there.
comment:4 Changed 14 years ago by
Note: I managed to trigger this bug in IE8 when working with lists, without pasting the attached (or any other) document from Word. I cannot find a way now to trigger this bug again, but it definitely happened.
comment:5 Changed 14 years ago by
I've found a reduced tc for this issue. Just load the following:
<ol> <li>1</li> <ol> <li>2</li> </ol> <li>3</li> </ol>
It hangs the editor.
comment:6 Changed 14 years ago by
Owner: | set to Sa'ar Zac Elias |
---|---|
Status: | confirmed → assigned |
comment:7 Changed 14 years ago by
The above case is not covered here ([6643]):
http://ckeditor.t/dt/core/htmlparser/htmlparser.html
Changed 14 years ago by
Attachment: | 7494.patch added |
---|
comment:9 Changed 14 years ago by
Status: | assigned → review |
---|
Patch removes the changes that are not required to fix either bugs.
comment:10 Changed 14 years ago by
Status: | review → review_failed |
---|
Some of tests are not passing after patch:
http://ckeditor.t/dt/core/htmlparser/fragment.html
In fact I found it strange that that code has been introduced but it's not needed.
Changed 14 years ago by
Attachment: | 7494_2.patch added |
---|
comment:11 Changed 14 years ago by
Owner: | changed from Sa'ar Zac Elias to Garry Yao |
---|---|
Status: | review_failed → review |
Patch targets also #7497
comment:12 Changed 14 years ago by
Status: | review → review_failed |
---|
Please, let's start to avoid enlarging parameters lists on functions calls just to hack the code. A simple list check on a new entry on CKEDITOR.dtd will probably make it work in the same way, having much cleaner code.
Additionally, please review the changes when creating patches. There is some minor unnecessary code formatting changes there.
Changed 14 years ago by
Attachment: | 7494_3.patch added |
---|
comment:13 follow-up: 14 Changed 14 years ago by
Status: | review_failed → review |
---|
Please, let's start to avoid enlarging parameters lists on functions calls just to hack the code
I'm not sure as:
- We're talking about an internally used function;
- The optional close is not a dtd semantics but used when we're opening a new (fixing) tag dynamically.
comment:14 Changed 14 years ago by
Status: | review → review_passed |
---|
Replying to garry.yao:
- We're talking about an internally used function;
Sorry, this is not a good excuse to mess up the code.
- The optional close is not a dtd semantics but used when we're opening a new (fixing) tag dynamically.
I got your point now.
---
Go ahead committing the change on fragment.js, but do not change element.js. Other than having duplicated code there on the last patch, there is no need at all to initialize those properties there, especially considering that this is an internal flag exclusive to the fragment.js code.
comment:15 Changed 14 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Fixed with [6646].
comment:16 Changed 11 years ago by
Cc: | damo,jamescun,tmonahan → damo, jamescun, tmonahan |
---|---|
Component: | General → Plugin : Paste from Word |
Caused by [6629]