Opened 17 years ago
Closed 17 years ago
#3352 closed Bug (fixed)
Enhance domIterator plugin
| Reported by: | Garry Yao | Owned by: | Garry Yao |
|---|---|---|---|
| Priority: | Must have (possibly next milestone) | Milestone: | CKEditor 3.0 |
| Component: | General | Version: | |
| Keywords: | Confirmed Review+ | Cc: |
Description
Certain utility methods( L15-L60 ) of the plugin are not updated with the current trunk codes, which should be refacted.
Attachments (4)
Change History (14)
Changed 17 years ago by
| Attachment: | 3352.patch added |
|---|
comment:1 Changed 17 years ago by
| Keywords: | Review? added |
|---|---|
| Status: | new → assigned |
comment:2 Changed 17 years ago by
| Priority: | Normal → High |
|---|
comment:3 Changed 17 years ago by
| Keywords: | Review- added; Review? removed |
|---|
The only problem I can find here is that at line 235 we were using a variable to indicate whether or not to continue from the next sibling (continueFromSibling), and the patch is instead forcing it to true always. For the rest, it looks ok.
Changed 17 years ago by
| Attachment: | 3352_2.patch added |
|---|
comment:4 Changed 17 years ago by
| Keywords: | Review? added; Review- removed |
|---|
Fix the above mentioned issue.
comment:5 follow-up: 6 Changed 17 years ago by
| Keywords: | Review- added; Review? removed |
|---|
The above case is broken after the patch:
- Select two paragraphs.
- Apply blockquote, or a list.
It's applied for the first line only.
comment:6 Changed 17 years ago by
Replying to fredck: The bug was due to when invoking 'CKEDITOR.dom.range::getTouchedStart/EndNode' on the following content:
<p id="p1">pa^ra1</p> <p id="p2">pa^ra2</p>
The result( touchedStart: p1, touchedEnd: p2 ) will exclude p2 from been iterated over, here it's safely to use 'CKEDITOR.dom.range::getBoundaryNodes' since the range is almost never collapsed.
I'm attaching a new patch regard this and including the necessary TCs also.
Changed 17 years ago by
| Attachment: | 3352_3.patch added |
|---|
comment:7 Changed 17 years ago by
| Keywords: | Review? added; Review- removed |
|---|
Changed 17 years ago by
| Attachment: | 3352_4.patch added |
|---|
comment:9 Changed 17 years ago by
| Keywords: | Review+ added; Review? removed |
|---|
comment:10 Changed 17 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |

Besides update to current APIs within the chunk, the patch also fix the next source node logic to ignore bookmark nodes which been identified at #3141.