Opened 10 years ago

Closed 10 years ago

#3728 closed Bug (fixed)

Automatic color options don't seem to do anything

Reported by: Damian Owned by: Frederico Caldeira Knabben
Priority: Normal Milestone: CKEditor 3.0
Component: General Version:
Keywords: IBM Confirmed Review+ Cc:

Description

The Font color and Background color toolbar options provide an "Automatic" selection. Based on the way that FCKEditor 2.6 has implemented this option, it should reset the background/font color. This doesn't appear to work in the nightly CKEditor builds.

Attachments (2)

3728.patch (843 bytes) - added by Martin Kou 10 years ago.
3728_2.patch (560 bytes) - added by Frederico Caldeira Knabben 10 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 10 years ago by Martin Kou

Keywords: Confirmed added

comment:2 Changed 10 years ago by Martin Kou

Owner: set to Martin Kou
Status: newassigned

Changed 10 years ago by Martin Kou

Attachment: 3728.patch added

comment:3 Changed 10 years ago by Martin Kou

Keywords: Review? added

This is due to some missing logic in colorbutton plugin.

comment:4 Changed 10 years ago by Garry Yao

Keywords: Review- added; Review? removed

I think you fixing is somehow conflicting with the existing logic there:

					if ( color )
						style.apply( editor.document );
					else
						style.remove( editor.document );

So the correct fix would either remove the current colored style OR apply the inherited style.

comment:5 Changed 10 years ago by Martin Kou

Keywords: Review? added; Review- removed

There's no conflict there. If the color is to be removed, then color would be null and thus style.remove() is executed. What's missing is that we need to detect the color to be removed before running style.remove(), and that's what my patch does.

comment:6 Changed 10 years ago by Garry Yao

Keywords: Review+ added; Review? removed

Sorry, my fault, doesn't get the idea of your patch.

comment:7 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review+ removed

The proposed patch doesn't work on mixed colors selection.

comment:8 Changed 10 years ago by Frederico Caldeira Knabben

Owner: changed from Martin Kou to Frederico Caldeira Knabben
Status: assignednew

It looks like it was just a small misunderstanding in the original implementation. I've a new patch for it.

Changed 10 years ago by Frederico Caldeira Knabben

Attachment: 3728_2.patch added

comment:9 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review? added; Review- removed
Status: newassigned

comment:10 Changed 10 years ago by Martin Kou

Keywords: Review+ added; Review? removed

comment:11 Changed 10 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: assignedclosed

Fixed with [3688].

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