Opened 9 years ago

Closed 8 years ago

#3068 closed Bug (fixed)

plugin:panel doesn't hide on subsequent click

Reported by: Garry Yao Owned by: Frederico Caldeira Knabben
Priority: Normal Milestone: CKEditor 3.0
Component: General Version: SVN (FCKeditor) - Retired
Keywords: Review- Firefox Cc:

Description (last modified by Garry Yao)

In The panel system (contextMenu and floatPanel)should auto hide on any click which doesn't fall into the panel region, now it's not working appropriately. It could be identified by the context menu

Procedures

  1. Open the replace by code example page;
  2. Left click on the document to open Context Menu;
  3. Click on the bold command;
  • Expected Result : The menu will hide before the command execution.
  1. Right click on the document region outside the menu;
  • Actual Result : The menu has always been there.

Attachments (3)

3068.patch (5.1 KB) - added by Artur Formella 9 years ago.
3068_2.patch (4.6 KB) - added by Artur Formella 8 years ago.
3068_3.patch (1.9 KB) - added by Frederico Caldeira Knabben 8 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 9 years ago by Garry Yao

Description: modified (diff)

comment:2 Changed 9 years ago by Artur Formella

Owner: set to Artur Formella
Status: newassigned

Changed 9 years ago by Artur Formella

Attachment: 3068.patch added

comment:3 Changed 9 years ago by Artur Formella

Keywords: Review? added

Tested in FF3, IE6, IE7, Chrome and Safari.

comment:4 Changed 9 years ago by Artur Formella

#3219 has been marked as DUP.

comment:5 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

I found the following problems:

  1. Codes around L151 of plugins/floatpanel/plugin.js is registering handlers on the same iframe element which would interfere with each one among the panels.
  2. L90 of plugins\stylescombo\plugin.js could be removed since the selection's already locked.
  3. Not sure why the function CKEDITOR.ui.floatPanel.allowBlur?
  4. [Minor]The patch path is not starting from project root.

comment:6 in reply to:  5 Changed 8 years ago by Artur Formella

Replying to garry.yao:

  1. L90 of plugins\stylescombo\plugin.js could be removed since the selection's already locked.

Could you provide this line?

comment:7 Changed 8 years ago by Artur Formella

About CKEDITOR.ui.floatPanel.allowBlur:

When float panel loses focus it should hide, but there is one exception when we click other combo/panelbutton.

In normal situation:

click combo1 (float1)
	"this.onShow.call( this );"
	show
	set focus (float1)
click combo2 (float2)
	"this.onShow.call( this );"
	show
	set focus (float2) and blur float1 so combo1 should close

The exception can be seen when float1=float2 (the same float panel, but different blocks)

click combo1 (float1)
	"this.onShow.call( this );"
	show
	set focus (float1)

click combo2 (float1)
	"this.onShow.call( this );"
	show
	set focus:
		blur float1 so float1 is hidden
		set focus ( but float1 is hidden)

This problem doesn't exists in IE, because focus and blur are set after some time and float1 in not hidden.

I couldn't workaround this so I decided to don't allow to close float panel in time between "this.onShow.call( this );" and "set focus"

Changed 8 years ago by Artur Formella

Attachment: 3068_2.patch added

comment:8 Changed 8 years ago by Artur Formella

Keywords: Review? added; Review- removed

comment:9 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed
Owner: changed from Artur Formella to Frederico Caldeira Knabben
Status: assignednew

I've checked the V2 code, and it started to sound strange to me that the V3 version, which is based on V2, is not working in this sense. Then, I've noted that there is a small difference in the way we register the focus/blur events in V2. We "useCapture" there for it, and it looks that this is the key to make it work. I'll come with a new patch for it.

Changed 8 years ago by Frederico Caldeira Knabben

Attachment: 3068_3.patch added

comment:10 Changed 8 years ago by Frederico Caldeira Knabben

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

The fact is that we are always forcing to not use capture in our event system. After all, IE doesn't have support for it, so we try to mimic it. So, I've introduced a way to force capture for those few cases we may need it.

comment:11 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

Fred's patch works well with focus switch between float panel <-> editor but not intermediate float panels' focus change, which Artur has introduced 'CKEDITOR.ui.floatPanel.allowBlur' for. The problem probably get resolved if these two patches are merged.

comment:12 Changed 8 years ago by Artur Formella

Keywords: Firefox added

Actually works in Chrome, Safari, IE (clicks are lost) and Opera.

comment:13 Changed 8 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: assignedclosed

It looks like the resent changes on the panels have fixed this issue.

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