Opened 8 years ago

Closed 8 years ago

#3831 closed Bug (fixed)

Cannot remove contextmenu from plugins

Reported by: Brett Owned by: Artur Formella
Priority: Normal Milestone: CKEditor 3.0
Component: General Version: 3.0 RC
Keywords: Confirmed Review+ Cc:

Description

Attempting to create a new config.js file with a command such as:

config.removePlugins = 'contextmenu';

...causes the WYSIWYG to not load. I tested a few more plugins and they caused the same issue to occur (tho, equally, others don't cause this issue to occur)

Windows 2003 Server using the "Replace by Code" method - if that makes a difference.

Attachments (3)

3831.patch (479 bytes) - added by Artur Formella 8 years ago.
3831_2.patch (1.8 KB) - added by Artur Formella 8 years ago.
3831_3.patch (1.3 KB) - added by Artur Formella 8 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 8 years ago by Artur Formella

Keywords: Confirmed added
Milestone: CKEditor 3.0
Owner: set to Artur Formella
Status: newassigned

Changed 8 years ago by Artur Formella

Attachment: 3831.patch added

comment:2 Changed 8 years ago by Artur Formella

Keywords: Review? added

comment:3 Changed 8 years ago by Artur Formella

Keywords: Review- added; Review? removed

Ops, menu button doesn't work.

Changed 8 years ago by Artur Formella

Attachment: 3831_2.patch added

comment:4 Changed 8 years ago by Artur Formella

Keywords: Review? added; Review- removed

CKEDITOR.plugins.contextMenu is required by buttonMenu.

There are 2 ways to fix this bug:

first - disable context menu if is loaded indirect,

second - rename CKEDITOR.plugins.contextMenu and move it somewhere else (i.e. to menu plugin) and create editor.contextMenu in contextmenu plugin (first part of current plugin)

Patch contains the first solution because is simpler.

comment:5 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

Removing the plugin is not a guarantee that it will not be loaded. There may be dependencies that will require it. What we're missing is having these dependencies well defined in the plugins, so nothing breaks.

For this specific case, we need the contextmenu plugin for the buttonmenu plugin, as identified by Artur. So, it's just a matter of updating the plugin requirements.

I would instead avoid adding the automation for disabling the context menu. It may be "really" required by other plugins. We may instead have a configuration option to disable it, but this is something that should be managed in another ticket.

So, the changes on _source/plugins/menubutton/plugin.js are the only one to be committed.

Changed 8 years ago by Artur Formella

Attachment: 3831_3.patch added

comment:6 Changed 8 years ago by Artur Formella

Keywords: Review? added; Review- removed

comment:7 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

The changelog entry is wrong now. It should be something like: "The attempt to remove the contextmenu plugin will not anymore break the editor". Please fix it when committing.

comment:8 Changed 8 years ago by Artur Formella

Resolution: fixed
Status: assignedclosed

Fixed with [3855]

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