Opened 6 years ago

Closed 6 years ago

#3352 closed Bug (fixed)

Enhance domIterator plugin

Reported by: garry.yao Owned by: garry.yao
Priority: High 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 6 years ago.
3352_2.patch (3.6 KB) - added by garry.yao 6 years ago.
3352_3.patch (12.0 KB) - added by garry.yao 6 years ago.
3352_4.patch (12.2 KB) - added by garry.yao 6 years ago.

Download all attachments as: .zip

Change History (14)

Changed 6 years ago by garry.yao

comment:1 Changed 6 years ago by garry.yao

  • Keywords Review? added
  • Status changed from new to assigned

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 6 years ago by garry.yao

  • Priority changed from Normal to High

comment:3 Changed 6 years ago by fredck

  • 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 6 years ago by garry.yao

comment:4 Changed 6 years ago by garry.yao

  • Keywords Review? added; Review- removed

Fix the above mentioned issue.

comment:5 follow-up: Changed 6 years ago by fredck

  • 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 6 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 6 years ago by garry.yao

comment:7 Changed 6 years ago by garry.yao

  • Keywords Review? added; Review- removed

Changed 6 years ago by garry.yao

comment:8 Changed 6 years ago by garry.yao

Adding document end reaching check.

comment:9 Changed 6 years ago by fredck

  • Keywords Review+ added; Review? removed

comment:10 Changed 6 years ago by garry.yao

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed with [3453] and [3454].

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