Opened 10 years ago

Closed 10 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 10 years ago.
3831_2.patch (1.8 KB) - added by Artur Formella 10 years ago.
3831_3.patch (1.3 KB) - added by Artur Formella 10 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 10 years ago by Artur Formella

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

Changed 10 years ago by Artur Formella

Attachment: 3831.patch added

comment:2 Changed 10 years ago by Artur Formella

Keywords: Review? added

comment:3 Changed 10 years ago by Artur Formella

Keywords: Review- added; Review? removed

Ops, menu button doesn't work.

Changed 10 years ago by Artur Formella

Attachment: 3831_2.patch added

comment:4 Changed 10 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 10 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 10 years ago by Artur Formella

Attachment: 3831_3.patch added

comment:6 Changed 10 years ago by Artur Formella

Keywords: Review? added; Review- removed

comment:7 Changed 10 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 10 years ago by Artur Formella

Resolution: fixed
Status: assignedclosed

Fixed with [3855]

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