Opened 7 years ago

Closed 7 years ago

#6011 closed Bug (fixed)

Disabling plugins should be more intuitive

Reported by: Wiktor Walc 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 (2)

6011.patch (2.9 KB) - added by Tobiasz Cudnik 7 years ago.
6011_2.patch (5.2 KB) - added by Tobiasz Cudnik 7 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 7 years ago by Frederico Caldeira Knabben

Keywords: Discussion removed
Milestone: CKEditor 3.5
Status: newconfirmed

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 7 years ago by Alfonso Martínez de Lizarrondo

The problem with the liststyle plugin is #5799

comment:3 Changed 7 years ago by Tobiasz Cudnik

Owner: set to Tobiasz Cudnik
Status: confirmedassigned

Related to #4652.

Changed 7 years ago by Tobiasz Cudnik

Attachment: 6011.patch added

comment:4 Changed 7 years ago by Tobiasz Cudnik

Status: assignedreview

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

comment:5 Changed 7 years ago by Frederico Caldeira Knabben

Status: reviewreview_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 7 years ago by Tobiasz Cudnik

Status: review_failedreview

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 7 years ago by Tobiasz Cudnik

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

Changed 7 years ago by Tobiasz Cudnik

Attachment: 6011_2.patch added

comment:8 Changed 7 years ago by Garry Yao

Resolution: fixed
Status: reviewclosed

'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 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy