Opened 8 months ago

Closed 4 months ago

#14707 closed New Feature (fixed)

Copy Formatting

Reported by: m.lewandowski Owned by: t.jakut
Priority: Normal Milestone: CKEditor 4.6.0
Component: General Version:
Keywords: Cc:

Description

Let's integrate Copy Formatting plugin into CKEditor 4.6.0.

Couple of things that needs to be done before releasing CF

  • Create SDK example.
  • Make sure that all needed language files are created (in other words just run fix_plugins.sh file).
  • Refine API docs.
  • Create necessary entry in meta directory.
  • Remove customer-specific ticket references.

Change History (10)

comment:1 Changed 8 months ago by m.lewandowski

  • Status changed from new to confirmed

comment:2 Changed 8 months ago by t.jakut

The current implementation on branch:t/14707 has some issues with Internet Explorer 8 – 23 tests fails there, at least one of them due to #13842.

I've also deleted workaround for #13886 and removed CKEDITOR.plugins.copyformatting._getAttributes in favor of our standard CKEDITOR.dom.element.getAttributes (introduced in #14602). Probably later I'll add some more adjustments.

I've also updated slightly API docs.


The main concern now is the support for IE8. The mentioned bug (#13842) is not a trivial one, but it seems to affect only tests, not actual functioning of the plugin. The plugin itself is working pretty well in IE8, but it was not thoroughly tested yet – more issues could be discovered.

comment:3 Changed 8 months ago by t.jakut

  • Owner set to t.jakut
  • Status changed from confirmed to assigned

comment:4 Changed 8 months ago by t.jakut

  • Status changed from assigned to review

OK, I've updated code on branch:t/14707. Now it's passing all tests in IE8 (although there is some workaround for #13842 in tests/plugins/copyformatting/getSelectedWordOffset.js. I've also fixed some issue with applying formatting to the table headers (the styles were also propagated to the siblings after the click).

comment:5 Changed 8 months ago by t.jakut

I've prepared some SDK examples and updated API docs on branch:t/14707.

comment:6 Changed 4 months ago by m.lewandowski

Code is good, but API docs required changes. I already applied and pushed them.

One code change was to create CKEDITOR.plugins.copyformatting.state type ("class") which then would be instantiated in CKEDITOR.editor instance. The reason for it was to keep docs clean, as former approach was creating a new namespace within the CKEDITOR.editor object.

Also extracting SDK samples into a separate ticket (#16489) to have Copy Formatting already merged into the master.

comment:7 Changed 4 months ago by t.jakut

LGTM. I've moved filter configuration into CKEDITOR.plugins.copyformatting.state constructor and fixed some typos in tests and API docs. Pushed changes to t/14707.

comment:8 Changed 4 months ago by m.lewandowski

Great job! It went golden with git:757c4a9ecd7a26d11c73f24bf02c26e51fa0e004 all the way to the major.

comment:9 Changed 4 months ago by m.lewandowski

  • Status changed from review to review_passed

comment:10 Changed 4 months ago by m.lewandowski

  • Resolution set to fixed
  • Status changed from review_passed to closed
Note: See TracTickets for help on using tickets.
© 2003 – 2016 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy