Ticket #1609 (closed New Feature: fixed)

Opened 6 years ago

Last modified 6 years ago

Make it possible to use RegisterDoubleClickHandler with any tag

Reported by: alfonsoml Owned by: alfonsoml
Priority: Normal Milestone: FCKeditor 2.6
Component: General Version:
Keywords: Review+ Cc:

Description

Now in order to use the RegisterDoubleClickHandler you must specify the node tags that you want to watch, but it could be better to also be able to register the event listener with any tag (so if you want to have a listener for both span and img you don't have to call it twice, thing get worse if you need to listen to various different elements)

Attachments

1609.patch (1.2 KB) - added by alfonsoml 6 years ago.
Proposed patch
1609_2.patch (1.5 KB) - added by alfonsoml 6 years ago.
Revised patch

Change History

Changed 6 years ago by alfonsoml

Proposed patch

comment:1 Changed 6 years ago by alfonsoml

  • Keywords Review? added
  • Milestone set to FCKeditor 2.6

The proposed patch includes the suggestion of #1608 and the code to have a generic handler.

to test it I've used this simple code:


function FCKeditor_OnComplete(editorInstance)
{
	editorInstance.RegisterDoubleClickHandler(ClickLinks, 'a');
	editorInstance.RegisterDoubleClickHandler(Everything);
}

function ClickLinks(o)
{
	alert("Link, " + o.nodeName)
}

function Everything(o)
{
	alert("Node, " + o.nodeName)
}

comment:2 Changed 6 years ago by martinkou

  • Keywords Review- added; Review? removed

It seems to me that line 289 - 298 might be unsafe, since the reference to oldFunction would be changed and thus at most only two function references can be stored. I might be wrong though.

How about changing it to use an array or an object to store all the event handlers like in FCKEvents? That's the more traditional and time-proven way of implementing a publisher and subscriber pattern.

Changed 6 years ago by alfonsoml

Revised patch

comment:3 Changed 6 years ago by alfonsoml

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

The trick in that code was that each new function was wrapping the previous one, so it would work, although I agree that it was far from ideal and should have rewritten it instead of just trying to use directly in the way that it was suggested.

The new patch uses exactly the same code than the FCKEvents.

comment:4 Changed 6 years ago by martinkou

  • Keywords Review+ added; Review? removed
  • Status changed from assigned to closed
  • Resolution set to fixed

I've reviewed and tested the patch, and merged it to trunk in [1178].

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