Ticket #6011 (closed Bug: fixed)

Opened 4 years ago

Last modified 4 years ago

Disabling plugins should be more intuitive

Reported by: wwalc Owned by: tobiasz.cudnik
Priority: Normal Milestone: CKEditor 3.4.2
Component: General Version: 3.0
Keywords: Cc:

Description

I'm not sure it it's just a problem with the contextmenu, anyway I guess it deserves a ticket.

If anyone wants to disable the contextmenu, it's quite logical that he'll do something like:

config.removePlugins = 'contextmenu';

Unfortunately it will not work. Instead, ones have to search in source code for all plugins that require contextmenu and disable them. I believe it should happen automatically. Why?

In CKEditor 3.2 to disable the context menu it was enough to add:

removePlugins : 'scayt,menubutton,contextmenu'

Unfortunately if someone upgraded to CKEditor 3.3, he noticed that CKEditor suddenly stopped working. This is because another plugin that requires contextmenu has been added (liststyle), causing a JS error:

editor.addMenuGroup is not a function

Attachments

6011.patch (2.9 KB) - added by tobiasz.cudnik 4 years ago.
6011_2.patch (5.2 KB) - added by tobiasz.cudnik 4 years ago.

Change History

comment:1 Changed 4 years ago by fredck

  • Status changed from new to confirmed
  • Keywords Discussion removed
  • Milestone set to CKEditor 3.5

The root of the problem here is not making "all" plugins removal easy. There are some plugins instead that are definitely optional, like contextmenu, and those should be taken with care by other plugins.

What happens is that other plugins simply consider that all dependent plugins are loaded and use their API extensions freely. That's bad and wrong. We need to remove this issues from our code.

For the context menu specific issue, please check #4652. I'm leaving this ticket open so we can take other plugins in consideration also.

comment:2 Changed 4 years ago by alfonsoml

The problem with the liststyle plugin is #5799

comment:3 Changed 4 years ago by tobiasz.cudnik

  • Owner set to tobiasz.cudnik
  • Status changed from confirmed to assigned

Related to #4652.

Changed 4 years ago by tobiasz.cudnik

comment:4 Changed 4 years ago by tobiasz.cudnik

  • Status changed from assigned to review

Besides liststyle plugin I've decoupled clipboard from dialog plugin, as copy/paste works mostly well without it.

comment:5 Changed 4 years ago by fredck

  • Status changed from review to review_failed
  • There is no need to require the "menu" plugin in liststyle, because we have editor.addMenuItems check (fixed at #5799).
  • In the menubutton plugin instead, it looks like simply removing the contextmenu require is not enough, because CKEDITOR.plugins.contextMenu is still called, which should through an error (not tested, just guessing).
  • In fact, the contextmenu plugin is (currently) definitely required by the menubutton plugin, so it's wrong to remove it from the require list. The real issue is that we should not have the contextMenu class being used by the menubutton plugin, but the more generic menu class instead.

comment:6 Changed 4 years ago by tobiasz.cudnik

  • Status changed from review_failed to review

I've decoupled menubutton and contextmenu plugins, although we could save about 40 lines of code if we would make this logic shared inside menu plugin or even by introducing the a new one.

comment:7 Changed 4 years ago by tobiasz.cudnik

It seems that there is already #4652 for moving this logic into menu plugin, scheduled for 3.5...

Changed 4 years ago by tobiasz.cudnik

comment:8 Changed 4 years ago by garry.yao

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

'clipboard' plugin is not subjected to change, it's not functionally complete without the help from dialogs, it's a matter of distinguishing 'dependency' and 'utilization'.

The problem stated at this ticket has been fixed, which is 'liststyle' plugin, and similar fixes like that should come as [MicroChanges micro-change] once discovered, we're safe to close this ticket now.

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