Opened 11 years ago

Closed 11 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)

3352.patch (3.6 KB) - added by Garry Yao 11 years ago.
3352_2.patch (3.6 KB) - added by Garry Yao 11 years ago.
3352_3.patch (12.0 KB) - added by Garry Yao 11 years ago.
3352_4.patch (12.2 KB) - added by Garry Yao 11 years ago.

Download all attachments as: .zip

Change History (14)

Changed 11 years ago by Garry Yao

Attachment: 3352.patch added

comment:1 Changed 11 years ago by Garry Yao

Keywords: Review? added
Status: newassigned

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.

comment:2 Changed 11 years ago by Garry Yao

Priority: NormalHigh

comment:3 Changed 11 years ago by Frederico Caldeira Knabben

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

Attachment: 3352_2.patch added

comment:4 Changed 11 years ago by Garry Yao

Keywords: Review? added; Review- removed

Fix the above mentioned issue.

comment:5 Changed 11 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

The above case is broken after the patch:

  1. Select two paragraphs.
  2. Apply blockquote, or a list.

It's applied for the first line only.

comment:6 in reply to:  5 Changed 11 years ago by Garry Yao

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

Attachment: 3352_3.patch added

comment:7 Changed 11 years ago by Garry Yao

Keywords: Review? added; Review- removed

Changed 11 years ago by Garry Yao

Attachment: 3352_4.patch added

comment:8 Changed 11 years ago by Garry Yao

Adding document end reaching check.

comment:9 Changed 11 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:10 Changed 11 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [3453] and [3454].

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