Ticket #3352 (closed Bug: fixed)

Opened 5 years ago

Last modified 5 years ago

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

3352.patch (3.6 KB) - added by garry.yao 5 years ago.
3352_2.patch (3.6 KB) - added by garry.yao 5 years ago.
3352_3.patch (12.0 KB) - added by garry.yao 5 years ago.
3352_4.patch (12.2 KB) - added by garry.yao 5 years ago.

Change History

Changed 5 years ago by garry.yao

comment:1 Changed 5 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 5 years ago by garry.yao

  • Priority changed from Normal to High

comment:3 Changed 5 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 5 years ago by garry.yao

comment:4 Changed 5 years ago by garry.yao

  • Keywords Review? added; Review- removed

Fix the above mentioned issue.

comment:5 follow-up: ↓ 6 Changed 5 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 5 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 5 years ago by garry.yao

comment:7 Changed 5 years ago by garry.yao

  • Keywords Review? added; Review- removed

Changed 5 years ago by garry.yao

comment:8 Changed 5 years ago by garry.yao

Adding document end reaching check.

comment:9 Changed 5 years ago by fredck

  • Keywords Review+ added; Review? removed

comment:10 Changed 5 years ago by garry.yao

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

Fixed with [3453] and [3454].

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