DoubleClick plugin vs Xinha.Config.dblclickList

[ejucovy]

In r1256 a Xinha.Config.dblclickList option was created:

  this.dblclickList = 
  {
    "a": [function(e, target) {e._createLink(target);}],
    "img": [function(e, target) {e._insertImage(target);}]
  };

This functionality already existed, as the DoubleClick plugin:

http://trac.xinha.org/browser/trunk/plugins/DoubleClick/DoubleClick.js#L38

The differences are:

• The DoubleClick plugin attaches its configuration to the Xinha editor object itself, instead of config. This now goes against Xinha's best practices.
• The DoubleClick plugin requires a plugin to be installed (obviously)
• The DoubleClick plugin uses the name dblClickList. The new core config uses the name dblclickList (note the case difference).

I think the DoubleClick plugin should be deprecated and moved to the unsupported_plugins directory, and it should issue a warning if used. The warning should direct users to the new Xinha.Config.dblclickList option.

(See #1485)

[gogo]

Agreed.

Looks like the plugin also handles td (but perhaps that should be table), built in should do that to.

Probably best if the deprecated plugin is cut down to simply copying it's config into the now built in config and let the built in do everything from then.

[ejucovy]

Replying to gogo:

Probably best if the deprecated plugin is cut down to simply copying it's config into the now built in config and let the built in do everything from then.

Oh, interesting -- you mean, the plugin should copy its dblClickList to editor.config.dblclickList on-load, and do nothing else?

That makes sense -- +1

[ejucovy]

plugin sets config.dblclickList and lets core handle the rest

[ejucovy]

Replying to ejucovy:

Replying to gogo:

Probably best if the deprecated plugin is cut down to simply copying it's config into the now built in config and let the built in do everything from then.

Oh, interesting -- you mean, the plugin should copy its dblClickList to editor.config.dblclickList on-load, and do nothing else?

I attached a patch that does this -- let me know if this wasn't what you had in mind.

(The patch isn't a complete fix for this issue -- I'll put together a complete patch later. First I just want to confirm the approach.)

By the way, is there a better way to iterate over the items in dblClickList? I'm doing for( var i in x ) with explicit typeof checks for both i and x[i] .. is there a better way to do this in javascript?

[gogo]

Yes that looks fine to me.

I think the typeof checks are the safe way of doing it, yes.

[ejucovy]

I applied the above patch, r1276.

Now just need to move it to unsupported_plugins.

I think the DoubleClick plugin should be deprecated and moved to the unsupported_plugins directory, and it should issue a warning if used. The warning should direct users to the new Xinha.Config.dblclickList option.

Actually, I won't specifically issue a warning directing users to the new option. Instead, when the plugin is loaded from unsupported_plugins, core emits a debug message which points to a documentation page about all the unsupported plugins (see #1558). That page explains what to use instead of the DoubleClick plugin. I think that's good enough.

[ejucovy]

I moved the plugin to unsupported_plugins. r1277.

[ejucovy]

Replying to gogo:

Looks like the plugin also handles td (but perhaps that should be table), built in should do that to.

Actually I'm not sure about this. I tested it, and the behavior is kind of bizarre. It inserts a sub-table within the current td, instead of calling up some way to edit the current table's options. This is because of the behavior of InsertTable?. So I think it's best to leave it as is, at least until/unless InsertTable? is changed to let you edit the current table.

(If TableOperations is used, we might want to call up the "edit table properties" dialog. I'm not sure. But, if TableOperations isn't installed, there's no good UI to use for this.)

[gogo]

Yes sounds like the old "td" double click isn't very useful.

The place to add a better handler would be in TableOperations, it can feed handlers into the core's double click list as it desires.

[ejucovy]

Replying to gogo:

Yes sounds like the old "td" double click isn't very useful. The place to add a better handler would be in TableOperations, it can feed handlers into the core's double click list as it desires.

Good idea, that makes sense. I've filed this enhancement at #1560.