Ticket #3041 (closed Bug: fixed)

Opened 6 years ago

Last modified 5 years ago

plugin:colorbutton incorrect state after color selected.

Reported by: garry.yao Owned by: arczi
Priority: Normal Milestone: CKEditor 3.0
Component: General Version: SVN (CKEditor) - OLD
Keywords: Confirmed Review+ Cc:

Description

The bgColor and textColor button should have no states after applying the styles. Currently we can still found sometimes these two buttons toggled on after panel is closed.

Attachments

test-colorbutton-states.patch (3.9 KB) - added by garry.yao 6 years ago.
3041.patch (1.7 KB) - added by garry.yao 6 years ago.
3041_2.patch (1.5 KB) - added by arczi 5 years ago.

Change History

Changed 6 years ago by garry.yao

comment:1 Changed 6 years ago by fredck

  • Keywords Confirmed added
  • Version changed from SVN (FCKeditor) to SVN (CKEditor)

See #3163 for information on how to reproduce it.

comment:2 Changed 6 years ago by garry.yao

  • Owner set to garry.yao
  • Status changed from new to assigned

When adding the following lines arround _source\plugins\panelbutton\plugin.js for debugging, it is found that the 'CKEDITOR.TRISTATE_OFF' state is incorrectly applied to 'text-color' when hidding the 'bg-color' panel.

		this._.__defineSetter__( 'state', function( state ){
			console.log( this.id );
			return state;
		} );

Changed 6 years ago by garry.yao

comment:3 Changed 6 years ago by garry.yao

  • Keywords Review? added

The bug was found to be caused by some incorrect handler function caching.

comment:4 follow-up: ↓ 5 Changed 6 years ago by fredck

  • Keywords Review- added; Review? removed

While this fix even makes sense, as we have a reference to "panel" that can't be shared by all buttons, it makes no difference for me. I can reproduce the problem in the same way before and after the patch:

  1. Click the "Background Color" button.
  2. Click the "Foreground Color" button.
  3. Click the "Background Color" twice. The "Foreground Color" button remains hilighted.

This looks like an issue with the floating panel system, as something similar can be done with the combos also.

In any case, please move the "clickFn" variable declaration from line 16 to line 71 at this point and commit your changes. A final solution is still to be found though.

comment:5 in reply to: ↑ 4 Changed 6 years ago by arczi

Replying to fredck:

  1. Click the "Background Color" button.
  2. Click the "Foreground Color" button.
  3. Click the "Background Color" twice. The "Foreground Color" button remains hilighted.

Patch from #3219 fixes this bug.

comment:6 Changed 6 years ago by garry.yao

Committed the patch with [3323].

comment:7 Changed 5 years ago by arczi

  • Owner changed from garry.yao to arczi
  • Status changed from assigned to new

I'm taking over the ticket after talk with Garry.

Changed 5 years ago by arczi

comment:8 Changed 5 years ago by arczi

  • Keywords Review? added; Review- removed

It was a part of http://dev.fckeditor.net/attachment/ticket/3068/3068_2.patch

This is the simplest way, is solves also #3222.

comment:9 Changed 5 years ago by martinkou

  • Keywords Review+ added; Review? removed

comment:10 Changed 5 years ago by arczi

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

Fixed with [3552]

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