Opened 10 years ago

Closed 10 years ago

#2909 closed Task (fixed)

V3: Move Range.CreateBookmark2 from V2

Reported by: Frederico Caldeira Knabben Owned by: Frederico Caldeira Knabben
Priority: Normal Milestone: CKEditor 3.0
Component: General Version:
Keywords: Confirmed Review+ Cc:

Description

There are a few features depending on the CreateBookmark2 and MoveToBookmark2 present on V2. These functions must be moved to V3.

Attachments (3)

2909.patch (8.6 KB) - added by Frederico Caldeira Knabben 10 years ago.
2909_2.patch (13.1 KB) - added by Frederico Caldeira Knabben 10 years ago.
2909_3.patch (13.1 KB) - added by Frederico Caldeira Knabben 10 years ago.

Download all attachments as: .zip

Change History (10)

Changed 10 years ago by Frederico Caldeira Knabben

Attachment: 2909.patch added

comment:1 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review? added
Status: newassigned

The proposed patch is much simpler than the implementation present on V2. So, an accurate review is needed.

comment:2 Changed 10 years ago by Martin Kou

Keywords: Review?- added; Review? removed

There's still a case where the bookmarks that came out of createBookmark2() is non-normalized.

Let's say after running a certain command, a text node in the following paragraph is broken up to two pieces:

<p>Part 1 Part 2</p>

And the selection range looks like this, which means it selects "Part 2" only:

startContainer : <p>
startOffset : 1

When we call createBookmark2() with this range:

  1. The normalization logic in createBookmark2() isn't executed, because startContainer is not a text node.
  2. The normalization logic in getAddress() is also not executed, for the same reason.

But the index path in the bookmark is certainly not normalized.

comment:3 Changed 10 years ago by Martin Kou

Keywords: Review- added; Review?- removed

Oops, forgot to delete the ? symbol.

Changed 10 years ago by Frederico Caldeira Knabben

Attachment: 2909_2.patch added

comment:4 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review? added; Review- removed

This new patch fixes and include a test for Martin's case. It also makes the normalization optional, and makes the moveToBookmark function work for both kinds of bookmarks.

Changed 10 years ago by Frederico Caldeira Knabben

Attachment: 2909_3.patch added

comment:5 Changed 10 years ago by Frederico Caldeira Knabben

I've attached a new patch as the previous one was not considering the normalization option when movingToBookmark.

comment:6 Changed 10 years ago by Martin Kou

Keywords: Review+ added; Review? removed

comment:7 Changed 10 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: assignedclosed

Fixed with [3065]. Click here for more info about our SVN system.

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