Opened 7 years ago

Closed 7 years ago

#3831 closed Bug (fixed)

Cannot remove contextmenu from plugins

Reported by: theromulans Owned by: arczi
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 arczi 7 years ago.
3831_2.patch (1.8 KB) - added by arczi 7 years ago.
3831_3.patch (1.3 KB) - added by arczi 7 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 7 years ago by arczi

  • Keywords Confirmed added
  • Milestone set to CKEditor 3.0
  • Owner set to arczi
  • Status changed from new to assigned

Changed 7 years ago by arczi

comment:2 Changed 7 years ago by arczi

  • Keywords Review? added

comment:3 Changed 7 years ago by arczi

  • Keywords Review- added; Review? removed

Ops, menu button doesn't work.

Changed 7 years ago by arczi

comment:4 Changed 7 years ago by arczi

  • 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 7 years ago by fredck

  • 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 7 years ago by arczi

comment:6 Changed 7 years ago by arczi

  • Keywords Review? added; Review- removed

comment:7 Changed 7 years ago by fredck

  • 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 7 years ago by arczi

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed with [3855]

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