Opened 10 years ago

Closed 10 years ago

#2864 closed Task (fixed)

Implement the insert and remove numbered list and bullet list commands.

Reported by: Martin Kou Owned by: Martin Kou
Priority: Normal Milestone: CKEditor 3.0
Component: Core : Lists Version: SVN (FCKeditor) - Retired
Keywords: Review+ Cc:

Description

Need to implement the list commands and add them to the toolbar.

Attachments (3)

2864.patch (11.7 KB) - added by Martin Kou 10 years ago.
2864_2.patch (31.5 KB) - added by Martin Kou 10 years ago.
2864_3.patch (33.7 KB) - added by Martin Kou 10 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 10 years ago by Martin Kou

Status: newassigned

Changed 10 years ago by Martin Kou

Attachment: 2864.patch added

comment:2 Changed 10 years ago by Martin Kou

Keywords: Review? added

The patch must be applied in conjunction with the patch to #2871.

Things fixed or modified in addition to adding the list command:

  1. Added the selectionSet event to CKEDITOR.dom.selection objects, which is fired when functions like selectRanges() or selectBookmarks() are fired.
  2. The editor will now fire selectionChanged event automatically if selections are changed via code.
  3. Added CKEDITOR.dom.node::getCommonAncestor().
  4. Fixed the issue in CKEDITOR.dom.domObject::getCustomData() where if the custom data value is 0 or "", the function will incorrectly treat the value as not set and return null.
  5. Added CKEDITOR.dom.domObject::removeCustomData().
  6. Added setMarker(), clearAllMarkers() and clearMarkers() under CKEDITOR.dom.element, these functions will be used in other modules like indent and blockquote.
  7. Added moveChildren(), replace(), trim(), ltrim(), rtrim(), getFirst(), getLast(), getDocument(), getChildCount(), getChild(), contains() under CKEDITOR.dom.documentFragment prototype.
  8. Added CKEDITOR.tools.bind(), duplicated from Garry's patch in #2768.

comment:3 Changed 10 years ago by Martin Kou

Simplified two things:

  1. Eliminated the duplicate code in CKEDITOR.dom.range::getCommonAncestor().
  2. Changed the selectionSet event in CKEDITOR.dom.selection to a simpler onSelectionSet function hook.

Selection after list removal or list type change is wrong now because the selection bookmarks are not serialize-able at this moment. Garry is trying to fix that issue in #2763 so I'll leave it as-is for now.

Changed 10 years ago by Martin Kou

Attachment: 2864_2.patch added

comment:4 Changed 10 years ago by Martin Kou

Also note that part of the logic in #2865 depends on the list plugin, since list indenting uses the logic in the list plugin.

comment:5 Changed 10 years ago by Martin Kou

Added missing lang entries.

Changed 10 years ago by Martin Kou

Attachment: 2864_3.patch added

comment:6 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:7 Changed 10 years ago by Martin Kou

Resolution: fixed
Status: assignedclosed

Fixed with [3051].

comment:8 Changed 10 years ago by Garry Yao

Resolution: fixed
Status: closedreopened

I found in FF source codes was changed after the following steps, markers nodes were left inside.

  1. Apply numbered/bullet list style
  2. Remove numbered/bullet list style
  3. Check codes in source mode

comment:9 Changed 10 years ago by Martin Kou

That's exactly due to the serialize-able selection problem that you're trying to fix in #2763.

What's happening right now is..

  1. Bookmarks are saved before changing lists.
  2. The original list is removed with the non-serialize-able bookmark nodes.
  3. The original list is cloned with the bookmark nodes, and modified to become the new list.
  4. The new list is added to the document with the cloned bookmark nodes.
  5. The selection plugin tries to restore selection to the removed bookmark nodes, leaving the cloned bookmark nodes in the document.

So this bug is not the list plugin's fault, but rather the selection system's fault.

comment:10 Changed 10 years ago by Garry Yao

Resolution: fixed
Status: reopenedclosed

Sorry, I should have noticed that, it seems the serialize-able bookmark works for this case since you're relying on Node::cloneNode which will reserve id attribute.

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