editor._createLink is not a function

[guest]

There's some confusing code in XinhaCore.js related to execCommand("_createLink"):

	    case "createlink":
	      this._createLink();

The _createLink function is never defined on the Xinha editor object.

From code comments elsewhere, it looks like the intention is to define the function in a plugin, which will be modules/CreateLink if no other plugin (like Linker) overrides it. ("This is the standard implementation of the Xinha.prototype._createLink method")

But, Xinha.prototype._createLink is NOT defined by Linker, nor by CreateLink. Instead, both plugins work by overriding the default behavior of the createlink button on the toolbar.

Default behavior:

    createlink: [ "Insert Web Link", ["ed_buttons_main.png",6,1], false, function(e) { e._createLink(); } ],

CreateLink override:

   editor.config.btnList.createlink[3] = function() { self.show(self._getSelectedAnchor()); }

Linker override:

    editor.config.btnList.createlink[3]
      =  function(e, objname, obj) { linker._createLink(linker._getSelectedAnchor()); };

(Here linker refers to the Linker plugin object, not the xinha editor object. The Linker object defines a _createLink method on itself, but not on the xinha editor object.)

CreateLink is a module, so it will always be loaded and override the default behavior. But there are three problems with this:

1. It's confusing to somebody reading the code.
2. It means there is no standard way to trigger a "createlink" command or to listen for a "createlink" event (which would be useful; see #1552)
3. XinhaCore.js itself assumes the editor object has a _createLink method

XinhaCore.js tries to call editor._createLink() if you double-click on an anchor element:

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

This causes a Javascript error in browsers, because e._createLink is not defined.

In fact the DoubleClick plugin has to override this definition:

  this.editor.dblClickList = {
    a: [ function(e) {e.config.btnList['createlink'][3](e); } ],

The same approach is used with InsertImage and editor._insertImage, but there it seems to be done (more) correctly: InsertImage defines editor.prototype._insertImage, and execCommand("insertimage") calls that function.

Proposed solutions:

1. (Better solution) The code should define editor.prototype._createLink, and execCommand("createlink") should be the standard way to trigger a link-creation action.
2. (Worse solution) References to editor._createLink should be removed.

[guest]

I've attached a patch which addresses these problems.

(It's from a git diff, so patch -p1 < link.diff - not -p0.)

The patch does the following:

• Linker and CreateLink now define an editor._createLink function, when they are first loaded. In both cases, the function gets the selected anchor, and then calls the plugin's own "interesting" method (which is historically called .show CreateLink and ._createLink in Linker). The function can also receive an explicit anchor element to be used instead of the selected anchor.
• editor.execCommand("createlink") is now the standard way to invoke link-creation. To pass an explicit anchor element, use editor.execCommand("createlink", false, anchorEl) instead.
• The default double-click handler for anchor elements now calls `editor.execCommand("createlink", false, anchorEl). This is defined in XinhaCore.js and also in the DoubleClick plugin.

Further cleanup would be nice - the two linky plugins both define some functions with identical logic that could be moved to core, and I think the same could be said of the default logic for the anchor-double-clicker. But those can be addressed separately, and are not essential.

[guest]

fixed case issues in original patch (createlink vs createLink)

[ejucovy]

I've now committed the patch in r1271

For posterity: I tried to track down when and why the editor._createLink confusion began. I traced it back to http://trac.xinha.org/browser/branches/ray/modules/CreateLink/link.js?rev=892 but I wasn't able to dig any farther, and that changeset doesn't seem to explain why.

[ejucovy]

Replying to guest:

Further cleanup would be nice - the two linky plugins both define some functions with identical logic that could be moved to core, and I think the same could be said of the default logic for the anchor-double-clicker. But those can be addressed separately, and are not essential.

I filed a ticket to record this "nicetohave", #1554