Opened 8 years ago

Closed 8 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 8 years ago.
3352_2.patch (3.6 KB) - added by Garry Yao 8 years ago.
3352_3.patch (12.0 KB) - added by Garry Yao 8 years ago.
3352_4.patch (12.2 KB) - added by Garry Yao 8 years ago.

Download all attachments as: .zip

Change History (14)

Changed 8 years ago by Garry Yao

Attachment: 3352.patch added

comment:1 Changed 8 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 8 years ago by Garry Yao

Priority: NormalHigh

comment:3 Changed 8 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 8 years ago by Garry Yao

Attachment: 3352_2.patch added

comment:4 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed

Fix the above mentioned issue.

comment:5 Changed 8 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 8 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 8 years ago by Garry Yao

Attachment: 3352_3.patch added

comment:7 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed

Changed 8 years ago by Garry Yao

Attachment: 3352_4.patch added

comment:8 Changed 8 years ago by Garry Yao

Adding document end reaching check.

comment:9 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:10 Changed 8 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [3453] and [3454].

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