Opened 10 years ago

Closed 10 years ago

#1609 closed New Feature (fixed)

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:


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 alfonsoml 10 years ago.
Proposed patch
1609_2.patch (1.5 KB) - added by alfonsoml 10 years ago.
Revised patch

Download all attachments as: .zip

Change History (6)

Changed 10 years ago by alfonsoml

Proposed patch

comment:1 Changed 10 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');

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

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

comment:2 Changed 10 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 10 years ago by alfonsoml

Revised patch

comment:3 Changed 10 years ago by alfonsoml

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

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 martinkou

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

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