Ticket #3068 (closed Bug: fixed)

Opened 6 years ago

Last modified 5 years ago

plugin:panel doesn't hide on subsequent click

Reported by: garry.yao Owned by: fredck
Priority: Normal Milestone: CKEditor 3.0
Component: General Version: SVN (FCKeditor) - Retired
Keywords: Review- Firefox Cc:

Description (last modified by garry.yao) (diff)

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

3068.patch (5.1 KB) - added by arczi 6 years ago.
3068_2.patch (4.6 KB) - added by arczi 6 years ago.
3068_3.patch (1.9 KB) - added by fredck 6 years ago.

Change History

comment:1 Changed 6 years ago by garry.yao

  • Description modified (diff)

comment:2 Changed 6 years ago by arczi

  • Owner set to arczi
  • Status changed from new to assigned

Changed 6 years ago by arczi

comment:3 Changed 6 years ago by arczi

  • Keywords Review? added

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

comment:4 Changed 6 years ago by arczi

#3219 has been marked as DUP.

comment:5 follow-up: ↓ 6 Changed 6 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 6 years ago by arczi

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 6 years ago by arczi

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 6 years ago by arczi

comment:8 Changed 6 years ago by arczi

  • Keywords Review? added; Review- removed

comment:9 Changed 6 years ago by fredck

  • Keywords Review- added; Review? removed
  • Owner changed from arczi to fredck
  • Status changed from assigned to new

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 6 years ago by fredck

comment:10 Changed 6 years ago by fredck

  • Status changed from new to assigned
  • Keywords Review? added; Review- removed

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 6 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 5 years ago by arczi

  • Keywords Firefox added

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

comment:13 Changed 5 years ago by fredck

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

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

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