Opened 10 years ago

Closed 10 years ago

#1609 closed New Feature (fixed)

Make it possible to use RegisterDoubleClickHandler with any tag

Reported by: Alfonso Martínez de Lizarrondo Owned by: Alfonso Martínez de Lizarrondo
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 (2)

1609.patch (1.2 KB) - added by Alfonso Martínez de Lizarrondo 10 years ago.
Proposed patch
1609_2.patch (1.5 KB) - added by Alfonso Martínez de Lizarrondo 10 years ago.
Revised patch

Download all attachments as: .zip

Change History (6)

Changed 10 years ago by Alfonso Martínez de Lizarrondo

Attachment: 1609.patch added

Proposed patch

comment:1 Changed 10 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review? added
Milestone: 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 10 years ago by Martin Kou

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 10 years ago by Alfonso Martínez de Lizarrondo

Attachment: 1609_2.patch added

Revised patch

comment:3 Changed 10 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review? added; Review- removed
Owner: set to Alfonso Martínez de Lizarrondo
Status: newassigned

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 10 years ago by Martin Kou

Keywords: Review+ added; Review? removed
Resolution: fixed
Status: assignedclosed

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

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