Opened 3 years ago

Closed 3 years ago

#13173 closed Bug (fixed)

[Adv. toolbar conf] config.removeButtons is ignored

Reported by: Piotrek Koszuliński Owned by: Artur Delura
Priority: Normal Milestone: CKEditor 4.5.0
Component: Toolbar Configurator Version: 4.5.0 Beta
Keywords: Cc:

Description

  1. Build basic preset.
  2. Open the adv toolbar configurator
  3. See that in the toolbar there are no undo and redo buttons but there are in the source.
  4. Switch to the basic configurator and see that it correctly disables those buttons.

Change History (10)

comment:1 Changed 3 years ago by Jakub Ś

Status: newconfirmed

comment:2 Changed 3 years ago by Artur Delura

Owner: set to Artur Delura
Status: confirmedassigned

comment:3 Changed 3 years ago by Artur Delura

To reproduce this issue on dev version we need to update a config.js to

CKEDITOR.editorConfig = function( config ) {

	config.plugins =
		'floatingspace,' +
		'list,' +
		'link,' +
		'toolbar,' +
		'undo,' +
		'wysiwygarea';

	config.toolbarGroups = [
		{ name: 'clipboard',   groups: [ 'undo' ] },
		{ name: 'links' }
	];

	config.removeButtons = 'Undo,Redo,Anchor';
};

comment:4 Changed 3 years ago by Artur Delura

Config listed above is translated to

CKEDITOR.editorConfig = function( config ) {
	config.toolbar = [
		{ name: 'clipboard', items: [ 'Undo', 'Redo' ] },
		{ name: 'links', items: [ 'Link', 'Unlink', 'Anchor' ] }
	];
	config.removeButtons = 'Undo,Redo,Anchor';
}

but do we really should handle removeButtons in adv toolbar configurator? I think we should end up with

CKEDITOR.editorConfig = function( config ) {
	config.toolbar = [
		{ name: 'links', items: [ 'Link', 'Unlink' ] }
	];
}

It's simpler and more logical - Why add things and remove them at once?

Last edited 3 years ago by Artur Delura (previous) (diff)

comment:5 Changed 3 years ago by Piotrek Koszuliński

Yes, I've been thinking about removing these buttons which are listed in config.removeButtons in config.js from the config.toolbar table that you can see in the configurator.

comment:6 Changed 3 years ago by Artur Delura

So one-liner fix in branch:t/13173b.

comment:7 Changed 3 years ago by Artur Delura

Status: assignedreview

comment:8 Changed 3 years ago by Wiktor Walc

Component: GeneralToolbar Configurator

comment:9 Changed 3 years ago by Piotrek Koszuliński

Status: reviewreview_passed

I noticed that the priority of config.toolbar and config.removeButtons is different in the toolbar configurator than in the toolbar plugin. E.g:

	config.toolbar = [ [ 'Source', 'Undo' ] ];
	config.removeButtons = 'Undo';

In such case CKEditor renders only 'Source' button, while the toolbar configurator (the editor) includes both.

I think though, that we can ignore this case, because it may happen, but it isn't correct.

comment:10 Changed 3 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_passedclosed

Fixed on major with git:ba216e3.

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